Re: [PATCH v2 31/31] nfsd: convert dprintks in check_export() to tracepoints

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");
> +		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]>


-- 
Chuck Lever