On Mon, 2026-01-19 at 11:47 -0500, Chuck Lever wrote:
> On 1/19/26 11:26 AM, Jeff Layton wrote:
> > Get rid of the dprintk messages in check_export(). Instead add new
> > tracepoints that show the terminal inode and the flags.
> >
> > Signed-off-by: Jeff Layton <[email protected]>
> > ---
> > fs/nfsd/export.c | 11 ++++++-----
> > fs/nfsd/trace.h | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 58 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
> > index
> > bc703cf58bfa210c7c57d49f22f15bc10d7cfc91..3cc336b953b38573966c43000f31cd341380837b
> > 100644
> > --- a/fs/nfsd/export.c
> > +++ b/fs/nfsd/export.c
> > @@ -435,31 +435,32 @@ static int check_export(const struct path *path, int
> > *flags, unsigned char *uuid
> > if (!(inode->i_sb->s_type->fs_flags & FS_REQUIRES_DEV) &&
> > !(*flags & NFSEXP_FSID) &&
> > uuid == NULL) {
> > - dprintk("exp_export: export of non-dev fs without fsid\n");
> > + trace_nfsd_check_export_need_fsid(inode, *flags);
> > return -EINVAL;
> > }
> >
> > if (!exportfs_can_decode_fh(inode->i_sb->s_export_op)) {
> > - dprintk("exp_export: export of invalid fs type.\n");
> > + trace_nfsd_check_export_invalid_fstype(inode, *flags);
> > return -EINVAL;
> > }
> >
> > if (!(inode->i_sb->s_export_op->flags & EXPORT_OP_STABLE_HANDLES)) {
> > - dprintk("%s: fs does not provide stable filehandles!\n",
> > __func__);
> > + trace_nfsd_check_export_no_stable_fh(inode, *flags);
> > return -EINVAL;
> > }
> >
> > if (is_idmapped_mnt(path->mnt)) {
> > dprintk("exp_export: export of idmapped mounts not yet
> > supported.\n");
Doh! I left the above dprintk() in -- fixed in tree.
> > + trace_nfsd_check_export_idmapped(inode, *flags);
> > return -EINVAL;
> > }
> >
> > if (inode->i_sb->s_export_op->flags & EXPORT_OP_NOSUBTREECHK &&
> > !(*flags & NFSEXP_NOSUBTREECHECK)) {
> > - dprintk("%s: %s does not support subtree checking!\n",
> > - __func__, inode->i_sb->s_type->name);
> > + trace_nfsd_check_export_subtree(inode, *flags);
> > return -EINVAL;
> > }
> > + trace_nfsd_check_export_success(inode, *flags);
> > return 0;
> > }
> >
> > diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
> > index
> > 5ae2a611e57f4b4e51a4d9eb6e0fccb66ad8d288..e3f5fe1181b605b34cb70d53f32739c3ef9b82f6
> > 100644
> > --- a/fs/nfsd/trace.h
> > +++ b/fs/nfsd/trace.h
> > @@ -339,6 +339,58 @@ DEFINE_EVENT_CONDITION(nfsd_fh_err_class, nfsd_##name,
> > \
> > DEFINE_NFSD_FH_ERR_EVENT(set_fh_dentry_badexport);
> > DEFINE_NFSD_FH_ERR_EVENT(set_fh_dentry_badhandle);
> >
> > +#define show_export_flags(val)
> > \
>
> Whacky. I thought we had one of these already, but I can't find one.
>
>
> > + __print_flags(val, "|", \
> > + { NFSEXP_READONLY, "READONLY" }, \
> > + { NFSEXP_INSECURE_PORT, "INSECURE" }, \
> > + { NFSEXP_ROOTSQUASH, "ROOTSQUASH" }, \
> > + { NFSEXP_ALLSQUASH, "ALLSQUASH" }, \
> > + { NFSEXP_ASYNC, "ASYNC" }, \
> > + { NFSEXP_GATHERED_WRITES, "GATHERED_WRITES" }, \
> > + { NFSEXP_NOREADDIRPLUS, "NOREADDIRPLUS" }, \
> > + { NFSEXP_SECURITY_LABEL, "SECURITY_LABEL" }, \
> > + { NFSEXP_NOHIDE, "NOHIDE" }, \
> > + { NFSEXP_NOSUBTREECHECK, "NOSUBTREECHECK" }, \
> > + { NFSEXP_NOAUTHNLM, "NOAUTHNLM" }, \
> > + { NFSEXP_MSNFS, "MSNFS" }, \
> > + { NFSEXP_FSID, "FSID" }, \
> > + { NFSEXP_CROSSMOUNT, "CROSSMOUNT" }, \
> > + { NFSEXP_NOACL, "NOACL" }, \
> > + { NFSEXP_V4ROOT, "V4ROOT" }, \
> > + { NFSEXP_PNFS, "PNFS" })
> > +
> > +DECLARE_EVENT_CLASS(nfsd_check_export_class,
> > + TP_PROTO(const struct inode *inode,
> > + int flags),
> > + TP_ARGS(inode, flags),
> > + TP_STRUCT__entry(
> > + __field(dev_t, dev)
> > + __field(ino_t, ino)
> > + __field(int, flags)
> > + ),
> > + TP_fast_assign(
> > + __entry->dev = inode->i_sb->s_dev;
> > + __entry->ino = inode->i_ino;
> > + __entry->flags = flags;
> > + ),
> > + TP_printk("dev=%u:%u:%lu flags=%s",
> > + MAJOR(__entry->dev), MINOR(__entry->dev),
> > + __entry->ino, show_export_flags(__entry->flags))
> > +)
> > +
> > +#define DEFINE_NFSD_CHECK_EXPORT_EVENT(name) \
> > +DEFINE_EVENT(nfsd_check_export_class, nfsd_check_export_##name, \
> > + TP_PROTO(const struct inode *inode, \
> > + int flags), \
> > + TP_ARGS(inode, flags))
> > +
> > +DEFINE_NFSD_CHECK_EXPORT_EVENT(need_fsid);
> > +DEFINE_NFSD_CHECK_EXPORT_EVENT(invalid_fstype);
> > +DEFINE_NFSD_CHECK_EXPORT_EVENT(no_stable_fh);
> > +DEFINE_NFSD_CHECK_EXPORT_EVENT(idmapped);
> > +DEFINE_NFSD_CHECK_EXPORT_EVENT(subtree);
> > +DEFINE_NFSD_CHECK_EXPORT_EVENT(success);
> > +
> > TRACE_EVENT(nfsd_exp_find_key,
> > TP_PROTO(const struct svc_expkey *key,
> > int status),
> >
>
> 'Twould be nice to report the namespace or client address that
> was making the failing request, but maybe that information is
> not available in check_export.
>
> Reviewed-by: Chuck Lever <[email protected]>
>
That might be possible, but it means refactoring check_export().
Given that we're still haggling over the flag name and semantics, lets
drop this patch from the series for now.
Once the other bits are settled, I'll respin the tracepoint patches on
top. That part can be better sorted out on the linux-nfs ml anyway.
Thanks!
--
Jeff Layton <[email protected]>
