Re: [2.4, 2.5, USB] locking issue

[David Brownell <david-b@pacbell.net>]

Greg KH wrote:
> On Mon, Nov 18, 2002 at 05:34:20PM +0300, Samium Gromoff wrote:
> 
>>        The possible problem is encountered in ehci-q.c and ehci-sched.c
>>  in 2.4.19-pre9 and in one occurence in ehci-q.c of 2.5.47.

It's not a problem, see below.  The 2.5 code is better, since only one
body of code is managing the TD queues for async (control, bulk) and
interrupt queue heads.


>>        the offending pattern is the same in both files:
>>
>>        if (!list_empty (qtd_list)) {
>> -----------------------8<----------------------------------------------
>>                list_splice (qtd_list, &qh->qtd_list);
>>                qh_update (qh, list_entry (qtd_list->next, struct ehci_qtd, qtd_list));
>> -----------------------8<----------------------------------------------
>>        } else {
>>                qh->hw_qtd_next = qh->hw_alt_next = EHCI_LIST_END;
>>        }

Slightly different in 2.5.48 since it uses a dummy TD (so certain race
conditions can't happen), and it won't line-wrap that qh_update() call.


>>        since list_splice() the qtd_list is diposed of its belongings and
>>        immediately in the next line we rely on qtd_list->next to point
>>        at an existing list_head.
>>
>>        i haven`t noticed any locking out there, and i`m afraid of what
>>        could result from a preemption happening between these two lines.
> 
> 
> Um, David, any thoughts about this?

That code runs under a spinlock_irqsave(&ehci->lock,...), so no
other task can preempt.  And list_splice() doesn't modify qtd_list,
so the qtd_list->next pointer stays valid ... it's like list_del(),
not list_del_init(), read <linux/list.h> to see.

- Dave