netdev
[Top] [All Lists]

Re: [PATCH] arp_queue: serializing unlink + kfree_skb

To: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH] arp_queue: serializing unlink + kfree_skb
From: "David S. Miller" <davem@xxxxxxxxxxxxx>
Date: Thu, 3 Feb 2005 17:23:57 -0800
Cc: anton@xxxxxxxxx, okir@xxxxxxx, netdev@xxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx
In-reply-to: <20050204012053.GA8949@xxxxxxxxxxxxxxxxxxx>
References: <20050131102920.GC4170@xxxxxxx> <E1CvZo6-0001Bz-00@xxxxxxxxxxxxxxxxxxxxxxxx> <20050203142705.GA11318@xxxxxxxxxxxxxxxxxxxxxxxxxx> <20050203203010.GA7081@xxxxxxxxxxxxxxxxxxx> <20050203141901.5ce04c92.davem@xxxxxxxxxxxxx> <20050203235044.GA8422@xxxxxxxxxxxxxxxxxxx> <20050203164922.2627a112.davem@xxxxxxxxxxxxx> <20050204012053.GA8949@xxxxxxxxxxxxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
On Fri, 4 Feb 2005 12:20:53 +1100
Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote:

> This is true if CPU 0 reads the count before reading skb->list.
> Without a memory barrier, atomic_read and reading skb->list can
> be reordered.  Put it another way, reading skb->list could return
> a cached value that was read from the main memory prior to the
> atomic_read.
> 
> So in order for CPU 0 to always see an up-to-date value of skb->list,
> it needs to do an smp_rmb() between the atomic_read and reading
> skb->list.

You're absolutely right.  Ok, so we do need to change kfree_skb().
I believe even with the memory barrier, the atomic_read() optimization
is still worth it.  atomic ops on sparc64 take a minimum of 40 some odd
cycles on UltraSPARC-III and later, whereas the memory barrier will
take up a single cycle most of the time.

So it'll look something like:

        if (atomic_read(&skb->users) == 1)
                smb_rmb();
        else if (!atomic_dec_and_test(&skb->users))
                return;
        __kfree_skb(skb);

<Prev in Thread] Current Thread [Next in Thread>