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

On Thu, Dec 15, 2011 at 5:01 PM, Oliver Neukum [off-list ref] wrote:

Am Donnerstag, 15. Dezember 2011, 07:43:09 schrieb Daniel Kurtz:

Hi,

quoted

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.

quoted

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.

quoted

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-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html