Thanks Bruce, for your time, attention, and comments! > On Sep 24, 2020, at 5:36 PM, J. Bruce Fields <bfields@xxxxxxxxxxxx> wrote: > > On Mon, Sep 21, 2020 at 02:10:49PM -0400, Chuck Lever wrote: >> As I've been working on various server bugs, I've been adding >> tracepoints that record NFS operation arguments. Here's an updated >> snapshot of this work for your review and comment. >> >> The idea here is to provide a degree of NFS traffic observability >> without needing network capture. Tracepoints are generally lighter- >> weight than full network capture, allowing effective capture-time >> data reduction: > > I do wonder when tracepoints seem to duplicate information you could get > from network traces, so thanks for taking the time to explain this. It > makes sense to me. > > The patches look fine. The only one I'm I'm on the fence about is the > last with the split up of the dispatch functions. I'll ask some > questions there.... To be clear to everyone, this series is still "preview". I expect more churn in these patches, thus I don't consider the series ready to be merged by any stretch. > --b. > >> >> - One or a handful of these can be enabled at a time >> - Each tracepoint records much less data per operation than capture >> - Extra capture-time filtering can reduce data amount even further >> - Some of these operations are infrequent enough that their >> tracepoint could be enabled persistently without a significant >> performance impact (for example, for security auditing) >> >> The topic branch has been updated as well: >> >> git://git.linux-nfs.org/projects/cel/cel-2.6.git nfsd-more-tracepoints >> >> >> Changes since RFC: >> * s/SPDK/SPDX and corrected the spelling of Christoph's surname >> * Fixed a build error noticed by <lkp@xxxxxxxxx> >> * Introduced generic headers for VFS and NFS protocol display macros >> * nfsd4_compoundstatus now displays NFS4ERR codes symbolically >> * The svc_process tracepoint now displays the RPC procedure symbolically >> * NFSD dispatcher now displays procedure names and status codes symbolically >> * fh_verify tracepoint tentatively included; it adds a lot of noise, but perhaps not much value >> * Cleaned up the remaining PROC() macros in the server code >> * Removed trace_printk's that were introduced during the RFC series >> * Removed redundant nfsd4_close tracepoint >> >> --- >> >> Chuck Lever (27): >> NFS: Move generic FS show macros to global header >> NFS: Move NFS protocol display macros to global header >> NFSD: Add SPDX header for fs/nfsd/trace.c >> SUNRPC: Move the svc_xdr_recvfrom() tracepoint >> SUNRPC: Add svc_xdr_authenticate tracepoint >> lockd: Replace PROC() macro with open code >> NFSACL: Replace PROC() macro with open code >> SUNRPC: Make trace_svc_process() display the RPC procedure symbolically >> NFSD: Clean up the show_nf_may macro >> NFSD: Remove extra "0x" in tracepoint format specifier >> NFSD: Constify @fh argument of knfsd_fh_hash() >> NFSD: Add tracepoint in nfsd_setattr() >> NFSD: Add tracepoint for nfsd_access() >> NFSD: nfsd_compound_status tracepoint should record XID >> NFSD: Add client ID lifetime tracepoints >> NFSD: Add tracepoints to report NFSv4 session state >> NFSD: Add a tracepoint to report the current filehandle >> NFSD: Add GETATTR tracepoint >> NFSD: Add tracepoint in nfsd4_stateid_preprocess() >> NFSD: Add tracepoint to report arguments to NFSv4 OPEN >> NFSD: Add a tracepoint for DELEGRETURN >> NFSD: Add a lookup tracepoint >> NFSD: Add lock and locku tracepoints >> NFSD: Add tracepoints to record the result of TEST_STATEID and FREE_STATEID >> NFSD: Rename nfsd_ tracepoints to nfsd4_ >> NFSD: Add tracepoints in the NFS dispatcher >> NFSD: Replace dprintk callsites in fs/nfsd/nfsfh.c >> >> >> fs/lockd/svc4proc.c | 263 +++++++++-- >> fs/lockd/svcproc.c | 265 +++++++++-- >> fs/nfs/callback_xdr.c | 2 + >> fs/nfs/nfs4trace.h | 387 ++-------------- >> fs/nfs/nfstrace.h | 113 +---- >> fs/nfs/pnfs.h | 4 - >> fs/nfsd/nfs2acl.c | 79 +++- >> fs/nfsd/nfs3acl.c | 54 ++- >> fs/nfsd/nfs3proc.c | 25 + >> fs/nfsd/nfs4callback.c | 28 +- >> fs/nfsd/nfs4layouts.c | 16 +- >> fs/nfsd/nfs4proc.c | 43 +- >> fs/nfsd/nfs4state.c | 100 ++-- >> fs/nfsd/nfsd.h | 1 + >> fs/nfsd/nfsfh.c | 36 +- >> fs/nfsd/nfsfh.h | 7 +- >> fs/nfsd/nfsproc.c | 21 + >> fs/nfsd/nfssvc.c | 198 +++++--- >> fs/nfsd/trace.c | 1 + >> fs/nfsd/trace.h | 844 ++++++++++++++++++++++++++++++---- >> fs/nfsd/vfs.c | 18 +- >> fs/nfsd/xdr4.h | 3 +- >> include/linux/nfs4.h | 4 + >> include/linux/sunrpc/svc.h | 1 + >> include/trace/events/fs.h | 30 ++ >> include/trace/events/nfs.h | 511 ++++++++++++++++++++ >> include/trace/events/sunrpc.h | 33 +- >> include/uapi/linux/nfsacl.h | 2 + >> net/sunrpc/svc_xprt.c | 4 +- >> net/sunrpc/svcauth.c | 5 +- >> 30 files changed, 2187 insertions(+), 911 deletions(-) >> create mode 100644 include/trace/events/nfs.h >> >> -- >> Chuck Lever -- Chuck Lever
