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

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

net/sunrpc/cache.c

@@ -321,12 +321,10 @@ static int cache_clean(void)
 			if (test_and_clear_bit(CACHE_PENDING, &ch->flags))
 				queue_loose(current_detail, ch);
 
-			if (!atomic_read(&ch->refcnt))
+			if (atomic_read(&ch->refcnt) == 1)
 				break;
 		}
 		if (ch) {
-			cache_get(ch);
-			clear_bit(CACHE_HASHED, &ch->flags);
 			*cp = ch->next;
 			ch->next = NULL;
 			current_detail->entries--;

net/sunrpc/svcauth.c

@@ -178,12 +178,12 @@ auth_domain_lookup(struct auth_domain *item, int set)
 		tmp = container_of(*hp, struct auth_domain, h);
 		if (!auth_domain_match(tmp, item))
 			continue;
-		cache_get(&tmp->h);
-		if (!set)
+		if (!set) {
+			cache_get(&tmp->h);
 			goto out_noset;
+		}
 		*hp = tmp->h.next;
 		tmp->h.next = NULL;
-		clear_bit(CACHE_HASHED, &tmp->h.flags);
 		auth_domain_drop(&tmp->h, &auth_domain_cache);
 		goto out_set;
 	}
@@ -192,9 +192,9 @@ auth_domain_lookup(struct auth_domain *item, int set)
 		goto out_nada;
 	auth_domain_cache.entries++;
 out_set:
-	set_bit(CACHE_HASHED, &item->h.flags);
 	item->h.next = *head;
 	*head = &item->h;
+	cache_get(&item->h);
 	write_unlock(&auth_domain_cache.hash_lock);
 	cache_fresh(&auth_domain_cache, &item->h, item->h.expiry_time);
 	cache_get(&item->h);