On Wed, Feb 03, 2010 at 05:31:31PM +1100, NeilBrown wrote:
> 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.
Looks right to me. Thanks, applied.
By the way, if we get sunrpc_cache_update(old, new1) and
sunrpc_cache_update(old, new2) simultaneously, what happens?
More generally: should we try to ensure that a cache never contains two
entries which match the same key?
--b.
>
> Signed-off-by: NeilBrown <neilb@xxxxxxx>
> ---
> net/sunrpc/cache.c | 17 ++++++++++++++---
> 1 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> index 9826c5c..3e1ef8b 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 */
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
