Re: [PATCH 09/11] xfs: remove the fork fields in the writepage_ctx and ioend

From: Dave Chinner
Date: Mon Oct 07 2019 - 17:59:50 EST


On Sun, Oct 06, 2019 at 05:46:06PM +0200, Christoph Hellwig wrote:
> In preparation for moving the writeback code to iomap.c, replace the
> XFS-specific COW fork concept with the iomap IOMAP_F_SHARED flag.

"In preparation for switching XFS to use the fs/iomap writeback
code..."?

I suspect the IOMAP_F_SHARED hunk I pointed out in the previous
patch should be in this one...

>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> Reviewed-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
> Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> fs/xfs/xfs_aops.c | 42 ++++++++++++++++++++++--------------------
> fs/xfs/xfs_aops.h | 2 +-
> 2 files changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/fs/xfs/xfs_aops.c b/fs/xfs/xfs_aops.c
> index 9f22885902ef..8c101081e3b1 100644
> --- a/fs/xfs/xfs_aops.c
> +++ b/fs/xfs/xfs_aops.c
> @@ -23,7 +23,6 @@
> */
> struct xfs_writepage_ctx {
> struct iomap iomap;
> - int fork;
> unsigned int data_seq;
> unsigned int cow_seq;
> struct xfs_ioend *ioend;
> @@ -257,7 +256,7 @@ xfs_end_ioend(
> */
> error = blk_status_to_errno(ioend->io_bio->bi_status);
> if (unlikely(error)) {
> - if (ioend->io_fork == XFS_COW_FORK)
> + if (ioend->io_flags & IOMAP_F_SHARED)
> xfs_reflink_cancel_cow_range(ip, offset, size, true);
> goto done;
> }
> @@ -265,7 +264,7 @@ xfs_end_ioend(
> /*
> * Success: commit the COW or unwritten blocks if needed.
> */
> - if (ioend->io_fork == XFS_COW_FORK)
> + if (ioend->io_flags & IOMAP_F_SHARED)
> error = xfs_reflink_end_cow(ip, offset, size);
> else if (ioend->io_type == IOMAP_UNWRITTEN)
> error = xfs_iomap_write_unwritten(ip, offset, size, false);
> @@ -298,7 +297,8 @@ xfs_ioend_can_merge(
> {
> if (ioend->io_bio->bi_status != next->io_bio->bi_status)
> return false;
> - if ((ioend->io_fork == XFS_COW_FORK) ^ (next->io_fork == XFS_COW_FORK))
> + if ((ioend->io_flags & IOMAP_F_SHARED) ^
> + (next->io_flags & IOMAP_F_SHARED))
> return false;
> if ((ioend->io_type == IOMAP_UNWRITTEN) ^
> (next->io_type == IOMAP_UNWRITTEN))

These probably should be indented too, as they are continuations,
not separate logic statements.

> @@ -768,7 +769,8 @@ xfs_add_to_ioend(
> bool merged, same_page = false;
>
> if (!wpc->ioend ||
> - wpc->fork != wpc->ioend->io_fork ||
> + (wpc->iomap.flags & IOMAP_F_SHARED) !=
> + (wpc->ioend->io_flags & IOMAP_F_SHARED) ||

Same here.

Cheers,

Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx