Re: [PATCH v2 2/7] NFSD: Re-organize nfsd_file_gc_worker()

[Jeff Layton <jlayton@kernel.org>]

On Tue, 2025-02-18 at 10:39 -0500, cel@kernel.org wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> Dave opines:
> 
> IMO, there is no need to do this unnecessary work on every object
> that is added to the LRU.  Changing the gc worker to always run
> every 2s and check if it has work to do like so:
> 
>  static void
>  nfsd_file_gc_worker(struct work_struct *work)
>  {
> -	nfsd_file_gc();
> -	if (list_lru_count(&nfsd_file_lru))
> -		nfsd_file_schedule_laundrette();
> +	if (list_lru_count(&nfsd_file_lru))
> +		nfsd_file_gc();
> +	nfsd_file_schedule_laundrette();
>  }
> 
> means that nfsd_file_gc() will be run the same way and have the same
> behaviour as the current code. When the system it idle, it does a
> list_lru_count() check every 2 seconds and goes back to sleep.
> That's going to be pretty much unnoticable on most machines that run
> NFS servers.
> 
> Suggested-by: Dave Chinner <david@fromorbit.com>
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>  fs/nfsd/filecache.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
> index 909b5bc72bd3..2933cba1e5f4 100644
> --- a/fs/nfsd/filecache.c
> +++ b/fs/nfsd/filecache.c
> @@ -549,9 +549,9 @@ nfsd_file_gc(void)
>  static void
>  nfsd_file_gc_worker(struct work_struct *work)
>  {
> -	nfsd_file_gc();
> +	nfsd_file_schedule_laundrette();
>  	if (list_lru_count(&nfsd_file_lru))
> -		nfsd_file_schedule_laundrette();
> +		nfsd_file_gc();
>  }
>  
>  static unsigned long

Given that it's a delayed workqueue job, it probably doesn't matter,
but why schedule the laundrette before doing the nfsd_file_gc() call?
-- 
Jeff Layton <jlayton@kernel.org>