[v3,0/15] Btrfs iomap
mbox series

Message ID 20190901200836.14959-1-rgoldwyn@suse.de
Headers show
Series
  • Btrfs iomap
Related show

Message

Goldwyn Rodrigues Sept. 1, 2019, 8:08 p.m. UTC
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.

[1] https://github.com/goldwynr/linux/tree/btrfs-iomap

Comments

Christoph Hellwig Sept. 2, 2019, 4:43 p.m. UTC | #1
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?
Shiyang Ruan Sept. 3, 2019, 3:51 a.m. UTC | #2
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
Darrick J. Wong Sept. 3, 2019, 4:29 a.m. UTC | #3
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?
> > 
> > 
> 
> 
>
Shiyang Ruan Sept. 3, 2019, 5 a.m. UTC | #4
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.
Christoph Hellwig Sept. 3, 2019, 6:21 a.m. UTC | #5
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.
Goldwyn Rodrigues Sept. 3, 2019, 1:44 p.m. UTC | #6
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.