| Message ID | 20180418025755.GN24738@magnolia (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, Apr 18, 2018 at 5:57 AM, Darrick J. Wong <darrick.wong@oracle.com> wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Since deduplication potentially has to read in all the pages in both > files in order to compare the contents, cap the deduplication request > length at MAX_RW_COUNT/2 (roughly 1GB) so that we have /some/ upper bound > on the request length and can't just lock up the kernel forever. Found > by running generic/304 after commit 1ddae54555b62a ("common/rc: add > missing 'local' keywords"). > With this patch generic/304 doesn't soft lock the kernel, but it doesn't seem 'quick' at all. I stopped waiting after 5 minutes. Is that expected? If so, best get the test out of the quick group. Can you shed some light on how the "missing 'local' keywords" change exposed this behavior? Thanks, Amir. -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Apr 22, 2018 at 05:57:32AM +0300, Amir Goldstein wrote: > On Wed, Apr 18, 2018 at 5:57 AM, Darrick J. Wong > <darrick.wong@oracle.com> wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > Since deduplication potentially has to read in all the pages in both > > files in order to compare the contents, cap the deduplication request > > length at MAX_RW_COUNT/2 (roughly 1GB) so that we have /some/ upper bound > > on the request length and can't just lock up the kernel forever. Found > > by running generic/304 after commit 1ddae54555b62a ("common/rc: add > > missing 'local' keywords"). > > > > With this patch generic/304 doesn't soft lock the kernel, but it doesn't > seem 'quick' at all. I stopped waiting after 5 minutes. Is that expected? > If so, best get the test out of the quick group. No, that is incorrect behavior; the test is buggy. > Can you shed some light on how the "missing 'local' keywords" change > exposed this behavior? See the patch I sent to the fstests lists to fix generic/304. --D > Thanks, > Amir. > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 17, 2018 at 07:57:55PM -0700, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > Since deduplication potentially has to read in all the pages in both > files in order to compare the contents, cap the deduplication request > length at MAX_RW_COUNT/2 (roughly 1GB) so that we have /some/ upper bound > on the request length and can't just lock up the kernel forever. Found > by running generic/304 after commit 1ddae54555b62a ("common/rc: add > missing 'local' keywords"). > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> Looks good, Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com> > --- > v2: halve the amount so that we do MAX_RW_COUNT IO max > --- > fs/xfs/xfs_file.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c > index 299aee4..9cafad4 100644 > --- a/fs/xfs/xfs_file.c > +++ b/fs/xfs/xfs_file.c > @@ -876,8 +876,18 @@ xfs_file_dedupe_range( > struct file *dst_file, > u64 dst_loff) > { > + struct inode *srci = file_inode(src_file); > + u64 max_dedupe; > int error; > > + /* > + * Since we have to read all these pages in to compare them, cut > + * it off at MAX_RW_COUNT/2 rounded down to the nearest block. > + * That means we won't do more than MAX_RW_COUNT IO per request. > + */ > + max_dedupe = (MAX_RW_COUNT >> 1) & ~(i_blocksize(srci) - 1); > + if (len > max_dedupe) > + len = max_dedupe; > error = xfs_reflink_remap_range(src_file, loff, dst_file, dst_loff, > len, true); > if (error) > -- > To unsubscribe from this list: send the line "unsubscribe fstests" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index 299aee4..9cafad4 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -876,8 +876,18 @@ xfs_file_dedupe_range( struct file *dst_file, u64 dst_loff) { + struct inode *srci = file_inode(src_file); + u64 max_dedupe; int error; + /* + * Since we have to read all these pages in to compare them, cut + * it off at MAX_RW_COUNT/2 rounded down to the nearest block. + * That means we won't do more than MAX_RW_COUNT IO per request. + */ + max_dedupe = (MAX_RW_COUNT >> 1) & ~(i_blocksize(srci) - 1); + if (len > max_dedupe) + len = max_dedupe; error = xfs_reflink_remap_range(src_file, loff, dst_file, dst_loff, len, true); if (error)