Messages in this thread | | | Date | Mon, 4 Oct 1999 00:34:15 +0200 (CEST) | From | Andrea Arcangeli <> | Subject | Re: Network-related Oopses on 2.2.13pre14 |
| |
On 4 Oct 1999, Andi Kleen wrote:
>I don't expect it to fix any problems. Interrupts never remove packets >from the backlog, only add them. dev_clear_backlog is protected against other
The fact that it's only adding elements is just too much to be safe.
We are walking a list that can change under us.
If you want to make the algorithm SMP safe then you should insert the proper mb()/wmb()/rmb() calls both in dev_clear_backlog and in skb_queue_tail.
Suppose you have this queue:
--> head -> elem1 -> elem2 -- | | -----------------------------
Suppose you want to add the newsk to the end of queue as netif_rx does.
Suppose dev_clear_backlog is walking the queue on the other CPU while it see the memory this way:
head -> elem -> elem -> newsk -> NULL
As you are not doing ordered read in dev_clear_backlog and ordered writes in skb_queue_tail you have no garantee you won't find the list corrupted. Actually skb_queue_tail don't even enforce ordering at the assembler level.
The effect of skb_queue_tail() could be reordered this way due memory speculative operations in SMP.
This is the core of the stock skb_queue_tail.
{ struct sk_buff *prev, *next;
newsk->list = list; list->qlen++; next = (struct sk_buff *)list; prev = next->prev; newsk->next = next; newsk->prev = prev; next->prev = newsk; prev->next = newsk; }
It may be easily reodered or at the assembler level or at the CPU level this way:
extern __inline__ void __skb_queue_tail(struct sk_buff_head *list, struct sk_buf { struct sk_buff *prev, *next;
newsk->list = list; list->qlen++; next = (struct sk_buff *)list; prev = next->prev; next->prev = newsk; prev->next = newsk;
/* HERE dev_clear_backlog is reading the last entry of the list that is newsk and then he is reading newsk->next before we have a chance to update newsk->next to fix the queue and so it will Oops (if newsk was not weird, if it was weird you'll have some more fun). */
newsk->next = next; newsk->prev = prev; }
Andrea
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |