linux-kernel - Re: [PATCH] xfs: fix confused tracepoints in xfs_reflink_end_atomic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aSQmomhODBHTip8j@infradead.org>
Date: Mon, 24 Nov 2025 01:34:26 -0800
From: Christoph Hellwig <hch@...radead.org>
To: alexjlzheng@...il.com
Cc: cem@...nel.org, linux-xfs@...r.kernel.org, linux-kernel@...r.kernel.org,
	Jinliang Zheng <alexjlzheng@...cent.com>,
	John Garry <john.g.garry@...cle.com>
Subject: Re: [PATCH] xfs: fix confused tracepoints in
 xfs_reflink_end_atomic_cow()

On Fri, Nov 21, 2025 at 07:56:56PM +0800, alexjlzheng@...il.com wrote:
> From: Jinliang Zheng <alexjlzheng@...cent.com>
> 
> The commit b1e09178b73a ("xfs: commit CoW-based atomic writes atomically")
> introduced xfs_reflink_end_atomic_cow() for atomic CoW-based writes, but
> it used the same tracepoint as xfs_reflink_end_cow(), making trace logs
> ambiguous.
> 
> This patch adds two new tracepoints trace_xfs_reflink_end_atomic_cow() and
> trace_xfs_reflink_end_atomic_cow_error() to distinguish them.

Confused sounds a bit strong, but otherwise this looks good:

Reviewed-by: Christoph Hellwig <hch@....de>

Semi-related:  back when this code was added I asked why we're not
using the transaction / defer ops chaining even for normale reflink
completions, as it should be just as efficient and that way we have
less code to maintain and less diverging code paths.  Or am I missing
something?

> 
> Signed-off-by: Jinliang Zheng <alexjlzheng@...cent.com>
> ---
>  fs/xfs/xfs_reflink.c | 4 ++--
>  fs/xfs/xfs_trace.h   | 2 ++
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 3f177b4ec131..47f532fd46e0 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -1003,7 +1003,7 @@ xfs_reflink_end_atomic_cow(
>  	struct xfs_trans		*tp;
>  	unsigned int			resblks;
>  
> -	trace_xfs_reflink_end_cow(ip, offset, count);
> +	trace_xfs_reflink_end_atomic_cow(ip, offset, count);
>  
>  	offset_fsb = XFS_B_TO_FSBT(mp, offset);
>  	end_fsb = XFS_B_TO_FSB(mp, offset + count);
> @@ -1028,7 +1028,7 @@ xfs_reflink_end_atomic_cow(
>  				end_fsb);
>  	}
>  	if (error) {
> -		trace_xfs_reflink_end_cow_error(ip, error, _RET_IP_);
> +		trace_xfs_reflink_end_atomic_cow_error(ip, error, _RET_IP_);
>  		goto out_cancel;
>  	}
>  	error = xfs_trans_commit(tp);
> diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
> index 79b8641880ab..29eefacb8226 100644
> --- a/fs/xfs/xfs_trace.h
> +++ b/fs/xfs/xfs_trace.h
> @@ -4186,12 +4186,14 @@ DEFINE_INODE_IREC_EVENT(xfs_reflink_convert_cow);
>  
>  DEFINE_SIMPLE_IO_EVENT(xfs_reflink_cancel_cow_range);
>  DEFINE_SIMPLE_IO_EVENT(xfs_reflink_end_cow);
> +DEFINE_SIMPLE_IO_EVENT(xfs_reflink_end_atomic_cow);
>  DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_remap_from);
>  DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_remap_to);
>  DEFINE_INODE_IREC_EVENT(xfs_reflink_cow_remap_skip);
>  
>  DEFINE_INODE_ERROR_EVENT(xfs_reflink_cancel_cow_range_error);
>  DEFINE_INODE_ERROR_EVENT(xfs_reflink_end_cow_error);
> +DEFINE_INODE_ERROR_EVENT(xfs_reflink_end_atomic_cow_error);
>  
>  
>  DEFINE_INODE_IREC_EVENT(xfs_reflink_cancel_cow);
> -- 
> 2.49.0
> 
> 
---end quoted text---