[NET] Add barriers for dst refcnt

[NET] Add barriers for dst refcnt

[Herbert Xu]

[-- Attachment #1: Type: text/plain, Size: 525 bytes --]

Hi Dave:

In light of the recent discussion about sk_buff, I think we need
the following patch for dst_entry.  This adds a memory barrier
before dst_release drops the refcnt, and a read memory barrier
before dst_destroy starts destroying the entry.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 589 bytes --]

===== include/net/dst.h 1.24 vs edited =====
--- 1.24/include/net/dst.h	2004-10-26 09:10:25 +10:00
+++ edited/include/net/dst.h	2005-02-04 18:24:46 +11:00
@@ -147,6 +147,7 @@
 {
 	if (dst) {
 		WARN_ON(atomic_read(&dst->__refcnt) < 1);
+		smp_mb__before_atomic_dec();
 		atomic_dec(&dst->__refcnt);
 	}
 }
===== net/core/dst.c 1.25 vs edited =====
--- 1.25/net/core/dst.c	2005-01-14 15:41:04 +11:00
+++ edited/net/core/dst.c	2005-02-04 18:26:06 +11:00
@@ -169,6 +169,8 @@
 	struct neighbour *neigh;
 	struct hh_cache *hh;
 
+	smp_rmb();
+
 again:
 	neigh = dst->neighbour;
 	hh = dst->hh;

Re: [NET] Add barriers for dst refcnt

[David S. Miller]

On Fri, 4 Feb 2005 18:33:11 +1100
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> In light of the recent discussion about sk_buff, I think we need
> the following patch for dst_entry.  This adds a memory barrier
> before dst_release drops the refcnt, and a read memory barrier
> before dst_destroy starts destroying the entry.
> 
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Good catch.

I "think" the dst_release() case is theoretical.  dst_release()
runs in a locked context for the thing referencing 'dst'.  Be it
the route hash, a socket, whatever.

BTW, looking at dst_release() call sites, xchg() might be another
primitive we need to watch out for memory barrier semantics of.
Since it returns a value, it would seem to need to be strictly
ordered wrt. other memory operations.  And sure enough, ppc64
has the memory barriers there.

Man, something else to audit on sparc64 and document in atomic_ops.txt
:-)

Anyways, Herbert's patch is correct and I'll apply it.  Thanks.

Re: [NET] Add barriers for dst refcnt

[Herbert Xu]

On Sat, Feb 05, 2005 at 05:21:01PM -0800, David S. Miller wrote:
> 
> I "think" the dst_release() case is theoretical.  dst_release()
> runs in a locked context for the thing referencing 'dst'.  Be it
> the route hash, a socket, whatever.

Unless dst_run_gc takes the same lock as the context doing the
dst_release you'll still be in trouble :)
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt