Messages in this thread Patch in this message |  | | | Date | Wed, 2 Feb 2005 16:20:23 -0800 | | From | "David S. Miller" <> | | Subject | Re: [PATCH] arp_queue: serializing unlink + kfree_skb |
| |
On Mon, 31 Jan 2005 22:33:26 +1100 Herbert Xu <herbert@gondor.apana.org.au> 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 herbert@gondor.apana.org.au # [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 <davem@davemloft.net> # # include/linux/skbuff.h # 2005/02/02 15:51:57-08:00 herbert@gondor.apana.org.au +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 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/
|  |