From: NeilBrown 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 Signed-off-by: Andrew Morton --- 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); _