Re: [PATCH v1 1/2] NFSD: Delay adding new entries to LRU

[Chuck Lever <cel@kernel.org>]

On 8/20/25 6:47 PM, NeilBrown wrote:
> On Thu, 21 Aug 2025, Chuck Lever wrote:
>> From: Chuck Lever <chuck.lever@oracle.com>
>>
>> Neil Brown observes:
>>> I would not include RC_INPROG entries in the lru at all - they are
>>> always ignored, and will be added when they are switched to
>>> RCU_DONE.
>>
>> I also removed a stale comment.
>>
>> Suggested-by: NeilBrown <neil@brown.name>
>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>> ---
>>  fs/nfsd/nfscache.c | 6 ------
>>  1 file changed, 6 deletions(-)
>>
>> diff --git a/fs/nfsd/nfscache.c b/fs/nfsd/nfscache.c
>> index ba9d326b3de6..6c06cf24b5c7 100644
>> --- a/fs/nfsd/nfscache.c
>> +++ b/fs/nfsd/nfscache.c
>> @@ -237,10 +237,6 @@ void nfsd_reply_cache_shutdown(struct nfsd_net *nn)
>>  
>>  }
>>  
>> -/*
>> - * Move cache entry to end of LRU list, and queue the cleaner to run if it's
>> - * not already scheduled.
>> - */
>>  static void
>>  lru_put_end(struct nfsd_drc_bucket *b, struct nfsd_cacherep *rp)
>>  {
>> @@ -453,8 +449,6 @@ nfsd_cache_insert(struct nfsd_drc_bucket *b, struct nfsd_cacherep *key,
>>  				nn->longest_chain_cachesize,
>>  				atomic_read(&nn->num_drc_entries));
>>  	}
>> -
>> -	lru_put_end(b, ret);
>>  	return ret;
> 
> A result of this change is that entries in the lru never have
>     ->c_state == RC_INPROG
> They are always RC_DONE.
> 
> So this can be added to the patch:
> 
> --- a/fs/nfsd/nfscache.c
> +++ b/fs/nfsd/nfscache.c
> @@ -272,12 +272,6 @@ nfsd_prune_bucket_locked(struct nfsd_net *nn, struct nfsd_drc_bucket *b,
>  
>  	/* The bucket LRU is ordered oldest-first. */
>  	list_for_each_entry_safe(rp, tmp, &b->lru_head, c_lru) {
> -		/*
> -		 * Don't free entries attached to calls that are still
> -		 * in-progress, but do keep scanning the list.
> -		 */
> -		if (rp->c_state == RC_INPROG)
> -			continue;
>  
>  		if (atomic_read(&nn->num_drc_entries) <= nn->max_drc_entries &&
>  		    time_before(expiry, rp->c_timestamp))

I didn't add this bit because I couldn't convince myself that it was
impossible for an entry in RC_INPROG state to get into the LRU
somehow. But if you think it is not possible, I'll buy that and add this
snippet.


> With that added:
> 
> Reviewed-by: NeilBrown <neil@brown.name>
> 
> Thanks,
> NeilBrown
> 
> 
>>  }
>>  
>> -- 
>> 2.50.0
>>
>>
> 


-- 
Chuck Lever