Re: [2.4, 2.5, USB] locking issue

From: David Brownell (david-b@pacbell.net)
Date: Wed Nov 20 2002 - 10:40:48 EST


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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Sat Nov 23 2002 - 22:00:32 EST