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/
