mbox series

[RFC,v2,0/7] xfs: reflink & dedupe for fsdax (read/write path).

Message ID 20191030041358.14450-1-ruansy.fnst@cn.fujitsu.com (mailing list archive)
Headers show
Series xfs: reflink & dedupe for fsdax (read/write path). | expand

Message

Ruan Shiyang Oct. 30, 2019, 4:13 a.m. UTC
This patchset aims to take care of this issue to make reflink and dedupe
work correctly (actually in read/write path, there still has some problems,
such as the page->mapping and page->index issue, in mmap path) in XFS under
fsdax mode.

It is based on Goldwyn's patchsets: "v4 Btrfs dax support" and the latest
iomap.  I borrowed some patches related and made a few fix to make it
basically works fine.

For dax framework: 
  1. adapt to the latest change in iomap (two iomaps).

For XFS:
  1. distinguish dax write/zero from normal write/zero.
  2. remap extents after COW.
  3. add file contents comparison function based on dax framework.
  4. use xfs_break_layouts() instead of break_layout to support dax.


Goldwyn Rodrigues (3):
  dax: replace mmap entry in case of CoW
  fs: dedup file range to use a compare function
  dax: memcpy before zeroing range

Shiyang Ruan (4):
  dax: Introduce dax_copy_edges() for COW.
  dax: copy data before write.
  xfs: handle copy-on-write in fsdax write() path.
  xfs: support dedupe for fsdax.

 fs/btrfs/ioctl.c       |   3 +-
 fs/dax.c               | 211 +++++++++++++++++++++++++++++++++++++----
 fs/iomap/buffered-io.c |   8 +-
 fs/ocfs2/file.c        |   2 +-
 fs/read_write.c        |  11 ++-
 fs/xfs/xfs_bmap_util.c |   6 +-
 fs/xfs/xfs_file.c      |  10 +-
 fs/xfs/xfs_iomap.c     |   3 +-
 fs/xfs/xfs_iops.c      |  11 ++-
 fs/xfs/xfs_reflink.c   |  79 ++++++++-------
 include/linux/dax.h    |  16 ++--
 include/linux/fs.h     |   9 +-
 12 files changed, 291 insertions(+), 78 deletions(-)

Comments

Goldwyn Rodrigues Oct. 30, 2019, 11:48 a.m. UTC | #1
On 12:13 30/10, Shiyang Ruan wrote:
> This patchset aims to take care of this issue to make reflink and dedupe
> work correctly (actually in read/write path, there still has some problems,
> such as the page->mapping and page->index issue, in mmap path) in XFS under
> fsdax mode.

Have you managed to solve the problem of multi-mapped pages? I don't
think we can include this until we solve that problem. This is the
problem I faced when I was doing the btrfs dax support.

Suppose there is an extent shared with multiple files. You map data for
both files. Which inode should page->mapping->host (precisely
page->mapping) point to? As Dave pointed out, this needs to be fixed at
the mm level, and will not only benefit dax with CoW but other
areas such as overlayfs and possibly containers.
Ruan Shiyang Oct. 31, 2019, 4:54 a.m. UTC | #2
On 10/30/19 7:48 PM, Goldwyn Rodrigues wrote:
> On 12:13 30/10, Shiyang Ruan wrote:
>> This patchset aims to take care of this issue to make reflink and dedupe
>> work correctly (actually in read/write path, there still has some problems,
>> such as the page->mapping and page->index issue, in mmap path) in XFS under
>> fsdax mode.
> 
> Have you managed to solve the problem of multi-mapped pages? I don't
> think we can include this until we solve that problem. This is the
> problem I faced when I was doing the btrfs dax support.

That problem still exists, didn't be solved in this patchset.  But I am 
also looking into it.  As you know, it's a bit difficult.

Since the iomap for cow is merged in for-next tree, I think it's time to 
update this in order to get some comments.

