On Mon, 31 Jan 2005 22:33:26 +1100 Herbert Xu <[EMAIL PROTECTED]> wrote:
> We're using atomic integers to signal that we're done with an object. > The object is usually represented by a piece of memory. > > The problem is that in most of the places where we do this (and that's > not just in the networking systems), there are no memory barriers between > the last reference to that object and the decrease on the atomic counter. I agree. > if (atomic_read(&skb->users) != 1) { > smp_mb__before_atomic_dec(); > if (!atomic_dec_and_test(&skb->users)) > return; > } > __kfree_skb(skb); This looks good. Olaf can you possibly ask the reproducer if this patch makes the ARP problem go away? # This is a BitKeeper generated diff -Nru style patch. # # ChangeSet # 2005/02/02 15:53:55-08:00 [EMAIL PROTECTED] # [NET]: Add missing memory barrier to SKB release. # # Here, we are using atomic counters to signal that we are done # with an object. The object is usually represented by a piece # of memory. # # The problem is that in most of the places we do this, there are # no memory barriers between the last reference to that object # and the decrease on the atomic counter. # # Based upon a race spotted in arp_queue handling by Olaf Kirch. # # Signed-off-by: David S. Miller <[EMAIL PROTECTED]> # # include/linux/skbuff.h # 2005/02/02 15:51:57-08:00 [EMAIL PROTECTED] +6 -2 # [NET]: Add missing memory barrier to SKB release. # diff -Nru a/include/linux/skbuff.h b/include/linux/skbuff.h --- a/include/linux/skbuff.h 2005-02-02 15:54:13 -08:00 +++ b/include/linux/skbuff.h 2005-02-02 15:54:13 -08:00 @@ -353,8 +353,12 @@ */ static inline void kfree_skb(struct sk_buff *skb) { - if (atomic_read(&skb->users) == 1 || atomic_dec_and_test(&skb->users)) - __kfree_skb(skb); + if (atomic_read(&skb->users) != 1) { + smp_mb__before_atomic_dec(); + if (!atomic_dec_and_test(&skb->users)) + return; + } + __kfree_skb(skb); } /* Use this if you didn't touch the skb state [for fast switching] */ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/