On Tue, 2025-02-18 at 10:39 -0500, cel@xxxxxxxxxx wrote:
> From: Chuck Lever <chuck.lever@xxxxxxxxxx>
>
> 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@xxxxxxxxxxxxx>
> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
