Re: [PATCH] nfsd: remove unnecessary cache_put from idmap_lookup

[Guoqing Jiang <guoqing.jiang@xxxxxxxxx>]

Hi Neil,

On 8/7/24 07:58, NeilBrown wrote:
> On Mon, 05 Aug 2024, Guoqing Jiang wrote:
> > It is not needed given cache_check already calls cache_put on failure,
> > otherwise we call cache_put twice in case of -ETIMEDOUT.
> cache_check() was called on a different *item.
>
> This cache_put() puts the *item returned by lookup_fn() two lines
> earlier.
>
> The current code is correct.

Thanks for point it out! So this cache_put is paired with lookup_fn which
calls cache_get on a new item.

BTW, I checked idmap_id_to_name which didn't put cache if there is no
enough buffer, not sure if we need it here.

diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
index 7a806ac13e31..7abddf7d8f6d 100644
--- a/fs/nfsd/nfs4idmap.c
+++ b/fs/nfsd/nfs4idmap.c
@@ -594,8 +594,10 @@ static __be32 idmap_id_to_name(struct xdr_stream *xdr,
        ret = strlen(item->name);
        WARN_ON_ONCE(ret > IDMAP_NAMESZ);
        p = xdr_reserve_space(xdr, ret + 4);
-       if (!p)
+       if (!p) {
+               cache_put(&item->h, nn->idtoname_cache);
                return nfserr_resource;
+       }
        p = xdr_encode_opaque(p, item->name, ret);
        cache_put(&item->h, nn->idtoname_cache);
        return 0;

Thanks,
Guoqing