From: NeilBrown <neilb@cse.unsw.edu.au>
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>
---
25-akpm/include/linux/sunrpc/cache.h | 19 +++++--------------
25-akpm/net/sunrpc/cache.c | 4 +---
25-akpm/net/sunrpc/svcauth.c | 8 ++++----
3 files changed, 10 insertions(+), 21 deletions(-)
diff -puN include/linux/sunrpc/cache.h~nfsd-discard-cache_hashed-flag-keeping-information-in-refcount-instead include/linux/sunrpc/cache.h
--- 25/include/linux/sunrpc/cache.h~nfsd-discard-cache_hashed-flag-keeping-information-in-refcount-instead 2005-02-17 18:08:43.000000000 -0800
+++ 25-akpm/include/linux/sunrpc/cache.h 2005-02-17 18:08:43.000000000 -0800
@@ -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_
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);
diff -puN net/sunrpc/cache.c~nfsd-discard-cache_hashed-flag-keeping-information-in-refcount-instead net/sunrpc/cache.c
--- 25/net/sunrpc/cache.c~nfsd-discard-cache_hashed-flag-keeping-information-in-refcount-instead 2005-02-17 18:08:43.000000000 -0800
+++ 25-akpm/net/sunrpc/cache.c 2005-02-17 18:08:43.000000000 -0800
@@ -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--;
diff -puN net/sunrpc/svcauth.c~nfsd-discard-cache_hashed-flag-keeping-information-in-refcount-instead net/sunrpc/svcauth.c
--- 25/net/sunrpc/svcauth.c~nfsd-discard-cache_hashed-flag-keeping-information-in-refcount-instead 2005-02-17 18:08:43.000000000 -0800
+++ 25-akpm/net/sunrpc/svcauth.c 2005-02-17 18:08:43.000000000 -0800
@@ -178,12 +178,12 @@ auth_domain_lookup(struct auth_domain *i
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 *i
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);
_