From: "ruansy.fnst@fujitsu.com" <ruansy.fnst@fujitsu.com> To: Su Yue <l@damenly.su> Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-xfs@vger.kernel.org" <linux-xfs@vger.kernel.org>, "linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>, "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>, "darrick.wong@oracle.com" <darrick.wong@oracle.com>, "willy@infradead.org" <willy@infradead.org>, "jack@suse.cz" <jack@suse.cz>, "viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>, "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>, "david@fromorbit.com" <david@fromorbit.com>, "hch@lst.de" <hch@lst.de>, "rgoldwyn@suse.de" <rgoldwyn@suse.de> Subject: RE: [PATCH v4 7/7] fs/xfs: Add dedupe support for fsdax Date: Fri, 9 Apr 2021 01:56:48 +0000 [thread overview] Message-ID: <OSAPR01MB291345BD169337AE2BCAA717F4739@OSAPR01MB2913.jpnprd01.prod.outlook.com> (raw) In-Reply-To: <czv4syut.fsf@damenly.su> > -----Original Message----- > From: Su Yue <l@damenly.su> > Subject: Re: [PATCH v4 7/7] fs/xfs: Add dedupe support for fsdax > > > On Thu 08 Apr 2021 at 20:04, Shiyang Ruan <ruansy.fnst@fujitsu.com> wrote: > > > Add xfs_break_two_dax_layouts() to break layout for tow dax files. > > Then call compare range function only when files are both DAX or not. > > > > Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com> > > > Not family with xfs code but reading code make my sleep better :) See bellow. > > > --- > > fs/xfs/xfs_file.c | 20 ++++++++++++++++++++ > > fs/xfs/xfs_inode.c | 8 +++++++- > > fs/xfs/xfs_inode.h | 1 + > > fs/xfs/xfs_reflink.c | 5 +++-- > > 4 files changed, 31 insertions(+), 3 deletions(-) > > > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index > > 5795d5d6f869..1fd457167c12 100644 > > --- a/fs/xfs/xfs_file.c > > +++ b/fs/xfs/xfs_file.c > > @@ -842,6 +842,26 @@ xfs_break_dax_layouts( > > 0, 0, xfs_wait_dax_page(inode)); > > } > > > > +int > > +xfs_break_two_dax_layouts( > > + struct inode *src, > > + struct inode *dest) > > +{ > > + int error; > > + bool retry = false; > > + > > +retry: > > > 'retry = false;' ? since xfs_break_dax_layouts() won't set retry to false if there is > no busy page in inode->i_mapping. > Dead loop will happen if retry is true once. Yes, I should move 'retry=false;' under the retry label. > > > + error = xfs_break_dax_layouts(src, &retry); > > + if (error || retry) > > + goto retry; > > + > > + error = xfs_break_dax_layouts(dest, &retry); > > + if (error || retry) > > + goto retry; > > + > > + return error; > > +} > > + > > int > > xfs_break_layouts( > > struct inode *inode, > > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index > > f93370bd7b1e..c01786917eef 100644 > > --- a/fs/xfs/xfs_inode.c > > +++ b/fs/xfs/xfs_inode.c > > @@ -3713,8 +3713,10 @@ xfs_ilock2_io_mmap( > > struct xfs_inode *ip2) > > { > > int ret; > > + struct inode *inode1 = VFS_I(ip1); > > + struct inode *inode2 = VFS_I(ip2); > > > > - ret = xfs_iolock_two_inodes_and_break_layout(VFS_I(ip1), > > VFS_I(ip2)); > > + ret = xfs_iolock_two_inodes_and_break_layout(inode1, inode2); > > if (ret) > > return ret; > > if (ip1 == ip2) > > @@ -3722,6 +3724,10 @@ xfs_ilock2_io_mmap( > > else > > xfs_lock_two_inodes(ip1, XFS_MMAPLOCK_EXCL, > > ip2, XFS_MMAPLOCK_EXCL); > > + > > + if (IS_DAX(inode1) && IS_DAX(inode2)) > > + ret = xfs_break_two_dax_layouts(inode1, inode2); > > + > ret is ignored here. Thanks, it's my mistake. -- Thanks, Ruan Shiyang. > > -- > Su > > return 0; > > } > > > > diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h index > > 88ee4c3930ae..5ef21924dddc 100644 > > --- a/fs/xfs/xfs_inode.h > > +++ b/fs/xfs/xfs_inode.h > > @@ -435,6 +435,7 @@ enum xfs_prealloc_flags { > > > > int xfs_update_prealloc_flags(struct xfs_inode *ip, > > enum xfs_prealloc_flags flags); > > +int xfs_break_two_dax_layouts(struct inode *inode1, struct > > inode *inode2); > > int xfs_break_layouts(struct inode *inode, uint *iolock, > > enum layout_break_reason reason); > > > > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c index > > a4cd6e8a7aa0..4426bcc8a985 100644 > > --- a/fs/xfs/xfs_reflink.c > > +++ b/fs/xfs/xfs_reflink.c > > @@ -29,6 +29,7 @@ > > #include "xfs_iomap.h" > > #include "xfs_sb.h" > > #include "xfs_ag_resv.h" > > +#include <linux/dax.h> > > > > /* > > * Copy on Write of Shared Blocks > > @@ -1324,8 +1325,8 @@ xfs_reflink_remap_prep( > > if (XFS_IS_REALTIME_INODE(src) || XFS_IS_REALTIME_INODE(dest)) > > goto out_unlock; > > > > - /* Don't share DAX file data for now. */ > > - if (IS_DAX(inode_in) || IS_DAX(inode_out)) > > + /* Don't share DAX file data with non-DAX file. */ > > + if (IS_DAX(inode_in) != IS_DAX(inode_out)) > > goto out_unlock; > > > > if (!IS_DAX(inode_in)) _______________________________________________ Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
WARNING: multiple messages have this Message-ID (diff)
From: "ruansy.fnst@fujitsu.com" <ruansy.fnst@fujitsu.com> To: Su Yue <l@damenly.su> Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-xfs@vger.kernel.org" <linux-xfs@vger.kernel.org>, "linux-nvdimm@lists.01.org" <linux-nvdimm@lists.01.org>, "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>, "darrick.wong@oracle.com" <darrick.wong@oracle.com>, "dan.j.williams@intel.com" <dan.j.williams@intel.com>, "willy@infradead.org" <willy@infradead.org>, "jack@suse.cz" <jack@suse.cz>, "viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>, "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>, "david@fromorbit.com" <david@fromorbit.com>, "hch@lst.de" <hch@lst.de>, "rgoldwyn@suse.de" <rgoldwyn@suse.de> Subject: RE: [PATCH v4 7/7] fs/xfs: Add dedupe support for fsdax Date: Fri, 9 Apr 2021 01:56:48 +0000 [thread overview] Message-ID: <OSAPR01MB291345BD169337AE2BCAA717F4739@OSAPR01MB2913.jpnprd01.prod.outlook.com> (raw) In-Reply-To: <czv4syut.fsf@damenly.su> > -----Original Message----- > From: Su Yue <l@damenly.su> > Subject: Re: [PATCH v4 7/7] fs/xfs: Add dedupe support for fsdax > > > On Thu 08 Apr 2021 at 20:04, Shiyang Ruan <ruansy.fnst@fujitsu.com> wrote: > > > Add xfs_break_two_dax_layouts() to break layout for tow dax files. > > Then call compare range function only when files are both DAX or not. > > > > Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com> > > > Not family with xfs code but reading code make my sleep better :) See bellow. > > > --- > > fs/xfs/xfs_file.c | 20 ++++++++++++++++++++ > > fs/xfs/xfs_inode.c | 8 +++++++- > > fs/xfs/xfs_inode.h | 1 + > > fs/xfs/xfs_reflink.c | 5 +++-- > > 4 files changed, 31 insertions(+), 3 deletions(-) > > > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index > > 5795d5d6f869..1fd457167c12 100644 > > --- a/fs/xfs/xfs_file.c > > +++ b/fs/xfs/xfs_file.c > > @@ -842,6 +842,26 @@ xfs_break_dax_layouts( > > 0, 0, xfs_wait_dax_page(inode)); > > } > > > > +int > > +xfs_break_two_dax_layouts( > > + struct inode *src, > > + struct inode *dest) > > +{ > > + int error; > > + bool retry = false; > > + > > +retry: > > > 'retry = false;' ? since xfs_break_dax_layouts() won't set retry to false if there is > no busy page in inode->i_mapping. > Dead loop will happen if retry is true once. Yes, I should move 'retry=false;' under the retry label. > > > + error = xfs_break_dax_layouts(src, &retry); > > + if (error || retry) > > + goto retry; > > + > > + error = xfs_break_dax_layouts(dest, &retry); > > + if (error || retry) > > + goto retry; > > + > > + return error; > > +} > > + > > int > > xfs_break_layouts( > > struct inode *inode, > > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c index > > f93370bd7b1e..c01786917eef 100644 > > --- a/fs/xfs/xfs_inode.c > > +++ b/fs/xfs/xfs_inode.c > > @@ -3713,8 +3713,10 @@ xfs_ilock2_io_mmap( > > struct xfs_inode *ip2) > > { > > int ret; > > + struct inode *inode1 = VFS_I(ip1); > > + struct inode *inode2 = VFS_I(ip2); > > > > - ret = xfs_iolock_two_inodes_and_break_layout(VFS_I(ip1), > > VFS_I(ip2)); > > + ret = xfs_iolock_two_inodes_and_break_layout(inode1, inode2); > > if (ret) > > return ret; > > if (ip1 == ip2) > > @@ -3722,6 +3724,10 @@ xfs_ilock2_io_mmap( > > else > > xfs_lock_two_inodes(ip1, XFS_MMAPLOCK_EXCL, > > ip2, XFS_MMAPLOCK_EXCL); > > + > > + if (IS_DAX(inode1) && IS_DAX(inode2)) > > + ret = xfs_break_two_dax_layouts(inode1, inode2); > > + > ret is ignored here. Thanks, it's my mistake. -- Thanks, Ruan Shiyang. > > -- > Su > > return 0; > > } > > > > diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h index > > 88ee4c3930ae..5ef21924dddc 100644 > > --- a/fs/xfs/xfs_inode.h > > +++ b/fs/xfs/xfs_inode.h > > @@ -435,6 +435,7 @@ enum xfs_prealloc_flags { > > > > int xfs_update_prealloc_flags(struct xfs_inode *ip, > > enum xfs_prealloc_flags flags); > > +int xfs_break_two_dax_layouts(struct inode *inode1, struct > > inode *inode2); > > int xfs_break_layouts(struct inode *inode, uint *iolock, > > enum layout_break_reason reason); > > > > diff --git a/fs/xfs/xfs_reflink.c b/fs/xfs/xfs_reflink.c index > > a4cd6e8a7aa0..4426bcc8a985 100644 > > --- a/fs/xfs/xfs_reflink.c > > +++ b/fs/xfs/xfs_reflink.c > > @@ -29,6 +29,7 @@ > > #include "xfs_iomap.h" > > #include "xfs_sb.h" > > #include "xfs_ag_resv.h" > > +#include <linux/dax.h> > > > > /* > > * Copy on Write of Shared Blocks > > @@ -1324,8 +1325,8 @@ xfs_reflink_remap_prep( > > if (XFS_IS_REALTIME_INODE(src) || XFS_IS_REALTIME_INODE(dest)) > > goto out_unlock; > > > > - /* Don't share DAX file data for now. */ > > - if (IS_DAX(inode_in) || IS_DAX(inode_out)) > > + /* Don't share DAX file data with non-DAX file. */ > > + if (IS_DAX(inode_in) != IS_DAX(inode_out)) > > goto out_unlock; > > > > if (!IS_DAX(inode_in))
next prev parent reply other threads:[~2021-04-09 1:57 UTC|newest] Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-04-08 12:04 [PATCH v4 0/7] fsdax,xfs: Add reflink&dedupe support for fsdax Shiyang Ruan 2021-04-08 12:04 ` Shiyang Ruan 2021-04-08 12:04 ` [PATCH v4 1/7] fsdax: Introduce dax_iomap_cow_copy() Shiyang Ruan 2021-04-08 12:04 ` Shiyang Ruan 2021-04-08 21:53 ` Darrick J. Wong 2021-04-08 21:53 ` Darrick J. Wong 2021-04-09 2:30 ` ruansy.fnst 2021-04-09 2:30 ` ruansy.fnst 2021-04-08 12:04 ` [PATCH v4 2/7] fsdax: Replace mmap entry in case of CoW Shiyang Ruan 2021-04-08 12:04 ` Shiyang Ruan 2021-04-08 22:11 ` Darrick J. Wong 2021-04-08 22:11 ` Darrick J. Wong 2021-04-08 12:04 ` [PATCH v4 3/7] fsdax: Add dax_iomap_cow_copy() for dax_iomap_zero Shiyang Ruan 2021-04-08 12:04 ` Shiyang Ruan 2021-04-08 12:04 ` [PATCH v4 4/7] iomap: Introduce iomap_apply2() for operations on two files Shiyang Ruan 2021-04-08 12:04 ` Shiyang Ruan 2021-04-08 22:26 ` Darrick J. Wong 2021-04-08 22:26 ` Darrick J. Wong 2021-04-08 12:04 ` [PATCH v4 5/7] fsdax: Dedup file range to use a compare function Shiyang Ruan 2021-04-08 12:04 ` Shiyang Ruan 2021-04-08 22:33 ` Darrick J. Wong 2021-04-08 22:33 ` Darrick J. Wong 2021-04-08 12:04 ` [PATCH v4 6/7] fs/xfs: Handle CoW for fsdax write() path Shiyang Ruan 2021-04-08 12:04 ` Shiyang Ruan 2021-04-08 22:40 ` Darrick J. Wong 2021-04-08 22:40 ` Darrick J. Wong 2021-04-08 12:04 ` [PATCH v4 7/7] fs/xfs: Add dedupe support for fsdax Shiyang Ruan 2021-04-08 12:04 ` Shiyang Ruan 2021-04-08 13:12 ` Su Yue 2021-04-08 13:12 ` Su Yue 2021-04-09 1:56 ` ruansy.fnst [this message] 2021-04-09 1:56 ` ruansy.fnst 2021-04-08 23:04 ` Darrick J. Wong 2021-04-08 23:04 ` Darrick J. Wong
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=OSAPR01MB291345BD169337AE2BCAA717F4739@OSAPR01MB2913.jpnprd01.prod.outlook.com \ --to=ruansy.fnst@fujitsu.com \ --cc=darrick.wong@oracle.com \ --cc=david@fromorbit.com \ --cc=hch@lst.de \ --cc=jack@suse.cz \ --cc=l@damenly.su \ --cc=linux-btrfs@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-nvdimm@lists.01.org \ --cc=linux-xfs@vger.kernel.org \ --cc=rgoldwyn@suse.de \ --cc=viro@zeniv.linux.org.uk \ --cc=willy@infradead.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.