Re: [PATCH v4 3/4] nfsd: implement server-stats-get netlink handler
Jeff Layton <[email protected]>
| Newsgroups | gmane.linux.kernel,gmane.linux.nfs |
|---|---|
| Message-ID | <[email protected]> |
On Wed, 2026-06-17 at 09:18 -0400, Chuck Lever wrote: > > 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. > Ok. I'll plan to add it to the interface and send it in v5. nfs-utils will need a follow-on patch to display the value. No promises on when I'll do that, but it shouldn't be too hard. -- Jeff Layton <[email protected]>