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