From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS, UNPARSEABLE_RELAY,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9A593C11F69 for ; Wed, 30 Jun 2021 16:06:32 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7DF196141A for ; Wed, 30 Jun 2021 16:06:32 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231342AbhF3QJA (ORCPT ); Wed, 30 Jun 2021 12:09:00 -0400 Received: from smtp-out2.suse.de ([195.135.220.29]:41580 "EHLO smtp-out2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229753AbhF3QI6 (ORCPT ); Wed, 30 Jun 2021 12:08:58 -0400 Received: from imap.suse.de (imap-alt.suse-dmz.suse.de [192.168.254.47]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id A088B20483; Wed, 30 Jun 2021 16:06:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1625069188; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=R+/U4JehVX0FhkBlOkfzWo0x8lCwrBaeOESsSdyk28Y=; b=Rjn+2hiBLWwyjfFNPxIHwptzno1cJItH37TEtJuQ8SvCUwv/dPVYwPrLNtPJ8zH+RmmBRT Hfr9wvdi+cCUR8+PUehJ30loQWlQ/tiNXO/is7shMoXM+T9V4u1KBtsSvmnynRrsAryjIH HdvzUCXxB9cU/hmnz19T2cZTJ+ATZVA= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1625069188; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=R+/U4JehVX0FhkBlOkfzWo0x8lCwrBaeOESsSdyk28Y=; b=s6m1jYejOk8yfoCaOzESlebrkrRGXExFJI4rTp5M34pM7Y3gmd9fgSqL9E2txte6IAu9+7 qMlYMdhuVZ3kMIAw== Received: from imap3-int (imap-alt.suse-dmz.suse.de [192.168.254.47]) by imap.suse.de (Postfix) with ESMTP id 1AA4D118DD; Wed, 30 Jun 2021 16:06:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1625069188; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=R+/U4JehVX0FhkBlOkfzWo0x8lCwrBaeOESsSdyk28Y=; b=Rjn+2hiBLWwyjfFNPxIHwptzno1cJItH37TEtJuQ8SvCUwv/dPVYwPrLNtPJ8zH+RmmBRT Hfr9wvdi+cCUR8+PUehJ30loQWlQ/tiNXO/is7shMoXM+T9V4u1KBtsSvmnynRrsAryjIH HdvzUCXxB9cU/hmnz19T2cZTJ+ATZVA= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1625069188; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=R+/U4JehVX0FhkBlOkfzWo0x8lCwrBaeOESsSdyk28Y=; b=s6m1jYejOk8yfoCaOzESlebrkrRGXExFJI4rTp5M34pM7Y3gmd9fgSqL9E2txte6IAu9+7 qMlYMdhuVZ3kMIAw== Received: from director2.suse.de ([192.168.254.72]) by imap3-int with ESMTPSA id Uui7A4SW3GDZWAAALh3uQQ (envelope-from ); Wed, 30 Jun 2021 16:06:28 +0000 Received: from localhost (brahms [local]) by brahms (OpenSMTPD) with ESMTPA id 578c595a; Wed, 30 Jun 2021 16:06:27 +0000 (UTC) Date: Wed, 30 Jun 2021 17:06:27 +0100 From: Luis Henriques To: Amir Goldstein Cc: kernel test robot , 0day robot , Nicolas Boichat , Olga Kornievskaia , LKML , lkp@lists.01.org, Alexander Viro , linux-fsdevel Subject: Re: [vfs] 94a4dd06a6: xfstests.generic.263.fail Message-ID: References: <20210513135644.GE20142@xsang-OptiPlex-9020> <877dk1zibo.fsf@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Jun 30, 2021 at 06:46:22PM +0300, Amir Goldstein wrote: > On Fri, May 14, 2021 at 2:03 PM Luis Henriques wrote: > > > > kernel test robot writes: > > > > > Greeting, > > > > > > FYI, we noticed the following commit (built with gcc-9): > > > > > > commit: 94a4dd06a6bbf3978b0bb1dddc2d8ec4e5bcad26 ("[PATCH v9] vfs: fix copy_file_range regression in cross-fs copies") > > > url: https://github.com/0day-ci/linux/commits/Luis-Henriques/vfs-fix-copy_file_range-regression-in-cross-fs-copies/20210510-170804 > > > base: https://git.kernel.org/cgit/linux/kernel/git/viro/vfs.git for-next > > > > > > in testcase: xfstests > > > version: xfstests-x86_64-73c0871-1_20210401 > > > with following parameters: > > > > > > disk: 4HDD > > > fs: xfs > > > test: generic-group-13 > > > ucode: 0x21 > > > > > > test-description: xfstests is a regression test suite for xfs and other files ystems. > > > test-url: git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git > > > > > > > > > on test machine: 4 threads 1 sockets Intel(R) Core(TM) i3-3220 CPU @ 3.30GHz with 8G memory > > > > > > caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace): > > > > > > > > > > > > > > > If you fix the issue, kindly add following tag > > > Reported-by: kernel test robot > > > > > > 2021-05-11 11:28:23 export TEST_DIR=/fs/sda1 > > > 2021-05-11 11:28:23 export TEST_DEV=/dev/sda1 > > > 2021-05-11 11:28:23 export FSTYP=xfs > > > 2021-05-11 11:28:23 export SCRATCH_MNT=/fs/scratch > > > 2021-05-11 11:28:23 mkdir /fs/scratch -p > > > 2021-05-11 11:28:23 export SCRATCH_DEV=/dev/sda4 > > > 2021-05-11 11:28:23 export SCRATCH_LOGDEV=/dev/sda2 > > > 2021-05-11 11:28:23 sed "s:^:generic/:" //lkp/benchmarks/xfstests/tests/generic-group-13 > > > 2021-05-11 11:28:23 ./check generic/260 generic/261 generic/262 generic/263 generic/264 generic/265 generic/266 generic/267 generic/268 generic/269 generic/270 generic/271 generic/272 generic/273 generic/274 generic/275 generic/276 generic/277 generic/278 generic/279 > > > FSTYP -- xfs (debug) > > > PLATFORM -- Linux/x86_64 lkp-ivb-d02 5.12.0-rc6-00061-g94a4dd06a6bb #1 SMP Tue May 11 00:58:17 CST 2021 > > > MKFS_OPTIONS -- -f -bsize=4096 /dev/sda4 > > > MOUNT_OPTIONS -- /dev/sda4 /fs/scratch > > > > > > generic/260 [not run] FITRIM not supported on /fs/scratch > > > generic/261 [not run] Reflink not supported by scratch filesystem type: xfs > > > generic/262 [not run] Reflink not supported by scratch filesystem type: xfs > > > generic/263 [failed, exit status 1]- output mismatch (see /lkp/benchmarks/xfstests/results//generic/263.out.bad) > > > --- tests/generic/263.out 2021-04-01 03:07:08.000000000 +0000 > > > +++ /lkp/benchmarks/xfstests/results//generic/263.out.bad 2021-05-11 11:28:29.773460096 +0000 > > > @@ -1,3 +1,32 @@ > > > QA output created by 263 > > > fsx -N 10000 -o 8192 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z > > > -fsx -N 10000 -o 128000 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z > > > +Seed set to 1 > > > +main: filesystem does not support clone range, disabling! > > > +main: filesystem does not support dedupe range, disabling! > > > +skipping zero size read > > > ... > > > (Run 'diff -u /lkp/benchmarks/xfstests/tests/generic/263.out /lkp/benchmarks/xfstests/results//generic/263.out.bad' to see the entire diff) > > > generic/264 [not run] Reflink not supported by scratch filesystem type: xfs > > > generic/265 [not run] Reflink not supported by scratch filesystem type: xfs > > > generic/266 [not run] Reflink not supported by scratch filesystem type: xfs > > > generic/267 [not run] Reflink not supported by scratch filesystem type: xfs > > > generic/268 [not run] Reflink not supported by scratch filesystem type: xfs > > > generic/269 48s > > > generic/270 61s > > > generic/271 [not run] Reflink not supported by scratch filesystem type: xfs > > > generic/272 [not run] Reflink not supported by scratch filesystem type: xfs > > > generic/273 17s > > > generic/274 14s > > > generic/275 11s > > > generic/276 [not run] Reflink not supported by scratch filesystem type: xfs > > > generic/277 3s > > > generic/278 [not run] Reflink not supported by scratch filesystem type: xfs > > > generic/279 [not run] Reflink not supported by scratch filesystem type: xfs > > > Ran: generic/260 generic/261 generic/262 generic/263 generic/264 generic/265 generic/266 generic/267 generic/268 generic/269 generic/270 generic/271 generic/272 generic/273 generic/274 generic/275 generic/276 generic/277 generic/278 generic/279 > > > Not run: generic/260 generic/261 generic/262 generic/264 generic/265 generic/266 generic/267 generic/268 generic/271 generic/272 generic/276 generic/278 generic/279 > > > Failures: generic/263 > > > Failed 1 of 20 tests > > > > OK, I see what's going on. There are 2 issues: one with patch and another > > one with the test itself. > > > > The CFR syscall should have been disabled in this test but it isn't > > because the test tries to copy 1 byte from a zero-sized file: > > > > int > > test_copy_range(void) > > { > > loff_t o1 = 0, o2 = 1; > > > > if (syscall(__NR_copy_file_range, fd, &o1, fd, &o2, 1, 0) == -1 && > > (errno == ENOSYS || errno == EOPNOTSUPP || errno == ENOTTY)) { > > if (!quiet) > > fprintf(stderr, > > "main: filesystem does not support " > > "copy range, disabling!\n"); > > return 0; > > } > > > > return 1; > > } > > > > The syscall is doing an early '0' return because the file size is < len. > > > > Fixing the kernel should probably be as easy as removing the > > short-circuiting check in vfs_copy_file_range(): > > > > if (len == 0) > > return 0; > > > > This will force the filesystems code to handle '0' size copies but will > > also make sure -EOPNOTSUPP is returned in this case. > > > > Sorry for the late reply. > The solution above is correct. > That is aligned with the behavior of vfs_clone_file_range(). > Need to call into the filesystem method also with 0 length > in order to learn about CFR support of this filesystem instance. Yep, this makes sense (I've seen you're detailed explanation in the other thread -- thanks!). I'll send out v11 in a sec. Cheers, -- Luís > > Alternatively, we could have something like: > > > > if (len == 0) { > > if (file_out->f_op->copy_file_range) > > return 0; > > else > > return -EOPNOTSUPP; > > } > > > > This does not catch the case of a filesystem driver that has > CFR method but a filesystem instance does not support CFR. > For example, overlayfs with ext4 as upper fs. > > > What do you guys think is the right thing to do? > > > > Additionally, the test should also be fixed with something as the patch > > bellow. By making sure we have 1 byte to copy we also ensure the syscall > > will return -EOPNOTSUPP, even with the current version of the patch. > > > > I don't think that the test should be fixed. > > Thanks, > Amir. > > > Cheers, > > -- > > Luis > > > > diff --git a/ltp/fsx.c b/ltp/fsx.c > > index cd0bae55aeb8..97db594ae142 100644 > > --- a/ltp/fsx.c > > +++ b/ltp/fsx.c > > @@ -1596,6 +1596,10 @@ int > > test_copy_range(void) > > { > > loff_t o1 = 0, o2 = 1; > > + int ret = 1; > > + > > + /* Make sure we have 1 byte to copy */ > > + ftruncate(fd, 1); > > > > if (syscall(__NR_copy_file_range, fd, &o1, fd, &o2, 1, 0) == -1 && > > (errno == ENOSYS || errno == EOPNOTSUPP || errno == ENOTTY)) { > > @@ -1603,10 +1607,13 @@ test_copy_range(void) > > fprintf(stderr, > > "main: filesystem does not support " > > "copy range, disabling!\n"); > > - return 0; > > + ret = 0; > > } > > > > - return 1; > > + /* Restore file size */ > > + ftruncate(fd, 0); > > + > > + return ret; > > } > > > > void From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============0712046615800600117==" MIME-Version: 1.0 From: Luis Henriques To: lkp@lists.01.org Subject: Re: [vfs] 94a4dd06a6: xfstests.generic.263.fail Date: Wed, 30 Jun 2021 17:06:27 +0100 Message-ID: In-Reply-To: List-Id: --===============0712046615800600117== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable On Wed, Jun 30, 2021 at 06:46:22PM +0300, Amir Goldstein wrote: > On Fri, May 14, 2021 at 2:03 PM Luis Henriques wro= te: > > > > kernel test robot writes: > > > > > Greeting, > > > > > > FYI, we noticed the following commit (built with gcc-9): > > > > > > commit: 94a4dd06a6bbf3978b0bb1dddc2d8ec4e5bcad26 ("[PATCH v9] vfs: fi= x copy_file_range regression in cross-fs copies") > > > url: https://github.com/0day-ci/linux/commits/Luis-Henriques/vfs-fix-= copy_file_range-regression-in-cross-fs-copies/20210510-170804 > > > base: https://git.kernel.org/cgit/linux/kernel/git/viro/vfs.git for-n= ext > > > > > > in testcase: xfstests > > > version: xfstests-x86_64-73c0871-1_20210401 > > > with following parameters: > > > > > > disk: 4HDD > > > fs: xfs > > > test: generic-group-13 > > > ucode: 0x21 > > > > > > test-description: xfstests is a regression test suite for xfs and oth= er files ystems. > > > test-url: git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git > > > > > > > > > on test machine: 4 threads 1 sockets Intel(R) Core(TM) i3-3220 CPU @ = 3.30GHz with 8G memory > > > > > > caused below changes (please refer to attached dmesg/kmsg for entire = log/backtrace): > > > > > > > > > > > > > > > If you fix the issue, kindly add following tag > > > Reported-by: kernel test robot > > > > > > 2021-05-11 11:28:23 export TEST_DIR=3D/fs/sda1 > > > 2021-05-11 11:28:23 export TEST_DEV=3D/dev/sda1 > > > 2021-05-11 11:28:23 export FSTYP=3Dxfs > > > 2021-05-11 11:28:23 export SCRATCH_MNT=3D/fs/scratch > > > 2021-05-11 11:28:23 mkdir /fs/scratch -p > > > 2021-05-11 11:28:23 export SCRATCH_DEV=3D/dev/sda4 > > > 2021-05-11 11:28:23 export SCRATCH_LOGDEV=3D/dev/sda2 > > > 2021-05-11 11:28:23 sed "s:^:generic/:" //lkp/benchmarks/xfstests/tes= ts/generic-group-13 > > > 2021-05-11 11:28:23 ./check generic/260 generic/261 generic/262 gener= ic/263 generic/264 generic/265 generic/266 generic/267 generic/268 generic/= 269 generic/270 generic/271 generic/272 generic/273 generic/274 generic/275= generic/276 generic/277 generic/278 generic/279 > > > FSTYP -- xfs (debug) > > > PLATFORM -- Linux/x86_64 lkp-ivb-d02 5.12.0-rc6-00061-g94a4dd06a= 6bb #1 SMP Tue May 11 00:58:17 CST 2021 > > > MKFS_OPTIONS -- -f -bsize=3D4096 /dev/sda4 > > > MOUNT_OPTIONS -- /dev/sda4 /fs/scratch > > > > > > generic/260 [not run] FITRIM not supported on /fs/scratch > > > generic/261 [not run] Reflink not supported by scratch filesystem t= ype: xfs > > > generic/262 [not run] Reflink not supported by scratch filesystem t= ype: xfs > > > generic/263 [failed, exit status 1]- output mismatch (see /lkp/benc= hmarks/xfstests/results//generic/263.out.bad) > > > --- tests/generic/263.out 2021-04-01 03:07:08.000000000 +0000 > > > +++ /lkp/benchmarks/xfstests/results//generic/263.out.bad 2021-05= -11 11:28:29.773460096 +0000 > > > @@ -1,3 +1,32 @@ > > > QA output created by 263 > > > fsx -N 10000 -o 8192 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z > > > -fsx -N 10000 -o 128000 -l 500000 -r PSIZE -t BSIZE -w BSIZE -Z > > > +Seed set to 1 > > > +main: filesystem does not support clone range, disabling! > > > +main: filesystem does not support dedupe range, disabling! > > > +skipping zero size read > > > ... > > > (Run 'diff -u /lkp/benchmarks/xfstests/tests/generic/263.out /lkp= /benchmarks/xfstests/results//generic/263.out.bad' to see the entire diff) > > > generic/264 [not run] Reflink not supported by scratch filesystem t= ype: xfs > > > generic/265 [not run] Reflink not supported by scratch filesystem t= ype: xfs > > > generic/266 [not run] Reflink not supported by scratch filesystem t= ype: xfs > > > generic/267 [not run] Reflink not supported by scratch filesystem t= ype: xfs > > > generic/268 [not run] Reflink not supported by scratch filesystem t= ype: xfs > > > generic/269 48s > > > generic/270 61s > > > generic/271 [not run] Reflink not supported by scratch filesystem t= ype: xfs > > > generic/272 [not run] Reflink not supported by scratch filesystem t= ype: xfs > > > generic/273 17s > > > generic/274 14s > > > generic/275 11s > > > generic/276 [not run] Reflink not supported by scratch filesystem t= ype: xfs > > > generic/277 3s > > > generic/278 [not run] Reflink not supported by scratch filesystem t= ype: xfs > > > generic/279 [not run] Reflink not supported by scratch filesystem t= ype: xfs > > > Ran: generic/260 generic/261 generic/262 generic/263 generic/264 gene= ric/265 generic/266 generic/267 generic/268 generic/269 generic/270 generic= /271 generic/272 generic/273 generic/274 generic/275 generic/276 generic/27= 7 generic/278 generic/279 > > > Not run: generic/260 generic/261 generic/262 generic/264 generic/265 = generic/266 generic/267 generic/268 generic/271 generic/272 generic/276 gen= eric/278 generic/279 > > > Failures: generic/263 > > > Failed 1 of 20 tests > > > > OK, I see what's going on. There are 2 issues: one with patch and anot= her > > one with the test itself. > > > > The CFR syscall should have been disabled in this test but it isn't > > because the test tries to copy 1 byte from a zero-sized file: > > > > int > > test_copy_range(void) > > { > > loff_t o1 =3D 0, o2 =3D 1; > > > > if (syscall(__NR_copy_file_range, fd, &o1, fd, &o2, 1, 0) =3D= =3D -1 && > > (errno =3D=3D ENOSYS || errno =3D=3D EOPNOTSUPP || errno = =3D=3D ENOTTY)) { > > if (!quiet) > > fprintf(stderr, > > "main: filesystem does not support " > > "copy range, disabling!\n"); > > return 0; > > } > > > > return 1; > > } > > > > The syscall is doing an early '0' return because the file size is < len. > > > > Fixing the kernel should probably be as easy as removing the > > short-circuiting check in vfs_copy_file_range(): > > > > if (len =3D=3D 0) > > return 0; > > > > This will force the filesystems code to handle '0' size copies but will > > also make sure -EOPNOTSUPP is returned in this case. > > > = > Sorry for the late reply. > The solution above is correct. > That is aligned with the behavior of vfs_clone_file_range(). > Need to call into the filesystem method also with 0 length > in order to learn about CFR support of this filesystem instance. Yep, this makes sense (I've seen you're detailed explanation in the other thread -- thanks!). I'll send out v11 in a sec. Cheers, -- Lu=C3=ADs > > Alternatively, we could have something like: > > > > if (len =3D=3D 0) { > > if (file_out->f_op->copy_file_range) > > return 0; > > else > > return -EOPNOTSUPP; > > } > > > = > This does not catch the case of a filesystem driver that has > CFR method but a filesystem instance does not support CFR. > For example, overlayfs with ext4 as upper fs. > = > > What do you guys think is the right thing to do? > > > > Additionally, the test should also be fixed with something as the patch > > bellow. By making sure we have 1 byte to copy we also ensure the sysca= ll > > will return -EOPNOTSUPP, even with the current version of the patch. > > > = > I don't think that the test should be fixed. > = > Thanks, > Amir. > = > > Cheers, > > -- > > Luis > > > > diff --git a/ltp/fsx.c b/ltp/fsx.c > > index cd0bae55aeb8..97db594ae142 100644 > > --- a/ltp/fsx.c > > +++ b/ltp/fsx.c > > @@ -1596,6 +1596,10 @@ int > > test_copy_range(void) > > { > > loff_t o1 =3D 0, o2 =3D 1; > > + int ret =3D 1; > > + > > + /* Make sure we have 1 byte to copy */ > > + ftruncate(fd, 1); > > > > if (syscall(__NR_copy_file_range, fd, &o1, fd, &o2, 1, 0) =3D= =3D -1 && > > (errno =3D=3D ENOSYS || errno =3D=3D EOPNOTSUPP || errno = =3D=3D ENOTTY)) { > > @@ -1603,10 +1607,13 @@ test_copy_range(void) > > fprintf(stderr, > > "main: filesystem does not support " > > "copy range, disabling!\n"); > > - return 0; > > + ret =3D 0; > > } > > > > - return 1; > > + /* Restore file size */ > > + ftruncate(fd, 0); > > + > > + return ret; > > } > > > > void --===============0712046615800600117==--