Re: Fwd: atomic_dec_and_test for child dst needed in dst_destroy?

[pravin b shelar <pravins@calsoftinc.com>]

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


>  
>
>This is racy (albeit very unlikely) because dst may be freed by
>dst_run_gc after the atomic_dec.
>
>When we are not the real parent of the dst (e.g., when we're xfrm_dst
>and the child is an rtentry), it may already be on the GC list.
>
>In fact the current code is buggy to, we need to check dst->flags
>before the dec as dst may no longer be valid afterwards.
>
>
>  
>
This patch  will remove the need for atomic_dec_and_test for this 
particular case.
Now we can break down the atomic_dec_and_test to atomic_dec & atomic_read.

Please comment.

Regards
Pravin.



[-- Attachment #2: dst-atomic --]
[-- Type: text/plain, Size: 991 bytes --]

Index: linux-2.6.11-dsta/net/core/dst.c
===================================================================
--- linux-2.6.11-dsta.orig/net/core/dst.c	2005-04-06 00:18:32.000000000 -0700
+++ linux-2.6.11-dsta/net/core/dst.c	2005-04-06 01:31:10.000000000 -0700
@@ -198,17 +198,19 @@
 
 	dst = child;
 	if (dst) {
-		if (atomic_dec_and_test(&dst->__refcnt)) {
-			/* We were real parent of this dst, so kill child. */
-			if (dst->flags&DST_NOHASH)
-				goto again;
-		} else {
-			/* Child is still referenced, return it for freeing. */
-			if (dst->flags&DST_NOHASH)
-				return dst;
-			/* Child is still in his hash table */
+		if (dst->flags&DST_NOHASH) {
+			atomic_dec(&dst->__refcnt);
+			if (atomic_read(&dst->__refcnt))
+				/* Child is still referenced, return it for freeing. */
+				return dst;	
+
+			/*We were real parent of this dst, so kill child. */
+			goto again;
 		}
+		else /* Child is still in his hash table */
+			atomic_dec(&dst->__refcnt);
 	}
+
 	return NULL;
 }