| Message ID | 20190901200836.14959-1-rgoldwyn@suse.de (mailing list archive) |
|---|---|
| Headers | show |
| Series | Btrfs iomap | expand |
On Sun, Sep 01, 2019 at 03:08:21PM -0500, Goldwyn Rodrigues wrote: > This is an effort to use iomap for btrfs. This would keep most > responsibility of page handling during writes in iomap code, hence > code reduction. For CoW support, changes are needed in iomap code > to make sure we perform a copy before the write. > This is in line with the discussion we had during adding dax support in > btrfs. This looks pretty good modulo a few comments. Can you please convert the XFS code to use your two iomaps for COW approach as well to validate it? Also the iomap_file_dirty helper would really benefit from using the two iomaps, any chance you could look into improving it to use your new infrastructure?
On 9/3/19 12:43 AM, Christoph Hellwig wrote: > On Sun, Sep 01, 2019 at 03:08:21PM -0500, Goldwyn Rodrigues wrote: >> This is an effort to use iomap for btrfs. This would keep most >> responsibility of page handling during writes in iomap code, hence >> code reduction. For CoW support, changes are needed in iomap code >> to make sure we perform a copy before the write. >> This is in line with the discussion we had during adding dax support in >> btrfs. > > This looks pretty good modulo a few comments. > > Can you please convert the XFS code to use your two iomaps for COW > approach as well to validate it? Hi, The XFS part of dax CoW support has been implementing recently. Please review this[1] if necessary. It's based on this iomap patchset(the 1st version), and uses the new srcmap. [1]: https://lkml.org/lkml/2019/7/31/449
On Tue, Sep 03, 2019 at 11:51:24AM +0800, Shiyang Ruan wrote: > > > On 9/3/19 12:43 AM, Christoph Hellwig wrote: > > On Sun, Sep 01, 2019 at 03:08:21PM -0500, Goldwyn Rodrigues wrote: > > > This is an effort to use iomap for btrfs. This would keep most > > > responsibility of page handling during writes in iomap code, hence > > > code reduction. For CoW support, changes are needed in iomap code > > > to make sure we perform a copy before the write. > > > This is in line with the discussion we had during adding dax support in > > > btrfs. > > > > This looks pretty good modulo a few comments. > > > > Can you please convert the XFS code to use your two iomaps for COW > > approach as well to validate it? > > Hi, > > The XFS part of dax CoW support has been implementing recently. Please > review this[1] if necessary. It's based on this iomap patchset(the 1st > version), and uses the new srcmap. > > [1]: https://lkml.org/lkml/2019/7/31/449 It sure would be nice to have (a) this patchset of Goldwyn's cleaned up a bit per the review comments and (b) the XFS DAX COW series rebased on that (instead of a month-old submission). ;) --D > -- > Thanks, > Shiyang Ruan. > > > > Also the iomap_file_dirty helper would really benefit from using the > > two iomaps, any chance you could look into improving it to use your > > new infrastructure? > > > > > > >
On 9/3/19 12:29 PM, Darrick J. Wong wrote: > On Tue, Sep 03, 2019 at 11:51:24AM +0800, Shiyang Ruan wrote: >> >> >> On 9/3/19 12:43 AM, Christoph Hellwig wrote: >>> On Sun, Sep 01, 2019 at 03:08:21PM -0500, Goldwyn Rodrigues wrote: >>>> This is an effort to use iomap for btrfs. This would keep most >>>> responsibility of page handling during writes in iomap code, hence >>>> code reduction. For CoW support, changes are needed in iomap code >>>> to make sure we perform a copy before the write. >>>> This is in line with the discussion we had during adding dax support in >>>> btrfs. >>> >>> This looks pretty good modulo a few comments. >>> >>> Can you please convert the XFS code to use your two iomaps for COW >>> approach as well to validate it? >> >> Hi, >> >> The XFS part of dax CoW support has been implementing recently. Please >> review this[1] if necessary. It's based on this iomap patchset(the 1st >> version), and uses the new srcmap. >> >> [1]: https://lkml.org/lkml/2019/7/31/449 > > It sure would be nice to have (a) this patchset of Goldwyn's cleaned up > a bit per the review comments and (b) the XFS DAX COW series rebased on > that (instead of a month-old submission). ;) Yes, I think so too. Just in case Christoph did not notice that.
On Tue, Sep 03, 2019 at 11:51:24AM +0800, Shiyang Ruan wrote: > The XFS part of dax CoW support has been implementing recently. Please > review this[1] if necessary. It's based on this iomap patchset(the 1st > version), and uses the new srcmap. > > [1]: https://lkml.org/lkml/2019/7/31/449 For the initial stage I'm not interested in DAX reflink support yet (although that is also very interesting), just to make sure the two iomap support is also used by the buffer write path for XFS, removing the current hack where iomap_begin reports the data fork mapping only and relies in ->writepage to handle that case.
On 18:43 02/09, Christoph Hellwig wrote: > On Sun, Sep 01, 2019 at 03:08:21PM -0500, Goldwyn Rodrigues wrote: > > This is an effort to use iomap for btrfs. This would keep most > > responsibility of page handling during writes in iomap code, hence > > code reduction. For CoW support, changes are needed in iomap code > > to make sure we perform a copy before the write. > > This is in line with the discussion we had during adding dax support in > > btrfs. > > This looks pretty good modulo a few comments. > > Can you please convert the XFS code to use your two iomaps for COW > approach as well to validate it? > > Also the iomap_file_dirty helper would really benefit from using the > two iomaps, any chance you could look into improving it to use your > new infrastructure? Yes. However, I haven't looked at much of XFS code, but from the initial looks of it, it is simple to read.