Am Mittwoch, 25. April 2012, 03:27:19 schrieb Ming Lei:
> On Wed, Apr 25, 2012 at 2:57 AM, Oliver Neukum <oliver@xxxxxxxxxx> wrote:
>
> usb_submit_urb()
> >>
> >> This submit won't happen because HID_OUT_RUNNING is not cleared.
> >
> > I may be dense, but as far as I can tell a resubmit will happen, exactly if
> > HID_OUT_RUNNING is _not_ cleared.
>
> In fact, it should be a double unlink bug, usb_unlink_urb can handle
> it correctly
> if the lock is held. We also can deal with it easily by checking urb->unlinked,
> so how about the below patch?
>
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index aa1c503..b530463 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -411,10 +411,10 @@ static void hid_irq_out(struct urb *urb)
> {
> struct hid_device *hid = urb->context;
> struct usbhid_device *usbhid = hid->driver_data;
> - unsigned long flags;
> + unsigned long status = urb->status;
Error codes are negative.
> @@ -546,8 +557,13 @@ static void __usbhid_submit_report(struct
> hid_device *hid, struct hid_report *re
> * no race because this is called under
> * spinlock
> */
> - if (time_after(jiffies, usbhid->last_out + HZ * 5))
> +
> + if (time_after(jiffies, usbhid->last_out + HZ * 5) &&
> + !usbhid->urbout->unlinked) {
> + spin_unlock(&usbhid->lock);
> usb_unlink_urb(usbhid->urbout);
> + spin_lock(&usbhid->lock);
> + }
> }
> return;
> }
Same objection. You are just making the race unlikelier. The flag
needs to be set under a lock you hold while checking time_after().
We'd be back at the original proposal.
Regards
Oliver
--
- - -
SUSE LINUX Products GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer, HRB 16746 (AG Nürnberg)
Maxfeldstraße 5
90409 Nürnberg
Germany
- - -
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
