Re: [PATCH v4 3/4] nfsd: implement server-stats-get netlink handler

"Chuck Lever" <[email protected]>
Newsgroups gmane.linux.nfs,gmane.linux.kernel
Message-ID <[email protected]>

On Wed, Jun 17, 2026, at 7:07 AM, Jeff Layton wrote:
> On Tue, 2026-06-16 at 20:57 -0400, Chuck Lever wrote:
>> Hey Jeff -
>> 
>> On Tue, Jun 16, 2026, at 8:45 AM, Jeff Layton wrote:
>> > Implement nfsd_nl_server_stats_get_dumpit() which exposes the
>> > NFS server statistics currently available via /proc/net/rpc/nfsd
>> > through the nfsd generic netlink family.
>> > 
>> > The handler uses a dump operation to stream statistics across
>> > multiple netlink messages:
>> > 
>> >   - First message: all scalar stats (reply cache, filehandle,
>> >     IO, network, RPC) plus per-version procedure counts
>> >     (proc2/3/4-ops) using per-netns vs_count arrays.
>> > 
>> >   - Subsequent messages: NFSv4 per-operation counts
>> >     (proc4ops-ops), one entry per message, using cb->args[0]
>> >     to track the current operation index across dump calls.
>> > 
>> > This allows nfsstat to retrieve server statistics via netlink
>> > with a procfs fallback for older kernels.
>> > 
>> > Assisted-by: Claude:claude-opus-4-6
>> > Signed-off-by: Jeff Layton <[email protected]>
>> > ---
>> >  Documentation/netlink/specs/nfsd.yaml | 105 +++++++++++++++++
>> >  fs/nfsd/netlink.c                     |   5 +
>> >  fs/nfsd/netlink.h                     |   2 +
>> >  fs/nfsd/nfsctl.c                      | 206 ++++++++++++++++++++++++++++++++++
>> >  include/uapi/linux/nfsd_netlink.h     |  35 ++++++
>> >  5 files changed, 353 insertions(+)
>> 
>> The procfs output in nfsd_show() emits one more counter that this dump
>> handler drops: wdeleg_getattr (NFSD_STATS_WDELEG_GETATTR), printed at
>> the end of the CONFIG_NFSD_V4 block in fs/nfsd/stats.c, right after
>> proc4ops. The netlink dump stops after proc4ops-ops and sets
>> cb->args[0] = -1, so the write-delegation GETATTR-conflict counter never
>> goes out on the wire.
>> 
>> There's no schema slot for it either: the server-stats attribute-set
>> ends at proc4ops-ops, so a consumer that prefers netlink and only falls
>> back to procfs on an old kernel loses this counter on every kernel new
>> enough to support server-stats-get.
>> 
>> I Suggest:
>> 
>> - add a wdeleg-getattr (u64) attribute to the server-stats
>>   attribute-set in nfsd.yaml and list it under the dump reply
>>   attributes;
>> 
>> - add NFSD_A_SERVER_STATS_WDELEG_GETATTR to the uapi enum, after
>>   PROC4OPS_OPS and before __NFSD_A_SERVER_STATS_MAX so existing
>>   values don't shift;
>> 
>> - emit it as a single scalar in the start == 0 message, wrapped in
>>   CONFIG_NFSD_V4 to match the procfs side.
>> 
>
> This omission was intentional. The stated purpose of this patchset was
> to convert nfsstat to use netlink. nfsstat does not access or display
> this field today. Without something that consumes this stat, I don't
> see the point of adding it here. We can always add it later when/if it
> becomes useful to userland.

If we were adding a greenfield kernel-userspace API, I would agree.
However, this API is meant to be compatible with the existing
procfs API, so it should match to prevent a regression (I'm not
aware of a consumer, this is only a general guideline).

For future reference: since a missing feature cannot be expressed
in a patch's diff body, your intention to exclude this field could
be mentioned in the commit message.


> Is there something that accesses this today that I'm not aware of? If
> not, is there some future intent to add this to nfsstat?

I think nfsstat should display the wdeleg-getattr field. It has simply
been forgotten.


-- 
Chuck Lever
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.