[PATCH] nfsd: discard CACHE_HASHED flag, keeping information in refcount instead.

This patch should fix a problem that has been experienced on at-least one
busy NFS server, but it has not had lots of testing yet.  If -mm could provide
that .....

The rpc auth cache currently differentiates between a reference due to
being in a hash chain (signalled by CACHE_HASHED flag) and any other
reference (counted in refcnt).

This is an artificial difference due to an historical accident, and it
makes cache_put unsafe.

This patch removes the distinction so now existance in a hash chain is
counted just like any other reference.  Thus a race window in cache_put is
closed.

Signed-off-by: Neil Brown <neilb@cse.unsw.edu.au>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

1 files changed, 5 insertions, 14 deletions

diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
index b902425d2be5..6864063d1b9f 100644
--- a/include/linux/sunrpc/cache.h
+++ b/include/linux/sunrpc/cache.h
@@ -37,8 +37,7 @@
  * Entries have a ref count and a 'hashed' flag which counts the existance
  * in the hash table.
  * We only expire entries when refcount is zero.
- * Existance in the cache is not measured in refcount but rather in
- * CACHE_HASHED flag.
+ * Existance in the cache is counted  the refcount.
  */
 
 /* Every cache item has a common header that is used
@@ -57,7 +56,6 @@ struct cache_head {
 #define	CACHE_VALID	0	/* Entry contains valid data */
 #define	CACHE_NEGATIVE	1	/* Negative entry - there is no match for the key */
 #define	CACHE_PENDING	2	/* An upcall has been sent but no reply received yet*/
-#define	CACHE_HASHED	3	/* Entry is in a hash table */
 
 #define	CACHE_NEW_EXPIRY 120	/* keep new things pending confirmation for 120 seconds */
 
@@ -185,7 +183,6 @@ RTN *FNAME ARGS										\
 											\
 			if (new)							\
 				{INIT;}							\
-			cache_get(&tmp->MEMBER);					\
 			if (set) {							\
 				if (!INPLACE && test_bit(CACHE_VALID, &tmp->MEMBER.flags))\
 				{ /* need to swap in new */				\
@@ -194,8 +191,6 @@ RTN *FNAME ARGS										\
 					new->MEMBER.next = tmp->MEMBER.next;		\
 					*hp = &new->MEMBER;				\
 					tmp->MEMBER.next = NULL;			\
-					set_bit(CACHE_HASHED, &new->MEMBER.flags);	\
-					clear_bit(CACHE_HASHED, &tmp->MEMBER.flags);	\
 					t2 = tmp; tmp = new; new = t2;			\
 				}							\
 				if (test_bit(CACHE_NEGATIVE,  &item->MEMBER.flags))	\
@@ -205,6 +200,7 @@ RTN *FNAME ARGS										\
 					clear_bit(CACHE_NEGATIVE, &tmp->MEMBER.flags);	\
 				}							\
 			}								\
+			cache_get(&tmp->MEMBER);					\
 			if (set||new) write_unlock(&(DETAIL)->hash_lock);		\
 			else read_unlock(&(DETAIL)->hash_lock);				\
 			if (set)							\
@@ -220,7 +216,7 @@ RTN *FNAME ARGS										\
 		new->MEMBER.next = *head;						\
 		*head = &new->MEMBER;							\
 		(DETAIL)->entries ++;							\
-		set_bit(CACHE_HASHED, &new->MEMBER.flags);				\
+		cache_get(&new->MEMBER);						\
 		if (set) {								\
 			tmp = new;							\
 			if (test_bit(CACHE_NEGATIVE, &item->MEMBER.flags))		\
@@ -268,15 +264,10 @@ static inline struct cache_head  *cache_get(struct cache_head *h)
 
 static inline int cache_put(struct cache_head *h, struct cache_detail *cd)
 {
-	atomic_dec(&h->refcnt);
-	if (!atomic_read(&h->refcnt) &&
+	if (atomic_read(&h->refcnt) <= 2 &&
 	    h->expiry_time < cd->nextcheck)
 		cd->nextcheck = h->expiry_time;
-	if (!test_bit(CACHE_HASHED, &h->flags) &&
-	    !atomic_read(&h->refcnt))
-		return 1;
-
-	return 0;
+	return atomic_dec_and_test(&h->refcnt);
 }
 
 extern void cache_init(struct cache_head *h);