Re: [PATCH 3/6] nfsd: filecache: use list_lru_walk_node() in nfsd_file_gc()

[Chuck Lever <chuck.lever@oracle.com>]

On 2/7/25 12:15 AM, NeilBrown wrote:
> list_lru_walk() is only useful when the aim is to remove all elements
> from the list_lru.

I think I get where this is going. Can the description cite some API
documentation that comports with this claim?


> It will repeated visit rotated element of the first
> per-node sublist before proceeding to subsrequent sublists.

s/repeated/repeatedly, s/element/elements, and s/subsrequent/subsequent.


> This patch changes to use list_lru_walk_node() and list_lru_count_node()
> on each individual node.
> 
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
>  fs/nfsd/filecache.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
> index 7dc20143c854..04588c03bdfe 100644
> --- a/fs/nfsd/filecache.c
> +++ b/fs/nfsd/filecache.c
> @@ -532,10 +532,14 @@ static void
>  nfsd_file_gc(void)
>  {
>  	LIST_HEAD(dispose);
> -	unsigned long ret;
> +	unsigned long ret = 0;
> +	int nid;
>  
> -	ret = list_lru_walk(&nfsd_file_lru, nfsd_file_lru_cb,
> -			    &dispose, list_lru_count(&nfsd_file_lru));
> +	for_each_node_state(nid, N_NORMAL_MEMORY) {
> +		unsigned long nr = list_lru_count_node(&nfsd_file_lru, nid);
> +		ret += list_lru_walk_node(&nfsd_file_lru, nid, nfsd_file_lru_cb,
> +					  &dispose, &nr);
> +	}
>  	trace_nfsd_file_gc_removed(ret, list_lru_count(&nfsd_file_lru));

Nit: Maybe this trace point should be placed in the loop and changed to
record the nid as well?


>  	nfsd_file_dispose_list_delayed(&dispose);

Wondering if maybe the nfsd_file_dispose_list_delayed() call should also
be moved inside the for loop to break up the disposal processing per
node as well.


>  }


-- 
Chuck Lever