On Sun, Aug 18, 2013 at 9:01 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Sun, 18 Aug 2013, Ming Lei wrote:
>
>> Complete() will be run with interrupt enabled, so change to
>> spin_lock_irqsave().
>>
>> Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
>> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxxxxx>
>> ---
>> drivers/usb/core/message.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
>> index 82927e1..8bba734 100644
>> --- a/drivers/usb/core/message.c
>> +++ b/drivers/usb/core/message.c
>> @@ -266,8 +266,9 @@ static void sg_complete(struct urb *urb)
>> {
>> struct usb_sg_request *io = urb->context;
>> int status = urb->status;
>> + unsigned long flags;
>>
>> - spin_lock(&io->lock);
>> + spin_lock_irqsave(&io->lock, flags);
>>
>> /* In 2.5 we require hcds' endpoint queues not to progress after fault
>> * reports, until the completion callback (this!) returns. That lets
>> @@ -326,7 +327,7 @@ static void sg_complete(struct urb *urb)
>> if (!io->count)
>> complete(&io->complete);
>>
>> - spin_unlock(&io->lock);
>> + spin_unlock_irqrestore(&io->lock, flags);
>> }
>
> As far as I can see, these don't need to disable interrupts. All they
> protect against is the code in usb_sg_wait() and usb_sg_cancel(), which
> both run in process context.
Yes.
> But will lockdep complain if they don't disable interrupts?
Looks lockdep won't complain because the lock can't be held in
another hardirq context.
As I mentioned in 00/50, the patchset is basically a mechanical
change, so one patch can be dropped if anyone reviews and
concludes it isn't needed.
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
