On Wed, Apr 25, 2012 at 2:19 PM, Oliver Neukum <oneukum@xxxxxxx> wrote:
>> @@ -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().
urb->unlinked is checked with holding both the two lockes, but set with
holding the unlink lock or the lock or both, looks it is enough to
avoid the race, isn't it?
> We'd be back at the original proposal.
Introducing a new flag to describe 'unlinked' is still OK, but
borrowing urb->unlinked is better, the bad is that it is private.
Thanks,
--
Ming Lei
--
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
