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

Chuck Lever
2025-02-07T14:43:32Z

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