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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Sep 06, 2019 at 06:55:07PM +0200, Christoph Hellwig wrote:
> 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:
> 
> http://git.infradead.org/users/hch/xfs.git/shortlog/refs/heads/xfs-cow-iomap

That looks somewhat reasonable. The XFS mapping function is turning
into spagetti and getting really hard to follow again, though.
Perhaps we should consider splitting the shared/COW path out of
it...

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx



[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux