On Tue, Jun 25, 2024 at 12:57:54AM -0400, Mike Snitzer wrote: > On Mon, Jun 24, 2024 at 02:26:42PM -0400, Chuck Lever wrote: > > On Mon, Jun 24, 2024 at 12:27:27PM -0400, Mike Snitzer wrote: > > > From: Weston Andros Adamson <dros@xxxxxxxxxxxxxxx> > > > diff --git a/fs/nfsd/localio.c b/fs/nfsd/localio.c > > > new file mode 100644 > > > index 000000000000..e9aa0997f898 > > > --- /dev/null > > > +++ b/fs/nfsd/localio.c > > > @@ -0,0 +1,244 @@ > > > +// SPDX-License-Identifier: GPL-2.0-only > > > +/* > > > + * NFS server support for local clients to bypass network stack > > > + * > > > + * Copyright (C) 2014 Weston Andros Adamson <dros@xxxxxxxxxxxxxxx> > > > + * Copyright (C) 2019 Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> > > > + * Copyright (C) 2024 Mike Snitzer <snitzer@xxxxxxxxxxxxxxx> > > > + */ > > > + > > > +#include <linux/exportfs.h> > > > +#include <linux/sunrpc/svcauth_gss.h> > > > +#include <linux/sunrpc/clnt.h> > > > +#include <linux/nfs.h> > > > +#include <linux/string.h> > > > + > > > +#include "nfsd.h" > > > +#include "vfs.h" > > > +#include "netns.h" > > > +#include "filecache.h" > > > + > > > +#define NFSDDBG_FACILITY NFSDDBG_FH > > > > I think I'd rather prefer to see trace points in here rather than > > new dprintk call sites. In any event, perhaps NFSDDBG_FH is not > > especially appropriate? > > I think NFSDDBG_FH is most appropriate given this localio is most > focused on opening a file handle. (The getuuid not so much) If more than one debugging facility is appropriate, consider using dfprintk() instead so each call site can specify its preferred facility. > I'm not loving how overdone the trace points interface is. in my > experience, trace points are also a serious source of conflicts when > backporting changes to older kernels. > > But if you were inclined to switch this code over to trace points once > it is merged, who am I to stop you! ;) Have a heartfelt conversation with yourself about whether all of these dprintk's need to stay, then. Anything you've been using strictly as a development tool can be considered disposable. Anything an admin or support engineer might want to enable to diagnose a local configuration or deployment issue is a candidate for staying in the code. And so on and so forth. -- Chuck Lever
