Re: [PATCH 3/3] HID: usbhid: defer LED setting to a workqueue

[Daniel Kurtz]

On Thu, Dec 15, 2011 at 5:01 PM, Oliver Neukum <oneukum@xxxxxxx> wrote:
>
> Am Donnerstag, 15. Dezember 2011, 07:43:09 schrieb Daniel Kurtz:
>
> Hi,
>
> > I'm sorry, I'm not seeing how using a workqueue to set the LEDs
> > changes anything with respect to how the driver deals with USB reset.
>
> You are unfortunately right. This means that the driver currently is already
> buggy.
>
> > With or without a workqueue, LED control urbs are being queued and/or
> > submitted asynchronously to reset.  AFAICT, the only thing that
> > changes here is exactly when __usbhid_submit_report() is called.  In
> > the original case, this happens directly in the input event handler.
> > In the workqueue case, it happens in a system worker thread some short
> > time after the input handler finishes.
>
> The current driver kills the ctrl URB in cease_io() which is not enough
> to prevent new IO.
>
> > Could you please be more specific about what this patch breaks and
> > perhaps give some guidance on how to fix it?
>
> It breaks nothing. It just continues a bug and I assumed it was not present.
> Basically the work queue must do nothing after pre_reset() and post_reset()
> ought to rerun the work in case some request came down during that time.

So, is this an Ack for this patchset?
Can we fix any existing races in later patches?

Regards,
-Daniel

>        Regards
>                Oliver
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/