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

[Guoqing Jiang <guoqing.jiang@linux.dev>]

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