On Mon, Apr 29, 2024 at 06:49:10AM +0200, Christoph Hellwig wrote:
> Defer the extent counter size upgrade until we know we're going to
> modify the extent mapping. This also defers dirtying the transaction
> and will allow us safely back out later in the function in later
> changes.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/xfs/xfs_reflink.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 7da0e8f961d351..9ce37d366534c3 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -751,14 +751,6 @@ xfs_reflink_end_cow_extent(
> xfs_ilock(ip, XFS_ILOCK_EXCL);
> xfs_trans_ijoin(tp, ip, 0);
>
> - error = xfs_iext_count_may_overflow(ip, XFS_DATA_FORK,
> - XFS_IEXT_REFLINK_END_COW_CNT);
> - if (error == -EFBIG)
> - error = xfs_iext_count_upgrade(tp, ip,
> - XFS_IEXT_REFLINK_END_COW_CNT);
> - if (error)
> - goto out_cancel;
> -
> /*
> * In case of racing, overlapping AIO writes no COW extents might be
> * left by the time I/O completes for the loser of the race. In that
I think this is actually a bug fix. If an xfs_iext_count_upgrade
dirties the transaction and then xfs_iext_lookup_extent cancels the
transaction due to the overlapping AIO race, the _trans_cancel shuts
down the fs, right?
Fixes: 4f86bb4b66c9 ("xfs: Conditionally upgrade existing inodes to use large extent counters")
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> @@ -787,6 +779,14 @@ xfs_reflink_end_cow_extent(
> del = got;
> xfs_trim_extent(&del, *offset_fsb, end_fsb - *offset_fsb);
>
> + error = xfs_iext_count_may_overflow(ip, XFS_DATA_FORK,
> + XFS_IEXT_REFLINK_END_COW_CNT);
> + if (error == -EFBIG)
> + error = xfs_iext_count_upgrade(tp, ip,
> + XFS_IEXT_REFLINK_END_COW_CNT);
> + if (error)
> + goto out_cancel;
> +
> /* Grab the corresponding mapping in the data fork. */
> nmaps = 1;
> error = xfs_bmapi_read(ip, del.br_startoff, del.br_blockcount, &data,
> --
> 2.39.2
>
>
