Re: Panic when pressing Caps Lock in the console

Vitaliy Makkoveev <[email protected]> Tue, 23 Jun 2026 00:33:22 +0300
Newsgroups gmane.os.openbsd.bugs
Message-ID <[email protected]>
On Mon, Jun 22, 2026 at 05:06:13PM -0400, John Troy wrote:
> On 6/22/26 4:39 PM, John Troy wrote:
> > On 6/19/26 6:54 AM, Stefan Sperling wrote:
> >> Nice find. I suspect a task should be used here, perhaps depending on
> >> the type of child HID device. We will definitely want interrupts to
> >> be used when ihidev is serving touchpads since jcs@ spent quite a lot
> >> of effort on avoiding polling mode for them.
> >>
> > 
> > Thanks for taking a look! As far as I can tell, ikbd_set_leds() is the
> > only caller of ihidev_send_report(), so modifying it wouldn't interfere
> > with touchpads. That said, it looked simpler to put the task in the ikbd
> > code where only a single byte needs to be handed off to the task rather
> > than a buffer. The patch below is patterned after how wskbd handles
> > backlight/brightness. Let me know if this really does belong in ihidev,
> > or somewhere else, instead.
> > 
> > -John
> 
> Apologies, Thunderbird mangled that diff. I think I found the right knob
> to stop it.
> 

It's all kernel locked. You don't need atomic operations here.

> 
> diff --git a/sys/dev/i2c/ikbd.c b/sys/dev/i2c/ikbd.c
> index 6551b918f01..6d058d5b7e2 100644
> --- a/sys/dev/i2c/ikbd.c
> +++ b/sys/dev/i2c/ikbd.c
> @@ -23,6 +23,8 @@
>  #include <sys/device.h>
>  #include <sys/ioctl.h>
>  #include <sys/timeout.h>
> +#include <sys/atomic.h>
> +#include <sys/task.h>
>  
>  #include <dev/i2c/i2cvar.h>
>  #include <dev/i2c/ihidev.h>
> @@ -38,6 +40,8 @@ struct ikbd_softc {
>  	struct ihidev	sc_hdev;
>  #define sc_ledsize	sc_hdev.sc_osize
>  	struct hidkbd	sc_kbd;
> +	struct task	sc_led_task;
> +	u_int		sc_led_report;
>  	int		sc_spl;
>  };
>  
> @@ -55,6 +59,7 @@ const struct wskbd_consops ikbd_consops = {
>  
>  int	ikbd_enable(void *, int);
>  void	ikbd_set_leds(void *, int);
> +void	ikbd_set_leds_task(void *);
>  int	ikbd_ioctl(void *, u_long, caddr_t, int, struct proc *);
>  
>  const struct wskbd_accessops ikbd_accessops = {
> @@ -117,6 +122,8 @@ ikbd_attach(struct device *parent, struct device *self, void *aux)
>  
>  	printf("\n");
>  
> +	task_set(&sc->sc_led_task, ikbd_set_leds_task, sc);
> +
>  	if (kbd->sc_console_keyboard) {
>  		extern struct wskbd_mapdata ukbd_keymapdata;
>  
> @@ -133,8 +140,11 @@ ikbd_detach(struct device *self, int flags)
>  {
>  	struct ikbd_softc *sc = (struct ikbd_softc *)self;
>  	struct hidkbd *kbd = &sc->sc_kbd;
> +	int rv;
>  
> -	return hidkbd_detach(kbd, flags);
> +	rv = hidkbd_detach(kbd, flags);
> +	taskq_del_barrier(systq, &sc->sc_led_task);
> +	return rv;
>  }
>  
>  void
> @@ -173,11 +183,22 @@ ikbd_set_leds(void *v, int leds)
>  	uint8_t res;
>  
>  	if (sc->sc_ledsize && hidkbd_set_leds(kbd, leds, &res) != 0) {
> -		ihidev_send_report((struct device *)sc->sc_hdev.sc_parent,
> -		    sc->sc_hdev.sc_report_id, &res, 1);
> +		atomic_store_int(&sc->sc_led_report, res);
> +		task_add(systq, &sc->sc_led_task);
>  	}
>  }
>  
> +void
> +ikbd_set_leds_task(void *v)
> +{
> +	struct ikbd_softc *sc = v;
> +	uint8_t res;
> +
> +	res = atomic_load_int(&sc->sc_led_report);
> +	ihidev_send_report((struct device *)sc->sc_hdev.sc_parent,
> +	    sc->sc_hdev.sc_report_id, &res, 1);
> +}
> +
>  int
>  ikbd_ioctl(void *v, u_long cmd, caddr_t data, int flag, struct proc *p)
>  {
> 
>