From 3af4974eb2c7867d6e160977195dfde586d0e564 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 3 Feb 2010 17:31:31 +1100 Subject: sunrpc: don't keep expired entries in the auth caches. currently expired entries remain in the auth caches as long as there is a reference. This was needed long ago when the auth_domain cache used the same cache infrastructure. But since that (being a very different sort of cache) was separated, this test is no longer needed. So remove the test on refcnt and tidy up the surrounding code. This allows the cache_dequeue call (which needed to be there to drop a potentially awkward reference) can be moved outside of the spinlock which is a better place for it. Signed-off-by: NeilBrown Signed-off-by: J. Bruce Fields --- net/sunrpc/cache.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) (limited to 'net/sunrpc/cache.c') diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c index 39bddba53ba1..83592e012585 100644 --- a/net/sunrpc/cache.c +++ b/net/sunrpc/cache.c @@ -397,31 +397,28 @@ static int cache_clean(void) /* Ok, now to clean this strand */ cp = & current_detail->hash_table[current_index]; - ch = *cp; - for (; ch; cp= & ch->next, ch= *cp) { + for (ch = *cp ; ch ; cp = & ch->next, ch = *cp) { if (current_detail->nextcheck > ch->expiry_time) current_detail->nextcheck = ch->expiry_time+1; if (ch->expiry_time >= get_seconds() && ch->last_refresh >= current_detail->flush_time) continue; - if (test_and_clear_bit(CACHE_PENDING, &ch->flags)) - cache_dequeue(current_detail, ch); - if (atomic_read(&ch->ref.refcount) == 1) - break; - } - if (ch) { *cp = ch->next; ch->next = NULL; current_detail->entries--; rv = 1; + break; } + write_unlock(¤t_detail->hash_lock); d = current_detail; if (!ch) current_index ++; spin_unlock(&cache_list_lock); if (ch) { + if (test_and_clear_bit(CACHE_PENDING, &ch->flags)) + cache_dequeue(current_detail, ch); cache_revisit_request(ch); cache_put(ch, d); } -- cgit v1.2.3 From 2f50d8b63dd6e5320a9d223298df19df3502da29 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 3 Feb 2010 17:31:31 +1100 Subject: sunrpc/cache: factor out cache_is_expired This removes a tiny bit of code duplication, but more important prepares for following patch which will perform the expiry check in cache_lookup and the rest of the validity check in cache_check. Signed-off-by: NeilBrown Signed-off-by: J. Bruce Fields --- net/sunrpc/cache.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'net/sunrpc/cache.c') diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c index 83592e012585..9826c5ceb995 100644 --- a/net/sunrpc/cache.c +++ b/net/sunrpc/cache.c @@ -49,6 +49,12 @@ static void cache_init(struct cache_head *h) h->last_refresh = now; } +static inline int cache_is_expired(struct cache_detail *detail, struct cache_head *h) +{ + return (h->expiry_time < get_seconds()) || + (detail->flush_time > h->last_refresh); +} + struct cache_head *sunrpc_cache_lookup(struct cache_detail *detail, struct cache_head *key, int hash) { @@ -184,9 +190,7 @@ static int cache_make_upcall(struct cache_detail *cd, struct cache_head *h) static inline int cache_is_valid(struct cache_detail *detail, struct cache_head *h) { if (!test_bit(CACHE_VALID, &h->flags) || - h->expiry_time < get_seconds()) - return -EAGAIN; - else if (detail->flush_time > h->last_refresh) + cache_is_expired(detail, h)) return -EAGAIN; else { /* entry is valid */ @@ -400,8 +404,7 @@ static int cache_clean(void) for (ch = *cp ; ch ; cp = & ch->next, ch = *cp) { if (current_detail->nextcheck > ch->expiry_time) current_detail->nextcheck = ch->expiry_time+1; - if (ch->expiry_time >= get_seconds() && - ch->last_refresh >= current_detail->flush_time) + if (!cache_is_expired(current_detail, ch)) continue; *cp = ch->next; -- cgit v1.2.3 From d202cce8963d9268ff355a386e20243e8332b308 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Wed, 3 Feb 2010 17:31:31 +1100 Subject: sunrpc: never return expired entries in sunrpc_cache_lookup If sunrpc_cache_lookup finds an expired entry, remove it from the cache and return a freshly created non-VALID entry instead. This ensures that we only ever get a usable entry, or an entry that will become usable once an update arrives. i.e. we will never need to repeat the lookup. This allows us to remove the 'is_expired' test from cache_check (i.e. from cache_is_valid). cache_check should never get an expired entry as 'lookup' will never return one. If it does happen - due to inconvenient timing - then just accept it as still valid, it won't be very much past it's use-by date. Signed-off-by: NeilBrown Signed-off-by: J. Bruce Fields --- net/sunrpc/cache.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) (limited to 'net/sunrpc/cache.c') diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c index 9826c5ceb995..3e1ef8bf4dc2 100644 --- a/net/sunrpc/cache.c +++ b/net/sunrpc/cache.c @@ -59,7 +59,7 @@ struct cache_head *sunrpc_cache_lookup(struct cache_detail *detail, struct cache_head *key, int hash) { struct cache_head **head, **hp; - struct cache_head *new = NULL; + struct cache_head *new = NULL, *freeme = NULL; head = &detail->hash_table[hash]; @@ -68,6 +68,9 @@ struct cache_head *sunrpc_cache_lookup(struct cache_detail *detail, for (hp=head; *hp != NULL ; hp = &(*hp)->next) { struct cache_head *tmp = *hp; if (detail->match(tmp, key)) { + if (cache_is_expired(detail, tmp)) + /* This entry is expired, we will discard it. */ + break; cache_get(tmp); read_unlock(&detail->hash_lock); return tmp; @@ -92,6 +95,13 @@ struct cache_head *sunrpc_cache_lookup(struct cache_detail *detail, for (hp=head; *hp != NULL ; hp = &(*hp)->next) { struct cache_head *tmp = *hp; if (detail->match(tmp, key)) { + if (cache_is_expired(detail, tmp)) { + *hp = tmp->next; + tmp->next = NULL; + detail->entries --; + freeme = tmp; + break; + } cache_get(tmp); write_unlock(&detail->hash_lock); cache_put(new, detail); @@ -104,6 +114,8 @@ struct cache_head *sunrpc_cache_lookup(struct cache_detail *detail, cache_get(new); write_unlock(&detail->hash_lock); + if (freeme) + cache_put(freeme, detail); return new; } EXPORT_SYMBOL_GPL(sunrpc_cache_lookup); @@ -189,8 +201,7 @@ static int cache_make_upcall(struct cache_detail *cd, struct cache_head *h) static inline int cache_is_valid(struct cache_detail *detail, struct cache_head *h) { - if (!test_bit(CACHE_VALID, &h->flags) || - cache_is_expired(detail, h)) + if (!test_bit(CACHE_VALID, &h->flags)) return -EAGAIN; else { /* entry is valid */ -- cgit v1.2.3 From a5990ea1254cd186b38744507aeec3136a0c1c95 Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Thu, 11 Mar 2010 14:08:10 -0800 Subject: sunrpc/cache: fix module refcnt leak in a failure path Don't forget to release the module refcnt if seq_open() returns failure. Signed-off-by: Li Zefan Cc: J. Bruce Fields Cc: Neil Brown Cc: Trond Myklebust Signed-off-by: Andrew Morton Signed-off-by: J. Bruce Fields --- net/sunrpc/cache.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'net/sunrpc/cache.c') diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c index 3e1ef8bf4dc2..a3f340c8b79a 100644 --- a/net/sunrpc/cache.c +++ b/net/sunrpc/cache.c @@ -1244,8 +1244,10 @@ static int content_open(struct inode *inode, struct file *file, if (!cd || !try_module_get(cd->owner)) return -EACCES; han = __seq_open_private(file, &cache_content_op, sizeof(*han)); - if (han == NULL) + if (han == NULL) { + module_put(cd->owner); return -ENOMEM; + } han->cd = cd; return 0; -- cgit v1.2.3