> 
> Suppose there is an extent shared with multiple files. You map data for
> both files. Which inode should page->mapping->host (precisely
> page->mapping) point to? As Dave pointed out, this needs to be fixed at
> the mm level, and will not only benefit dax with CoW but other
> areas such as overlayfs and possibly containers.

Yes, I will try to figure out a solution.
>
Ruan Shiyang Nov. 8, 2019, 3:10 a.m. UTC | #3
Hi Darrick, Dave,

Do you have any comment on this?

On 10/30/19 12:13 PM, Shiyang Ruan wrote:
> This patchset aims to take care of this issue to make reflink and dedupe
> work correctly (actually in read/write path, there still has some problems,
> such as the page->mapping and page->index issue, in mmap path) in XFS under
> fsdax mode.
> 
> It is based on Goldwyn's patchsets: "v4 Btrfs dax support" and the latest
> iomap.  I borrowed some patches related and made a few fix to make it
> basically works fine.
> 
> For dax framework:
>    1. adapt to the latest change in iomap (two iomaps).
> 
> For XFS:
>    1. distinguish dax write/zero from normal write/zero.
>    2. remap extents after COW.
>    3. add file contents comparison function based on dax framework.
>    4. use xfs_break_layouts() instead of break_layout to support dax.
> 
> 
> Goldwyn Rodrigues (3):
>    dax: replace mmap entry in case of CoW
>    fs: dedup file range to use a compare function
>    dax: memcpy before zeroing range
> 
> Shiyang Ruan (4):
>    dax: Introduce dax_copy_edges() for COW.
>    dax: copy data before write.
>    xfs: handle copy-on-write in fsdax write() path.
>    xfs: support dedupe for fsdax.
> 
>   fs/btrfs/ioctl.c       |   3 +-
>   fs/dax.c               | 211 +++++++++++++++++++++++++++++++++++++----
>   fs/iomap/buffered-io.c |   8 +-
>   fs/ocfs2/file.c        |   2 +-
>   fs/read_write.c        |  11 ++-
>   fs/xfs/xfs_bmap_util.c |   6 +-
>   fs/xfs/xfs_file.c      |  10 +-
>   fs/xfs/xfs_iomap.c     |   3 +-
>   fs/xfs/xfs_iops.c      |  11 ++-
>   fs/xfs/xfs_reflink.c   |  79 ++++++++-------
>   include/linux/dax.h    |  16 ++--
>   include/linux/fs.h     |   9 +-
>   12 files changed, 291 insertions(+), 78 deletions(-)
>
Dan Williams Nov. 8, 2019, 3:30 a.m. UTC | #4
On Thu, Nov 7, 2019 at 7:11 PM Shiyang Ruan <ruansy.fnst@cn.fujitsu.com> wrote:
>
> Hi Darrick, Dave,
>
> Do you have any comment on this?

Christoph pointed out at ALPSS that this problem has significant
overlap with the shared page-cache for reflink problem. So I think we
need to solve that first and then circle back to dax reflink support.
I'm starting to take a look at that.
Dave Chinner Nov. 14, 2019, 8:24 p.m. UTC | #5
On Thu, Nov 07, 2019 at 07:30:32PM -0800, Dan Williams wrote:
> On Thu, Nov 7, 2019 at 7:11 PM Shiyang Ruan <ruansy.fnst@cn.fujitsu.com> wrote:
> >
> > Hi Darrick, Dave,
> >
> > Do you have any comment on this?
> 
> Christoph pointed out at ALPSS that this problem has significant
> overlap with the shared page-cache for reflink problem. So I think we
> need to solve that first and then circle back to dax reflink support.
> I'm starting to take a look at that.

I think the DAX side is somewhat simpler because it doesn't really
need to involve the page cache and we don't have to worry about
subtly breaking random filesystems. Hence I'd suggest we sort out a
solution for DAX first, then worry about page cache stuff. The
shared page cache for reflink feature is not a show stopper -
multiple references for DAX is a show stopper. Let's deal with the
DAX problem first.

Cheers,

-Dave.