On Wed, Jun 19, 2013 at 12:36 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, 18 Jun 2013, Ming Lei wrote:
>
>> We disable local IRQs here in case of running complete() by
>> tasklet to avoid possible deadlock because drivers may call
>> spin_lock() to hold lock which might be acquired in one hard
>> interrupt handler.
>>
>> The local_irq_save()/local_irq_restore() around complete()
>> will be removed if current USB drivers have been cleaned up
>> and no one may trigger the above deadlock situation when
>> running complete() in tasklet.
>
> This should be part of the first patch, not added on separately.
Yes, but I want to highlight the change since that will be removed
after drivers have been cleaned up.
>
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index a272968..09a8263 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>> @@ -1673,7 +1673,28 @@ static void __usb_hcd_giveback_urb(struct urb *urb)
>>
>> /* pass ownership to the completion handler */
>> urb->status = status;
>> - urb->complete (urb);
>> +
>> + /*
>> + * We disable local IRQs here in case of running complete() by
>> + * tasklet to avoid possible deadlock because drivers may call
>> + * spin_lock() to hold lock which might be acquired in one hard
>> + * interrupt handler.
>> + *
>> + * The local_irq_save()/local_irq_restore() around complete()
>> + * will be removed if current USB drivers have been cleaned up
>> + * and no one may trigger the above deadlock situation when
>> + * running complete() in tasklet.
>> + */
>> + if (hcd_giveback_urb_in_bh(hcd)) {
>> + unsigned long flags;
>> +
>> + local_irq_save(flags);
>> + urb->complete (urb);
>> + local_irq_restore(flags);
>> + } else {
>> + urb->complete (urb);
>> + }
>> +
>
> There's no reason to bother with the test. You might as well always do
> the local_irq_save.
Looks fine.
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
