On Thu, Feb 13, 2025 at 01:56:14PM +0000, John Garry wrote:
> For CoW-based atomic write support, always allocate a cow hole in
> xfs_reflink_allocate_cow() to write the new data.
>
> The semantics is that if @atomic is set, we will be passed a CoW fork
> extent mapping for no error returned.
>
> Signed-off-by: John Garry <john.g.garry@xxxxxxxxxx>
> ---
> fs/xfs/xfs_iomap.c | 2 +-
> fs/xfs/xfs_reflink.c | 12 +++++++-----
> fs/xfs/xfs_reflink.h | 2 +-
> 3 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index d61460309a78..ab79f0080288 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -865,7 +865,7 @@ xfs_direct_write_iomap_begin(
> /* may drop and re-acquire the ilock */
> error = xfs_reflink_allocate_cow(ip, &imap, &cmap, &shared,
> &lockmode,
> - (flags & IOMAP_DIRECT) || IS_DAX(inode));
> + (flags & IOMAP_DIRECT) || IS_DAX(inode), false);
Now I'm /really/ think it's time for some reflink allocation flags,
because the function signature now involves two booleans...
> if (error)
> goto out_unlock;
> if (shared)
> diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c
> index 8428f7b26ee6..3dab3ba900a3 100644
> --- a/fs/xfs/xfs_reflink.c
> +++ b/fs/xfs/xfs_reflink.c
> @@ -435,7 +435,8 @@ xfs_reflink_fill_cow_hole(
> struct xfs_bmbt_irec *cmap,
> bool *shared,
> uint *lockmode,
> - bool convert_now)
> + bool convert_now,
> + bool atomic)
...but this can come later. Also, is atomic==true only for the
ATOMIC_SW operation? I think so, but that's the unfortunate thing about
booleans.
> {
> struct xfs_mount *mp = ip->i_mount;
> struct xfs_trans *tp;
> @@ -466,7 +467,7 @@ xfs_reflink_fill_cow_hole(
> *lockmode = XFS_ILOCK_EXCL;
>
> error = xfs_find_trim_cow_extent(ip, imap, cmap, shared, &found);
> - if (error || !*shared)
> + if (error || (!*shared && !atomic))
> goto out_trans_cancel;
>
> if (found) {
> @@ -566,7 +567,8 @@ xfs_reflink_allocate_cow(
> struct xfs_bmbt_irec *cmap,
> bool *shared,
> uint *lockmode,
> - bool convert_now)
> + bool convert_now,
> + bool atomic)
Nit: ^ space before tab.
If the answer to the question above is 'yes' then with that nit fixed,
Reviewed-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>
--D
> {
> int error;
> bool found;
> @@ -578,7 +580,7 @@ xfs_reflink_allocate_cow(
> }
>
> error = xfs_find_trim_cow_extent(ip, imap, cmap, shared, &found);
> - if (error || !*shared)
> + if (error || (!*shared && !atomic))
> return error;
>
> /* CoW fork has a real extent */
> @@ -592,7 +594,7 @@ xfs_reflink_allocate_cow(
> */
> if (cmap->br_startoff > imap->br_startoff)
> return xfs_reflink_fill_cow_hole(ip, imap, cmap, shared,
> - lockmode, convert_now);
> + lockmode, convert_now, atomic);
>
> /*
> * CoW fork has a delalloc reservation. Replace it with a real extent.
> diff --git a/fs/xfs/xfs_reflink.h b/fs/xfs/xfs_reflink.h
> index cc4e92278279..754d2bb692d3 100644
> --- a/fs/xfs/xfs_reflink.h
> +++ b/fs/xfs/xfs_reflink.h
> @@ -32,7 +32,7 @@ int xfs_bmap_trim_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *imap,
>
> int xfs_reflink_allocate_cow(struct xfs_inode *ip, struct xfs_bmbt_irec *imap,
> struct xfs_bmbt_irec *cmap, bool *shared, uint *lockmode,
> - bool convert_now);
> + bool convert_now, bool atomic);
> extern int xfs_reflink_convert_cow(struct xfs_inode *ip, xfs_off_t offset,
> xfs_off_t count);
>
> --
> 2.31.1
>
>
