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

[pravin b shelar <pravins@xxxxxxxxxxxxxx>]

>  
>
> 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.


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;
 }