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: 637 bytes --]

Herbert Xu wrote:

>pravin b shelar <pravins@calsoftinc.com> wrote:
>  
>
>>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.
>>    
>>
>
>Sorry I overlooked your patch.  Otherwise we could have solved this
>one day earlier :)
>  
>
ah ... I was just waiting :)

>  
>
>>Please comment.
>>    
>>
>
>There is just one problem.  We need an extra barrier before or after
>the goto since we no longer have the implicit barrier given by
>atomic_dec_and_test.
>  
>
I am attaching updated patch with barrier.

Regards,
Pravin.



[-- Attachment #2: dst-atomic_dec_and_test-removed.patch --]
[-- Type: text/x-patch, Size: 1176 bytes --]

This patch removes atomic_dec_and_test from dst_destroy, used to test child dst __refcnt.

Signed-off by: Pravin B. Shelar <pravins@calsoftinc.com>

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-07 04:53:28.000000000 -0700
@@ -198,17 +198,21 @@
 
 	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);
+			smp_mb__after_atomic_dec();
+
+			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;
 }