Re: [PATCH 15/15] xfs: Use the new iomap infrastructure for CoW

Christoph Hellwig
2019-09-06T16:55:11Z

On Thu, Sep 05, 2019 at 10:06:50AM -0500, Goldwyn Rodrigues wrote:
> diff --git a/fs/xfs/xfs_iomap.c b/fs/xfs/xfs_iomap.c
> index 8321733c16c3..13495d8a1ee2 100644
> --- a/fs/xfs/xfs_iomap.c
> +++ b/fs/xfs/xfs_iomap.c
> @@ -1006,7 +1006,10 @@ xfs_file_iomap_begin(
> */
> if (directio || imap.br_startblock == HOLESTARTBLOCK)
> imap = cmap;
> + else
> + xfs_bmbt_to_iomap(ip, srcmap, &cmap, false);
>
> + iomap->flags |= IOMAP_F_COW;

I don't think this is correct. We should only set IOMAP_F_COW
when we actually fill out the srcmap. Which is a very good agument
for Darrick's suggestion to check for a non-emptry srcmap.

Also this is missing the actually interesting part in
xfs_file_iomap_begin_delay.

I ended up spending the better half of the day trying to implement
that and did run into a few bugs in the core iomap changes, mostly
due to a confusion which iomap to use. So I ended up reworking those
a bit to:

a) check srcmap->type to see if there is a valid srcmap
b) set the srcmap pointer to iomap so that it doesn't need to
be special cased all over
c) fixed up a few more places to use the srcmap

This now at least survives xfstests -g quick on a 4k xfs file system
for. Here is my current tree: