Re: [PATCH 1/3] pNFS: report clora_changed in the cb_layoutrecall_file tracepoint

Hi Ben,

On Wed, Jun 24, 2026, at 3:50 PM, Benjamin Coddington wrote:
> A CB_LAYOUTRECALL carries the clora_changed flag (RFC 8881, Section
> 20.3.3), which tells the client whether the server is changing the
> layout (and therefore whether the client should flush modified data to
> the storage devices before returning, or stop writing to them and go
> through the metadata server). The client decodes this into
> cbl_layoutchanged, but it is otherwise invisible.
>
> Give nfs4_cb_layoutrecall_file its own event definition and report
> clora_changed, so the intent of a recall can be observed in a trace.
>
> Signed-off-by: Benjamin Coddington <bcodding-F/[email protected]>
> ---
>  fs/nfs/callback_proc.c |  2 +-
>  fs/nfs/nfs4trace.h     | 55 +++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 55 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
> index 4ea9221ded42..021572312b0e 100644
> --- a/fs/nfs/callback_proc.c
> +++ b/fs/nfs/callback_proc.c
> @@ -316,7 +316,7 @@ static u32 initiate_file_draining(struct nfs_client *clp,
>  	nfs_iput_and_deactive(ino);
>  out_noput:
>  	trace_nfs4_cb_layoutrecall_file(clp, &args->cbl_fh, ino,
> -			&args->cbl_stateid, -rv);
> +			&args->cbl_stateid, args->cbl_layoutchanged, -rv);
>  	return rv;
>  }
> 
> diff --git a/fs/nfs/nfs4trace.h b/fs/nfs/nfs4trace.h
> index c939533b9881..18a5708c17fc 100644
> --- a/fs/nfs/nfs4trace.h
> +++ b/fs/nfs/nfs4trace.h
> @@ -1515,7 +1515,60 @@ DECLARE_EVENT_CLASS(nfs4_inode_stateid_callback_event,
>  			), \
>  			TP_ARGS(clp, fhandle, inode, stateid, error))
>  DEFINE_NFS4_INODE_STATEID_CALLBACK_EVENT(nfs4_cb_recall);
> -DEFINE_NFS4_INODE_STATEID_CALLBACK_EVENT(nfs4_cb_layoutrecall_file);
> +
> +TRACE_EVENT(nfs4_cb_layoutrecall_file,
> +		TP_PROTO(
> +			const struct nfs_client *clp,
> +			const struct nfs_fh *fhandle,
> +			const struct inode *inode,
> +			const nfs4_stateid *stateid,
> +			unsigned int changed,
> +			int error
> +		),
> +
> +		TP_ARGS(clp, fhandle, inode, stateid, changed, error),
> +
> +		TP_STRUCT__entry(
> +			__field(unsigned long, error)
> +			__field(dev_t, dev)
> +			__field(u32, fhandle)
> +			__field(u64, fileid)
> +			__string(dstaddr, clp ? clp->cl_hostname : "unknown")
> +			__field(int, stateid_seq)
> +			__field(u32, stateid_hash)
> +			__field(unsigned int, changed)
> +		),
> +
> +		TP_fast_assign(
> +			__entry->error = error < 0 ? -error : 0;
> +			__entry->fhandle = nfs_fhandle_hash(fhandle);
> +			if (!IS_ERR_OR_NULL(inode)) {
> +				__entry->fileid = NFS_FILEID(inode);

Note that Jeff had some patches that went into 7.2 to store the nfs fileid
directly in the inode. The NFS_FILEID() macro has been removed, and you should
use inode->i_ino instead.

Thanks,
Anna

> +				__entry->dev = inode->i_sb->s_dev;
> +			} else {
> +				__entry->fileid = 0;
> +				__entry->dev = 0;
> +			}
> +			__assign_str(dstaddr);
> +			__entry->stateid_seq =
> +				be32_to_cpu(stateid->seqid);
> +			__entry->stateid_hash =
> +				nfs_stateid_hash(stateid);
> +			__entry->changed = changed;
> +		),
> +
> +		TP_printk(
> +			"error=%ld (%s) fileid=%02x:%02x:%llu fhandle=0x%08x "
> +			"stateid=%d:0x%08x dstaddr=%s clora_changed=%u",
> +			-__entry->error,
> +			show_nfs4_status(__entry->error),
> +			MAJOR(__entry->dev), MINOR(__entry->dev),
> +			(unsigned long long)__entry->fileid,
> +			__entry->fhandle,
> +			__entry->stateid_seq, __entry->stateid_hash,
> +			__get_str(dstaddr), __entry->changed
> +		)
> +);
> 
>  #define show_stateid_type(type) \
>  	__print_symbolic(type, \
> -- 
> 2.53.0