| Message ID | 1560161596-30156-1-git-send-email-sumit.garg@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v4] syscalls/sync_file_range: add partial file sync test-cases | expand |
Hi Sumit, On Mon, Jun 10, 2019 at 6:14 PM Sumit Garg <sumit.garg@linaro.org> wrote: > Add partial file sync tests as part of sync_file_range02 test-case. > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org> > --- > > Changes in v4: > vfat FS doesn't support sparse files. So handle this via pre-filling the > test file in case of "test3". > > Changes in v3: > 1. Add upper bound check for synced size to device. > 2. Refactor tests for more code reuse. > 3. Add another test to check sync over partial write. > > Changes in v2: > 1. Do full file write instead of partial and test sync partial file. > > .../syscalls/sync_file_range/sync_file_range02.c | 53 > ++++++++++++++++++---- > 1 file changed, 43 insertions(+), 10 deletions(-) > > diff --git a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > index 82d77f7..9454a56 100644 > --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > @@ -22,23 +22,36 @@ > #include "check_sync_file_range.h" > > #define MNTPOINT "mnt_point" > -#define FNAME MNTPOINT"/test" > -#define FILE_SIZE_MB 32 > -#define FILE_SIZE (FILE_SIZE_MB * TST_MB) > +#define FNAME1 MNTPOINT"/test1" > +#define FNAME2 MNTPOINT"/test2" > +#define FNAME3 MNTPOINT"/test3" > +#define FILE_SZ_MB 32 > +#define FILE_SZ (FILE_SZ_MB * TST_MB) > #define MODE 0644 > > -static void verify_sync_file_range(void) > +struct testcase { > + char *fname; > + off64_t sync_off; > + off64_t sync_size; > + size_t exp_sync_size; > + off64_t write_off; > + size_t write_size_mb; > +}; > + > +static void verify_sync_file_range(struct testcase *tc) > { > int fd; > unsigned long written; > > - fd = SAFE_OPEN(FNAME, O_RDWR|O_CREAT, MODE); > + fd = SAFE_OPEN(tc->fname, O_RDWR|O_CREAT, MODE); > + > + lseek(fd, tc->write_off, SEEK_SET); > > tst_dev_bytes_written(tst_device->dev); > > - tst_fill_fd(fd, 0, TST_MB, FILE_SIZE_MB); > + tst_fill_fd(fd, 0, TST_MB, tc->write_size_mb); > > - TEST(sync_file_range(fd, 0, FILE_SIZE, > + TEST(sync_file_range(fd, tc->sync_off, tc->sync_size, > SYNC_FILE_RANGE_WAIT_BEFORE | > SYNC_FILE_RANGE_WRITE | > SYNC_FILE_RANGE_WAIT_AFTER)); > @@ -50,23 +63,43 @@ static void verify_sync_file_range(void) > > SAFE_CLOSE(fd); > > - if (written >= FILE_SIZE) > + if ((written >= tc->exp_sync_size) && > + (written <= (tc->exp_sync_size + tc->exp_sync_size/10))) > tst_res(TPASS, "Test file range synced to device"); > else > - tst_res(TFAIL, "Synced %li, expected %i", written, > FILE_SIZE); > + tst_res(TFAIL, "Synced %li, expected %li", written, > + tc->exp_sync_size); > +} > + > +static struct testcase testcases[] = { > + { FNAME1, 0, FILE_SZ, FILE_SZ, 0, FILE_SZ_MB }, > + { FNAME2, FILE_SZ/4, FILE_SZ/2, FILE_SZ/2, 0, FILE_SZ_MB }, > + { FNAME3, FILE_SZ/4, FILE_SZ/2, FILE_SZ/4, FILE_SZ/2, FILE_SZ_MB/4 > }, > +}; > + > +static void run(unsigned int i) > +{ > + verify_sync_file_range(&testcases[i]); > } > > static void setup(void) > { > if (!check_sync_file_range()) > tst_brk(TCONF, "sync_file_range() not supported"); > Reviewed-by: Li Wang <liwang@redhat.com> That'd be great if we have code comments here, anyway the patch makes sense to me. /* * vfat FS doesn't support sparse files. So handle this via pre-filling the * test file in case of "test3". */ + > + if (!strcmp(tst_device->fs_type, "vfat")) { > + tst_res(TINFO, "Pre-filling file"); > + tst_fill_file(FNAME3, 0, TST_MB, FILE_SZ_MB); > + sync(); > + } > } > > static struct tst_test test = { > + .tcnt = ARRAY_SIZE(testcases), > .needs_root = 1, > .mount_device = 1, > .all_filesystems = 1, > .mntpoint = MNTPOINT, > .setup = setup, > - .test_all = verify_sync_file_range, > + .test = run, > }; > -- > 2.7.4 > > -- Regards, Li Wang <div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small">Hi Sumit,</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jun 10, 2019 at 6:14 PM Sumit Garg <<a href="mailto:sumit.garg@linaro.org">sumit.garg@linaro.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Add partial file sync tests as part of sync_file_range02 test-case.<br> <br> Signed-off-by: Sumit Garg <<a href="mailto:sumit.garg@linaro.org" target="_blank">sumit.garg@linaro.org</a>><br> ---<br> <br> Changes in v4:<br> vfat FS doesn't support sparse files. So handle this via pre-filling the<br> test file in case of "test3".<br> <br> Changes in v3:<br> 1. Add upper bound check for synced size to device.<br> 2. Refactor tests for more code reuse.<br> 3. Add another test to check sync over partial write.<br> <br> Changes in v2:<br> 1. Do full file write instead of partial and test sync partial file.<br> <br> .../syscalls/sync_file_range/sync_file_range02.c | 53 ++++++++++++++++++----<br> 1 file changed, 43 insertions(+), 10 deletions(-)<br> <br> diff --git a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c<br> index 82d77f7..9454a56 100644<br> --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c<br> +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c<br> @@ -22,23 +22,36 @@<br> #include "check_sync_file_range.h"<br> <br> #define MNTPOINT "mnt_point"<br> -#define FNAME MNTPOINT"/test"<br> -#define FILE_SIZE_MB 32<br> -#define FILE_SIZE (FILE_SIZE_MB * TST_MB)<br> +#define FNAME1 MNTPOINT"/test1"<br> +#define FNAME2 MNTPOINT"/test2"<br> +#define FNAME3 MNTPOINT"/test3"<br> +#define FILE_SZ_MB 32<br> +#define FILE_SZ (FILE_SZ_MB * TST_MB)<br> #define MODE 0644<br> <br> -static void verify_sync_file_range(void)<br> +struct testcase {<br> + char *fname;<br> + off64_t sync_off;<br> + off64_t sync_size;<br> + size_t exp_sync_size;<br> + off64_t write_off;<br> + size_t write_size_mb;<br> +};<br> +<br> +static void verify_sync_file_range(struct testcase *tc)<br> {<br> int fd;<br> unsigned long written;<br> <br> - fd = SAFE_OPEN(FNAME, O_RDWR|O_CREAT, MODE);<br> + fd = SAFE_OPEN(tc->fname, O_RDWR|O_CREAT, MODE);<br> +<br> + lseek(fd, tc->write_off, SEEK_SET);<br> <br> tst_dev_bytes_written(tst_device->dev);<br> <br> - tst_fill_fd(fd, 0, TST_MB, FILE_SIZE_MB);<br> + tst_fill_fd(fd, 0, TST_MB, tc->write_size_mb);<br> <br> - TEST(sync_file_range(fd, 0, FILE_SIZE,<br> + TEST(sync_file_range(fd, tc->sync_off, tc->sync_size,<br> SYNC_FILE_RANGE_WAIT_BEFORE |<br> SYNC_FILE_RANGE_WRITE |<br> SYNC_FILE_RANGE_WAIT_AFTER));<br> @@ -50,23 +63,43 @@ static void verify_sync_file_range(void)<br> <br> SAFE_CLOSE(fd);<br> <br> - if (written >= FILE_SIZE)<br> + if ((written >= tc->exp_sync_size) &&<br> + (written <= (tc->exp_sync_size + tc->exp_sync_size/10)))<br> tst_res(TPASS, "Test file range synced to device");<br> else<br> - tst_res(TFAIL, "Synced %li, expected %i", written, FILE_SIZE);<br> + tst_res(TFAIL, "Synced %li, expected %li", written,<br> + tc->exp_sync_size);<br> +}<br> +<br> +static struct testcase testcases[] = {<br> + { FNAME1, 0, FILE_SZ, FILE_SZ, 0, FILE_SZ_MB },<br> + { FNAME2, FILE_SZ/4, FILE_SZ/2, FILE_SZ/2, 0, FILE_SZ_MB },<br> + { FNAME3, FILE_SZ/4, FILE_SZ/2, FILE_SZ/4, FILE_SZ/2, FILE_SZ_MB/4 },<br> +};<br> +<br> +static void run(unsigned int i)<br> +{<br> + verify_sync_file_range(&testcases[i]);<br> }<br> <br> static void setup(void)<br> {<br> if (!check_sync_file_range())<br> tst_brk(TCONF, "sync_file_range() not supported");<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Reviewed-by: Li Wang <<a href="mailto:liwang@redhat.com">liwang@redhat.com</a>></div><br></div><div><div class="gmail_default" style="font-size:small">That'd be great if we have code comments here, anyway the patch makes sense to me.</div></div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">/* </div><div class="gmail_default" style="font-size:small"> * vfat FS doesn't support sparse files. So handle this via pre-filling the</div><div class="gmail_default" style="font-size:small"> * test file in case of "test3". </div><div class="gmail_default" style="font-size:small">*/</div><div class="gmail_default" style="font-size:small"><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> +<br> + if (!strcmp(tst_device->fs_type, "vfat")) {<br> + tst_res(TINFO, "Pre-filling file");<br> + tst_fill_file(FNAME3, 0, TST_MB, FILE_SZ_MB);<br> + sync();<br> + }<br> }<br> <br> static struct tst_test test = {<br> + .tcnt = ARRAY_SIZE(testcases),<br> .needs_root = 1,<br> .mount_device = 1,<br> .all_filesystems = 1,<br> .mntpoint = MNTPOINT,<br> .setup = setup,<br> - .test_all = verify_sync_file_range,<br> + .test = run,<br> };<br> -- <br> 2.7.4<br> <br> </blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div>Regards,<br></div><div>Li Wang<br></div></div></div></div>
Hi Cyril, Do you have any further comments on this patch? In case we don't have any further comments, would you like me to send next version with code comments as suggested by Li or they can be incorporated while applying the patch? -Sumit On Mon, 10 Jun 2019 at 17:34, Li Wang <liwang@redhat.com> wrote: > > Hi Sumit, > > On Mon, Jun 10, 2019 at 6:14 PM Sumit Garg <sumit.garg@linaro.org> wrote: >> >> Add partial file sync tests as part of sync_file_range02 test-case. >> >> Signed-off-by: Sumit Garg <sumit.garg@linaro.org> >> --- >> >> Changes in v4: >> vfat FS doesn't support sparse files. So handle this via pre-filling the >> test file in case of "test3". >> >> Changes in v3: >> 1. Add upper bound check for synced size to device. >> 2. Refactor tests for more code reuse. >> 3. Add another test to check sync over partial write. >> >> Changes in v2: >> 1. Do full file write instead of partial and test sync partial file. >> >> .../syscalls/sync_file_range/sync_file_range02.c | 53 ++++++++++++++++++---- >> 1 file changed, 43 insertions(+), 10 deletions(-) >> >> diff --git a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c >> index 82d77f7..9454a56 100644 >> --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c >> +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c >> @@ -22,23 +22,36 @@ >> #include "check_sync_file_range.h" >> >> #define MNTPOINT "mnt_point" >> -#define FNAME MNTPOINT"/test" >> -#define FILE_SIZE_MB 32 >> -#define FILE_SIZE (FILE_SIZE_MB * TST_MB) >> +#define FNAME1 MNTPOINT"/test1" >> +#define FNAME2 MNTPOINT"/test2" >> +#define FNAME3 MNTPOINT"/test3" >> +#define FILE_SZ_MB 32 >> +#define FILE_SZ (FILE_SZ_MB * TST_MB) >> #define MODE 0644 >> >> -static void verify_sync_file_range(void) >> +struct testcase { >> + char *fname; >> + off64_t sync_off; >> + off64_t sync_size; >> + size_t exp_sync_size; >> + off64_t write_off; >> + size_t write_size_mb; >> +}; >> + >> +static void verify_sync_file_range(struct testcase *tc) >> { >> int fd; >> unsigned long written; >> >> - fd = SAFE_OPEN(FNAME, O_RDWR|O_CREAT, MODE); >> + fd = SAFE_OPEN(tc->fname, O_RDWR|O_CREAT, MODE); >> + >> + lseek(fd, tc->write_off, SEEK_SET); >> >> tst_dev_bytes_written(tst_device->dev); >> >> - tst_fill_fd(fd, 0, TST_MB, FILE_SIZE_MB); >> + tst_fill_fd(fd, 0, TST_MB, tc->write_size_mb); >> >> - TEST(sync_file_range(fd, 0, FILE_SIZE, >> + TEST(sync_file_range(fd, tc->sync_off, tc->sync_size, >> SYNC_FILE_RANGE_WAIT_BEFORE | >> SYNC_FILE_RANGE_WRITE | >> SYNC_FILE_RANGE_WAIT_AFTER)); >> @@ -50,23 +63,43 @@ static void verify_sync_file_range(void) >> >> SAFE_CLOSE(fd); >> >> - if (written >= FILE_SIZE) >> + if ((written >= tc->exp_sync_size) && >> + (written <= (tc->exp_sync_size + tc->exp_sync_size/10))) >> tst_res(TPASS, "Test file range synced to device"); >> else >> - tst_res(TFAIL, "Synced %li, expected %i", written, FILE_SIZE); >> + tst_res(TFAIL, "Synced %li, expected %li", written, >> + tc->exp_sync_size); >> +} >> + >> +static struct testcase testcases[] = { >> + { FNAME1, 0, FILE_SZ, FILE_SZ, 0, FILE_SZ_MB }, >> + { FNAME2, FILE_SZ/4, FILE_SZ/2, FILE_SZ/2, 0, FILE_SZ_MB }, >> + { FNAME3, FILE_SZ/4, FILE_SZ/2, FILE_SZ/4, FILE_SZ/2, FILE_SZ_MB/4 }, >> +}; >> + >> +static void run(unsigned int i) >> +{ >> + verify_sync_file_range(&testcases[i]); >> } >> >> static void setup(void) >> { >> if (!check_sync_file_range()) >> tst_brk(TCONF, "sync_file_range() not supported"); > > > Reviewed-by: Li Wang <liwang@redhat.com> > > That'd be great if we have code comments here, anyway the patch makes sense to me. > > /* > * vfat FS doesn't support sparse files. So handle this via pre-filling the > * test file in case of "test3". > */ > >> + >> + if (!strcmp(tst_device->fs_type, "vfat")) { >> + tst_res(TINFO, "Pre-filling file"); >> + tst_fill_file(FNAME3, 0, TST_MB, FILE_SZ_MB); >> + sync(); >> + } >> } >> >> static struct tst_test test = { >> + .tcnt = ARRAY_SIZE(testcases), >> .needs_root = 1, >> .mount_device = 1, >> .all_filesystems = 1, >> .mntpoint = MNTPOINT, >> .setup = setup, >> - .test_all = verify_sync_file_range, >> + .test = run, >> }; >> -- >> 2.7.4 >> > > > -- > Regards, > Li Wang
Hi! > Do you have any further comments on this patch? Not yet, I'm about to review it. > In case we don't have any further comments, would you like me to send > next version with code comments as suggested by Li or they can be > incorporated while applying the patch? No need to resend, I can add the comment myself.
Hi!
Pushed with following modifications, thanks.
With these modification the test was stable and worked fine for more
than 100 iterations for me. It still fails for FUSE based filesystems
but that is to be expected since FUSE does not implement sync_file_range
yet.
diff --git a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
index 9454a560a..d4c29f9c2 100644
--- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
+++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
@@ -36,6 +36,7 @@ struct testcase {
size_t exp_sync_size;
off64_t write_off;
size_t write_size_mb;
+ const char *desc;
};
static void verify_sync_file_range(struct testcase *tc)
@@ -61,20 +62,34 @@ static void verify_sync_file_range(struct testcase *tc)
written = tst_dev_bytes_written(tst_device->dev);
+ fsync(fd);
+
> Let's make sure there are no outstanding writes schedulled, fixes
> running the test with -i 10
SAFE_CLOSE(fd);
if ((written >= tc->exp_sync_size) &&
(written <= (tc->exp_sync_size + tc->exp_sync_size/10)))
- tst_res(TPASS, "Test file range synced to device");
+ tst_res(TPASS, "%s", tc->desc);
else
- tst_res(TFAIL, "Synced %li, expected %li", written,
- tc->exp_sync_size);
+ tst_res(TFAIL, "%s: Synced %li, expected %li", tc->desc,
+ written, tc->exp_sync_size);
}
static struct testcase testcases[] = {
- { FNAME1, 0, FILE_SZ, FILE_SZ, 0, FILE_SZ_MB },
- { FNAME2, FILE_SZ/4, FILE_SZ/2, FILE_SZ/2, 0, FILE_SZ_MB },
- { FNAME3, FILE_SZ/4, FILE_SZ/2, FILE_SZ/4, FILE_SZ/2, FILE_SZ_MB/4 },
+ {FNAME1,
+ 0, FILE_SZ,
+ FILE_SZ,
+ 0, FILE_SZ_MB,
+ "Sync equals write"},
+ {FNAME2,
+ FILE_SZ/4, FILE_SZ/2,
+ FILE_SZ/2,
+ 0, FILE_SZ_MB,
+ "Sync inside of write"},
+ {FNAME3,
+ FILE_SZ/4, FILE_SZ/2,
+ FILE_SZ/4,
+ FILE_SZ/2, FILE_SZ_MB/4,
+ "Sync overlaps with write"},
};
> This simply adds test description so that it's clear which subtest
> failed.
static void run(unsigned int i)
@@ -87,10 +102,17 @@ static void setup(void)
if (!check_sync_file_range())
tst_brk(TCONF, "sync_file_range() not supported");
+ /*
+ * Fat does not support sparse files, we have to pre-fill the file so
+ * that the zero-filled start of the file has been written to disk
+ * before the test starts.
+ */
if (!strcmp(tst_device->fs_type, "vfat")) {
tst_res(TINFO, "Pre-filling file");
tst_fill_file(FNAME3, 0, TST_MB, FILE_SZ_MB);
- sync();
+ int fd = SAFE_OPEN(FNAME3, O_RDONLY);
+ fsync(fd);
+ SAFE_CLOSE(fd);
}
> Strangely doing sync(); here failed for me, that may be actually a
> kernel bug however there is no need for using the big hammer and sync
> everything in the system anyways.
}
On Mon, Jun 17, 2019 at 6:06 PM Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > Pushed with following modifications, thanks. > > With these modification the test was stable and worked fine for more > than 100 iterations for me. It still fails for FUSE based filesystems > but that is to be expected since FUSE does not implement sync_file_range > yet. > What is the proposed way to resolve this failure? If FUSE does not implement sync_file_range, shouldn't test detect it and return TCONF? Thanks, Amir.
Hi! > > With these modification the test was stable and worked fine for more > > than 100 iterations for me. It still fails for FUSE based filesystems > > but that is to be expected since FUSE does not implement sync_file_range > > yet. > > > > What is the proposed way to resolve this failure? > If FUSE does not implement sync_file_range, shouldn't test detect > it and return TCONF? Well the call returns success but does not sync anything on FUSE based filesystems so the choices here are: 1) Disable the test of FUSE 2) Keep the test failing and ignore the failures Which one do you prefer?
On Wed, Jun 19, 2019 at 12:58 PM Cyril Hrubis <chrubis@suse.cz> wrote: > > Hi! > > > With these modification the test was stable and worked fine for more > > > than 100 iterations for me. It still fails for FUSE based filesystems > > > but that is to be expected since FUSE does not implement sync_file_range > > > yet. > > > > > > > What is the proposed way to resolve this failure? > > If FUSE does not implement sync_file_range, shouldn't test detect > > it and return TCONF? > > Well the call returns success but does not sync anything on FUSE based > filesystems so the choices here are: > > 1) Disable the test of FUSE > 2) Keep the test failing and ignore the failures > > Which one do you prefer? > I prefer the first option. My preference may seem inconsistent with my opinion that F_SETLEASE tests should NOT be disabled for overlayfs. The difference is that sync_file_range() API does not promise any real guaranties vs. F_SETLEASE that has a very clear definition of how it should behave. A subtle difference, but I think it matters. BTW, I did already fix the overlayfs F_SETLEASE bug. It is queued for 5.3 (by file locks maintainer). I will post fixes to LTP F_SETLEASE tests once the kernel fix is merged. Thanks, Amir.
Hi Sumit, On Mon, Jun 10, 2019 at 03:43:16PM +0530, Sumit Garg wrote: <snip> > > - if (written >= FILE_SIZE) > + if ((written >= tc->exp_sync_size) && > + (written <= (tc->exp_sync_size + tc->exp_sync_size/10))) May I ask why it is +1/10 of expected sync_size as upper bound here, since it looks like a magic number to me. We encountered test failure in the second case in a debug kernel, reproducible about once out of 20 times run. The reason is unclear yet, however my guess is that more pages could be written to disk in a debug kernel than a release kernel. My codes and config as below: tree: https://github.com/alibaba/cloud-kernel :: ck-4.19.67 branch; config: https://github.com/alibaba/cloud-kernel/blob/master/config-4.19.y-x86_64-debug If you like you can build a test kernel on a KVM guest and try to reproduce, or just run a RHEL8 kernel I guess (@Li Wang, you can have a try on RHEL8 debug kernel if possible). a sample output: tst_device.c:87: INFO: Found free device 0 '/dev/loop0' tst_supported_fs_types.c:60: INFO: Kernel supports ext2 tst_supported_fs_types.c:44: INFO: mkfs.ext2 does exist tst_supported_fs_types.c:60: INFO: Kernel supports ext3 tst_supported_fs_types.c:44: INFO: mkfs.ext3 does exist tst_supported_fs_types.c:60: INFO: Kernel supports ext4 tst_supported_fs_types.c:44: INFO: mkfs.ext4 does exist tst_supported_fs_types.c:60: INFO: Kernel supports xfs tst_supported_fs_types.c:44: INFO: mkfs.xfs does exist tst_supported_fs_types.c:60: INFO: Kernel supports btrfs tst_supported_fs_types.c:44: INFO: mkfs.btrfs does exist tst_supported_fs_types.c:60: INFO: Kernel supports vfat tst_supported_fs_types.c:44: INFO: mkfs.vfat does exist tst_supported_fs_types.c:83: INFO: Filesystem exfat is not supported tst_supported_fs_types.c:83: INFO: Filesystem ntfs is not supported tst_test.c:1179: INFO: Testing on ext2 tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext2 opts='' extra opts='' mke2fs 1.43.5 (04-Aug-2017) tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s sync_file_range02.c:71: PASS: Sync equals write sync_file_range02.c:71: PASS: Sync inside of write sync_file_range02.c:71: PASS: Sync overlaps with write tst_test.c:1179: INFO: Testing on ext3 tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext3 opts='' extra opts='' mke2fs 1.43.5 (04-Aug-2017) tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s sync_file_range02.c:71: PASS: Sync equals write sync_file_range02.c:71: PASS: Sync inside of write sync_file_range02.c:71: PASS: Sync overlaps with write tst_test.c:1179: INFO: Testing on ext4 tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext4 opts='' extra opts='' mke2fs 1.43.5 (04-Aug-2017) tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s sync_file_range02.c:71: PASS: Sync equals write sync_file_range02.c:74: FAIL: Sync inside of write: Synced 19658752, expected 16777216 ^^^^ sync_file_range02.c:71: PASS: Sync overlaps with write tst_test.c:1179: INFO: Testing on xfs tst_mkfs.c:90: INFO: Formatting /dev/loop0 with xfs opts='' extra opts='' tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s sync_file_range02.c:71: PASS: Sync equals write sync_file_range02.c:71: PASS: Sync inside of write sync_file_range02.c:71: PASS: Sync overlaps with write tst_test.c:1179: INFO: Testing on btrfs tst_mkfs.c:90: INFO: Formatting /dev/loop0 with btrfs opts='' extra opts='' tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s sync_file_range02.c:71: PASS: Sync equals write sync_file_range02.c:71: PASS: Sync inside of write sync_file_range02.c:71: PASS: Sync overlaps with write tst_test.c:1179: INFO: Testing on vfat tst_mkfs.c:90: INFO: Formatting /dev/loop0 with vfat opts='' extra opts='' tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s sync_file_range02.c:111: INFO: Pre-filling file sync_file_range02.c:71: PASS: Sync equals write sync_file_range02.c:71: PASS: Sync inside of write sync_file_range02.c:71: PASS: Sync overlaps with write Summary: passed 17 failed 1 skipped 0 warnings 0 Any thoughts would be appreicated. Thanks, Caspar > tst_res(TPASS, "Test file range synced to device"); > else > - tst_res(TFAIL, "Synced %li, expected %i", written, FILE_SIZE); > + tst_res(TFAIL, "Synced %li, expected %li", written, > + tc->exp_sync_size); > +} > + > +static struct testcase testcases[] = { > + { FNAME1, 0, FILE_SZ, FILE_SZ, 0, FILE_SZ_MB }, > + { FNAME2, FILE_SZ/4, FILE_SZ/2, FILE_SZ/2, 0, FILE_SZ_MB }, > + { FNAME3, FILE_SZ/4, FILE_SZ/2, FILE_SZ/4, FILE_SZ/2, FILE_SZ_MB/4 }, > +}; > + > +static void run(unsigned int i) > +{ > + verify_sync_file_range(&testcases[i]); > } > > static void setup(void) > { > if (!check_sync_file_range()) > tst_brk(TCONF, "sync_file_range() not supported"); > + > + if (!strcmp(tst_device->fs_type, "vfat")) { > + tst_res(TINFO, "Pre-filling file"); > + tst_fill_file(FNAME3, 0, TST_MB, FILE_SZ_MB); > + sync(); > + } > } > > static struct tst_test test = { > + .tcnt = ARRAY_SIZE(testcases), > .needs_root = 1, > .mount_device = 1, > .all_filesystems = 1, > .mntpoint = MNTPOINT, > .setup = setup, > - .test_all = verify_sync_file_range, > + .test = run, > }; > -- > 2.7.4 > > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp -- Thanks, Caspar -- Mailing list info: https://lists.linux.it/listinfo/ltp
Hi Caspar, On Sun, 29 Sep 2019 at 18:58, Caspar Zhang <caspar@casparzhang.com> wrote: > > Hi Sumit, > > On Mon, Jun 10, 2019 at 03:43:16PM +0530, Sumit Garg wrote: > > <snip> > > > > > - if (written >= FILE_SIZE) > > + if ((written >= tc->exp_sync_size) && > > + (written <= (tc->exp_sync_size + tc->exp_sync_size/10))) > > May I ask why it is +1/10 of expected sync_size as upper bound here, > since it looks like a magic number to me. It was an outcome of discussion here [1]. The reason being to test that only particular portion of file is written to device for whom sync has been invoked and +1/10 as upper bound to incorporate for any metadata. [1] https://patchwork.ozlabs.org/patch/1051647/ > > We encountered test failure in the second case in a debug kernel, > reproducible about once out of 20 times run. Interesting case. Can you share results after applying below patch? diff --git a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c index eb08143..1bc1a44 100644 --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c @@ -42,7 +42,7 @@ struct testcase { static void verify_sync_file_range(struct testcase *tc) { int fd; - unsigned long written; + unsigned long written, written_pre; fd = SAFE_OPEN(tc->fname, O_RDWR|O_CREAT, MODE); @@ -52,6 +52,8 @@ static void verify_sync_file_range(struct testcase *tc) tst_fill_fd(fd, 0, TST_MB, tc->write_size_mb); + written_pre = tst_dev_bytes_written(tst_device->dev); + TEST(sync_file_range(fd, tc->sync_off, tc->sync_size, SYNC_FILE_RANGE_WAIT_BEFORE | SYNC_FILE_RANGE_WRITE | @@ -70,8 +72,8 @@ static void verify_sync_file_range(struct testcase *tc) (written <= (tc->exp_sync_size + tc->exp_sync_size/10))) tst_res(TPASS, "%s", tc->desc); else - tst_res(TFAIL, "%s: Synced %li, expected %li", tc->desc, - written, tc->exp_sync_size); + tst_res(TFAIL, "%s: Synced %li, expected %li, pre-sync %li", + tc->desc, written, tc->exp_sync_size, written_pre); } static struct testcase testcases[] = { -Sumit > > The reason is unclear yet, however my guess is that more pages could be > written to disk in a debug kernel than a release kernel. > > My codes and config as below: > > tree: https://github.com/alibaba/cloud-kernel :: ck-4.19.67 branch; > config: https://github.com/alibaba/cloud-kernel/blob/master/config-4.19.y-x86_64-debug > > If you like you can build a test kernel on a KVM guest and try to > reproduce, or just run a RHEL8 kernel I guess (@Li Wang, you can have a > try on RHEL8 debug kernel if possible). > > a sample output: > > tst_device.c:87: INFO: Found free device 0 '/dev/loop0' > tst_supported_fs_types.c:60: INFO: Kernel supports ext2 > tst_supported_fs_types.c:44: INFO: mkfs.ext2 does exist > tst_supported_fs_types.c:60: INFO: Kernel supports ext3 > tst_supported_fs_types.c:44: INFO: mkfs.ext3 does exist > tst_supported_fs_types.c:60: INFO: Kernel supports ext4 > tst_supported_fs_types.c:44: INFO: mkfs.ext4 does exist > tst_supported_fs_types.c:60: INFO: Kernel supports xfs > tst_supported_fs_types.c:44: INFO: mkfs.xfs does exist > tst_supported_fs_types.c:60: INFO: Kernel supports btrfs > tst_supported_fs_types.c:44: INFO: mkfs.btrfs does exist > tst_supported_fs_types.c:60: INFO: Kernel supports vfat > tst_supported_fs_types.c:44: INFO: mkfs.vfat does exist > tst_supported_fs_types.c:83: INFO: Filesystem exfat is not supported > tst_supported_fs_types.c:83: INFO: Filesystem ntfs is not supported > tst_test.c:1179: INFO: Testing on ext2 > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext2 opts='' extra opts='' > mke2fs 1.43.5 (04-Aug-2017) > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > sync_file_range02.c:71: PASS: Sync equals write > sync_file_range02.c:71: PASS: Sync inside of write > sync_file_range02.c:71: PASS: Sync overlaps with write > tst_test.c:1179: INFO: Testing on ext3 > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext3 opts='' extra opts='' > mke2fs 1.43.5 (04-Aug-2017) > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > sync_file_range02.c:71: PASS: Sync equals write > sync_file_range02.c:71: PASS: Sync inside of write > sync_file_range02.c:71: PASS: Sync overlaps with write > tst_test.c:1179: INFO: Testing on ext4 > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext4 opts='' extra opts='' > mke2fs 1.43.5 (04-Aug-2017) > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > sync_file_range02.c:71: PASS: Sync equals write > sync_file_range02.c:74: FAIL: Sync inside of write: Synced 19658752, expected 16777216 > ^^^^ > sync_file_range02.c:71: PASS: Sync overlaps with write > tst_test.c:1179: INFO: Testing on xfs > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with xfs opts='' extra opts='' > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > sync_file_range02.c:71: PASS: Sync equals write > sync_file_range02.c:71: PASS: Sync inside of write > sync_file_range02.c:71: PASS: Sync overlaps with write > tst_test.c:1179: INFO: Testing on btrfs > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with btrfs opts='' extra opts='' > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > sync_file_range02.c:71: PASS: Sync equals write > sync_file_range02.c:71: PASS: Sync inside of write > sync_file_range02.c:71: PASS: Sync overlaps with write > tst_test.c:1179: INFO: Testing on vfat > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with vfat opts='' extra opts='' > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > sync_file_range02.c:111: INFO: Pre-filling file > sync_file_range02.c:71: PASS: Sync equals write > sync_file_range02.c:71: PASS: Sync inside of write > sync_file_range02.c:71: PASS: Sync overlaps with write > > Summary: > passed 17 > failed 1 > skipped 0 > warnings 0 > > Any thoughts would be appreicated. > > Thanks, > Caspar > > > > tst_res(TPASS, "Test file range synced to device"); > > else > > - tst_res(TFAIL, "Synced %li, expected %i", written, FILE_SIZE); > > + tst_res(TFAIL, "Synced %li, expected %li", written, > > + tc->exp_sync_size); > > +} > > + > > +static struct testcase testcases[] = { > > + { FNAME1, 0, FILE_SZ, FILE_SZ, 0, FILE_SZ_MB }, > > + { FNAME2, FILE_SZ/4, FILE_SZ/2, FILE_SZ/2, 0, FILE_SZ_MB }, > > + { FNAME3, FILE_SZ/4, FILE_SZ/2, FILE_SZ/4, FILE_SZ/2, FILE_SZ_MB/4 }, > > +}; > > + > > +static void run(unsigned int i) > > +{ > > + verify_sync_file_range(&testcases[i]); > > } > > > > static void setup(void) > > { > > if (!check_sync_file_range()) > > tst_brk(TCONF, "sync_file_range() not supported"); > > + > > + if (!strcmp(tst_device->fs_type, "vfat")) { > > + tst_res(TINFO, "Pre-filling file"); > > + tst_fill_file(FNAME3, 0, TST_MB, FILE_SZ_MB); > > + sync(); > > + } > > } > > > > static struct tst_test test = { > > + .tcnt = ARRAY_SIZE(testcases), > > .needs_root = 1, > > .mount_device = 1, > > .all_filesystems = 1, > > .mntpoint = MNTPOINT, > > .setup = setup, > > - .test_all = verify_sync_file_range, > > + .test = run, > > }; > > -- > > 2.7.4 > > > > > > -- > > Mailing list info: https://lists.linux.it/listinfo/ltp > > -- > Thanks, > Caspar -- Mailing list info: https://lists.linux.it/listinfo/ltp
Hi Sumit, On Fri, Oct 04, 2019 at 01:03:19PM +0530, Sumit Garg wrote: > Hi Caspar, > > On Sun, 29 Sep 2019 at 18:58, Caspar Zhang <caspar@casparzhang.com> wrote: > > > > Hi Sumit, > > > > On Mon, Jun 10, 2019 at 03:43:16PM +0530, Sumit Garg wrote: > > > > <snip> > > > > > > > > - if (written >= FILE_SIZE) > > > + if ((written >= tc->exp_sync_size) && > > > + (written <= (tc->exp_sync_size + tc->exp_sync_size/10))) > > > > May I ask why it is +1/10 of expected sync_size as upper bound here, > > since it looks like a magic number to me. > > It was an outcome of discussion here [1]. The reason being to test > that only particular portion of file is written to device for whom > sync has been invoked and +1/10 as upper bound to incorporate for any > metadata. I see, thanks for explanation. > > [1] https://patchwork.ozlabs.org/patch/1051647/ > > > > > We encountered test failure in the second case in a debug kernel, > > reproducible about once out of 20 times run. > > Interesting case. Can you share results after applying below patch? Tested this patch, no TFAIL occured in debug kernel after 200+ times run, looks good to me. Thanks! Please add my Reviewed-by: Caspar Zhang <caspar@linux.alibaba.com> directly if you're going to make a formal patch later. Thanks, Caspar > > diff --git a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > index eb08143..1bc1a44 100644 > --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > @@ -42,7 +42,7 @@ struct testcase { > static void verify_sync_file_range(struct testcase *tc) > { > int fd; > - unsigned long written; > + unsigned long written, written_pre; > > fd = SAFE_OPEN(tc->fname, O_RDWR|O_CREAT, MODE); > > @@ -52,6 +52,8 @@ static void verify_sync_file_range(struct testcase *tc) > > tst_fill_fd(fd, 0, TST_MB, tc->write_size_mb); > > + written_pre = tst_dev_bytes_written(tst_device->dev); > + > TEST(sync_file_range(fd, tc->sync_off, tc->sync_size, > SYNC_FILE_RANGE_WAIT_BEFORE | > SYNC_FILE_RANGE_WRITE | > @@ -70,8 +72,8 @@ static void verify_sync_file_range(struct testcase *tc) > (written <= (tc->exp_sync_size + tc->exp_sync_size/10))) > tst_res(TPASS, "%s", tc->desc); > else > - tst_res(TFAIL, "%s: Synced %li, expected %li", tc->desc, > - written, tc->exp_sync_size); > + tst_res(TFAIL, "%s: Synced %li, expected %li, pre-sync %li", > + tc->desc, written, tc->exp_sync_size, written_pre); > } > > static struct testcase testcases[] = { > > -Sumit > > > > > The reason is unclear yet, however my guess is that more pages could be > > written to disk in a debug kernel than a release kernel. > > > > My codes and config as below: > > > > tree: https://github.com/alibaba/cloud-kernel :: ck-4.19.67 branch; > > config: https://github.com/alibaba/cloud-kernel/blob/master/config-4.19.y-x86_64-debug > > > > If you like you can build a test kernel on a KVM guest and try to > > reproduce, or just run a RHEL8 kernel I guess (@Li Wang, you can have a > > try on RHEL8 debug kernel if possible). > > > > a sample output: > > > > tst_device.c:87: INFO: Found free device 0 '/dev/loop0' > > tst_supported_fs_types.c:60: INFO: Kernel supports ext2 > > tst_supported_fs_types.c:44: INFO: mkfs.ext2 does exist > > tst_supported_fs_types.c:60: INFO: Kernel supports ext3 > > tst_supported_fs_types.c:44: INFO: mkfs.ext3 does exist > > tst_supported_fs_types.c:60: INFO: Kernel supports ext4 > > tst_supported_fs_types.c:44: INFO: mkfs.ext4 does exist > > tst_supported_fs_types.c:60: INFO: Kernel supports xfs > > tst_supported_fs_types.c:44: INFO: mkfs.xfs does exist > > tst_supported_fs_types.c:60: INFO: Kernel supports btrfs > > tst_supported_fs_types.c:44: INFO: mkfs.btrfs does exist > > tst_supported_fs_types.c:60: INFO: Kernel supports vfat > > tst_supported_fs_types.c:44: INFO: mkfs.vfat does exist > > tst_supported_fs_types.c:83: INFO: Filesystem exfat is not supported > > tst_supported_fs_types.c:83: INFO: Filesystem ntfs is not supported > > tst_test.c:1179: INFO: Testing on ext2 > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext2 opts='' extra opts='' > > mke2fs 1.43.5 (04-Aug-2017) > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > sync_file_range02.c:71: PASS: Sync equals write > > sync_file_range02.c:71: PASS: Sync inside of write > > sync_file_range02.c:71: PASS: Sync overlaps with write > > tst_test.c:1179: INFO: Testing on ext3 > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext3 opts='' extra opts='' > > mke2fs 1.43.5 (04-Aug-2017) > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > sync_file_range02.c:71: PASS: Sync equals write > > sync_file_range02.c:71: PASS: Sync inside of write > > sync_file_range02.c:71: PASS: Sync overlaps with write > > tst_test.c:1179: INFO: Testing on ext4 > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext4 opts='' extra opts='' > > mke2fs 1.43.5 (04-Aug-2017) > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > sync_file_range02.c:71: PASS: Sync equals write > > sync_file_range02.c:74: FAIL: Sync inside of write: Synced 19658752, expected 16777216 > > ^^^^ > > sync_file_range02.c:71: PASS: Sync overlaps with write > > tst_test.c:1179: INFO: Testing on xfs > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with xfs opts='' extra opts='' > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > sync_file_range02.c:71: PASS: Sync equals write > > sync_file_range02.c:71: PASS: Sync inside of write > > sync_file_range02.c:71: PASS: Sync overlaps with write > > tst_test.c:1179: INFO: Testing on btrfs > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with btrfs opts='' extra opts='' > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > sync_file_range02.c:71: PASS: Sync equals write > > sync_file_range02.c:71: PASS: Sync inside of write > > sync_file_range02.c:71: PASS: Sync overlaps with write > > tst_test.c:1179: INFO: Testing on vfat > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with vfat opts='' extra opts='' > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > sync_file_range02.c:111: INFO: Pre-filling file > > sync_file_range02.c:71: PASS: Sync equals write > > sync_file_range02.c:71: PASS: Sync inside of write > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > > Summary: > > passed 17 > > failed 1 > > skipped 0 > > warnings 0 > > > > Any thoughts would be appreicated. > > > > Thanks, > > Caspar > > > > > > > tst_res(TPASS, "Test file range synced to device"); > > > else > > > - tst_res(TFAIL, "Synced %li, expected %i", written, FILE_SIZE); > > > + tst_res(TFAIL, "Synced %li, expected %li", written, > > > + tc->exp_sync_size); > > > +} > > > + > > > +static struct testcase testcases[] = { > > > + { FNAME1, 0, FILE_SZ, FILE_SZ, 0, FILE_SZ_MB }, > > > + { FNAME2, FILE_SZ/4, FILE_SZ/2, FILE_SZ/2, 0, FILE_SZ_MB }, > > > + { FNAME3, FILE_SZ/4, FILE_SZ/2, FILE_SZ/4, FILE_SZ/2, FILE_SZ_MB/4 }, > > > +}; > > > + > > > +static void run(unsigned int i) > > > +{ > > > + verify_sync_file_range(&testcases[i]); > > > } > > > > > > static void setup(void) > > > { > > > if (!check_sync_file_range()) > > > tst_brk(TCONF, "sync_file_range() not supported"); > > > + > > > + if (!strcmp(tst_device->fs_type, "vfat")) { > > > + tst_res(TINFO, "Pre-filling file"); > > > + tst_fill_file(FNAME3, 0, TST_MB, FILE_SZ_MB); > > > + sync(); > > > + } > > > } > > > > > > static struct tst_test test = { > > > + .tcnt = ARRAY_SIZE(testcases), > > > .needs_root = 1, > > > .mount_device = 1, > > > .all_filesystems = 1, > > > .mntpoint = MNTPOINT, > > > .setup = setup, > > > - .test_all = verify_sync_file_range, > > > + .test = run, > > > }; > > > -- > > > 2.7.4 > > > > > > > > > -- > > > Mailing list info: https://lists.linux.it/listinfo/ltp > > > > -- > > Thanks, > > Caspar -- Thanks, Caspar -- Mailing list info: https://lists.linux.it/listinfo/ltp
Hi Caspar, On Wed, 16 Oct 2019 at 08:24, Caspar Zhang <caspar@casparzhang.com> wrote: > > Hi Sumit, > > On Fri, Oct 04, 2019 at 01:03:19PM +0530, Sumit Garg wrote: > > Hi Caspar, > > > > On Sun, 29 Sep 2019 at 18:58, Caspar Zhang <caspar@casparzhang.com> wrote: > > > > > > Hi Sumit, > > > > > > On Mon, Jun 10, 2019 at 03:43:16PM +0530, Sumit Garg wrote: > > > > > > <snip> > > > > > > > > > > > - if (written >= FILE_SIZE) > > > > + if ((written >= tc->exp_sync_size) && > > > > + (written <= (tc->exp_sync_size + tc->exp_sync_size/10))) > > > > > > May I ask why it is +1/10 of expected sync_size as upper bound here, > > > since it looks like a magic number to me. > > > > It was an outcome of discussion here [1]. The reason being to test > > that only particular portion of file is written to device for whom > > sync has been invoked and +1/10 as upper bound to incorporate for any > > metadata. > > I see, thanks for explanation. > > > > > [1] https://patchwork.ozlabs.org/patch/1051647/ > > > > > > > > We encountered test failure in the second case in a debug kernel, > > > reproducible about once out of 20 times run. > > > > Interesting case. Can you share results after applying below patch? > > Tested this patch, no TFAIL occured in debug kernel after 200+ times > run, looks good to me. Thanks! Please add my From these results, the reason for the failure that you reported earlier seems to be writes to the device during "tst_fill_fd()" operation (they were found negligible/zero with normal kernel). But it's strange to know that you didn't get any TFAIL after the patch as I expected "Sync equals write" to fail. So can you also put following debug print and share logs of your test run? --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c @@ -68,6 +68,8 @@ static void verify_sync_file_range(struct testcase *tc) SAFE_CLOSE(fd); + printf("%s: Synced %li, expected %li, pre-sync %li\n", + tc->desc, written, tc->exp_sync_size, written_pre); if ((written >= tc->exp_sync_size) && (written <= (tc->exp_sync_size + tc->exp_sync_size/10))) tst_res(TPASS, "%s", tc->desc); -Sumit > > Reviewed-by: Caspar Zhang <caspar@linux.alibaba.com> > > directly if you're going to make a formal patch later. > > Thanks, > Caspar > > > > > diff --git a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > > b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > > index eb08143..1bc1a44 100644 > > --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > > +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > > @@ -42,7 +42,7 @@ struct testcase { > > static void verify_sync_file_range(struct testcase *tc) > > { > > int fd; > > - unsigned long written; > > + unsigned long written, written_pre; > > > > fd = SAFE_OPEN(tc->fname, O_RDWR|O_CREAT, MODE); > > > > @@ -52,6 +52,8 @@ static void verify_sync_file_range(struct testcase *tc) > > > > tst_fill_fd(fd, 0, TST_MB, tc->write_size_mb); > > > > + written_pre = tst_dev_bytes_written(tst_device->dev); > > + > > TEST(sync_file_range(fd, tc->sync_off, tc->sync_size, > > SYNC_FILE_RANGE_WAIT_BEFORE | > > SYNC_FILE_RANGE_WRITE | > > @@ -70,8 +72,8 @@ static void verify_sync_file_range(struct testcase *tc) > > (written <= (tc->exp_sync_size + tc->exp_sync_size/10))) > > tst_res(TPASS, "%s", tc->desc); > > else > > - tst_res(TFAIL, "%s: Synced %li, expected %li", tc->desc, > > - written, tc->exp_sync_size); > > + tst_res(TFAIL, "%s: Synced %li, expected %li, pre-sync %li", > > + tc->desc, written, tc->exp_sync_size, written_pre); > > } > > > > static struct testcase testcases[] = { > > > > -Sumit > > > > > > > > The reason is unclear yet, however my guess is that more pages could be > > > written to disk in a debug kernel than a release kernel. > > > > > > My codes and config as below: > > > > > > tree: https://github.com/alibaba/cloud-kernel :: ck-4.19.67 branch; > > > config: https://github.com/alibaba/cloud-kernel/blob/master/config-4.19.y-x86_64-debug > > > > > > If you like you can build a test kernel on a KVM guest and try to > > > reproduce, or just run a RHEL8 kernel I guess (@Li Wang, you can have a > > > try on RHEL8 debug kernel if possible). > > > > > > a sample output: > > > > > > tst_device.c:87: INFO: Found free device 0 '/dev/loop0' > > > tst_supported_fs_types.c:60: INFO: Kernel supports ext2 > > > tst_supported_fs_types.c:44: INFO: mkfs.ext2 does exist > > > tst_supported_fs_types.c:60: INFO: Kernel supports ext3 > > > tst_supported_fs_types.c:44: INFO: mkfs.ext3 does exist > > > tst_supported_fs_types.c:60: INFO: Kernel supports ext4 > > > tst_supported_fs_types.c:44: INFO: mkfs.ext4 does exist > > > tst_supported_fs_types.c:60: INFO: Kernel supports xfs > > > tst_supported_fs_types.c:44: INFO: mkfs.xfs does exist > > > tst_supported_fs_types.c:60: INFO: Kernel supports btrfs > > > tst_supported_fs_types.c:44: INFO: mkfs.btrfs does exist > > > tst_supported_fs_types.c:60: INFO: Kernel supports vfat > > > tst_supported_fs_types.c:44: INFO: mkfs.vfat does exist > > > tst_supported_fs_types.c:83: INFO: Filesystem exfat is not supported > > > tst_supported_fs_types.c:83: INFO: Filesystem ntfs is not supported > > > tst_test.c:1179: INFO: Testing on ext2 > > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext2 opts='' extra opts='' > > > mke2fs 1.43.5 (04-Aug-2017) > > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > > sync_file_range02.c:71: PASS: Sync equals write > > > sync_file_range02.c:71: PASS: Sync inside of write > > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > tst_test.c:1179: INFO: Testing on ext3 > > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext3 opts='' extra opts='' > > > mke2fs 1.43.5 (04-Aug-2017) > > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > > sync_file_range02.c:71: PASS: Sync equals write > > > sync_file_range02.c:71: PASS: Sync inside of write > > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > tst_test.c:1179: INFO: Testing on ext4 > > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext4 opts='' extra opts='' > > > mke2fs 1.43.5 (04-Aug-2017) > > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > > sync_file_range02.c:71: PASS: Sync equals write > > > sync_file_range02.c:74: FAIL: Sync inside of write: Synced 19658752, expected 16777216 > > > ^^^^ > > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > tst_test.c:1179: INFO: Testing on xfs > > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with xfs opts='' extra opts='' > > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > > sync_file_range02.c:71: PASS: Sync equals write > > > sync_file_range02.c:71: PASS: Sync inside of write > > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > tst_test.c:1179: INFO: Testing on btrfs > > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with btrfs opts='' extra opts='' > > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > > sync_file_range02.c:71: PASS: Sync equals write > > > sync_file_range02.c:71: PASS: Sync inside of write > > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > tst_test.c:1179: INFO: Testing on vfat > > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with vfat opts='' extra opts='' > > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > > sync_file_range02.c:111: INFO: Pre-filling file > > > sync_file_range02.c:71: PASS: Sync equals write > > > sync_file_range02.c:71: PASS: Sync inside of write > > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > > > > Summary: > > > passed 17 > > > failed 1 > > > skipped 0 > > > warnings 0 > > > > > > Any thoughts would be appreicated. > > > > > > Thanks, > > > Caspar > > > > > > > > > > tst_res(TPASS, "Test file range synced to device"); > > > > else > > > > - tst_res(TFAIL, "Synced %li, expected %i", written, FILE_SIZE); > > > > + tst_res(TFAIL, "Synced %li, expected %li", written, > > > > + tc->exp_sync_size); > > > > +} > > > > + > > > > +static struct testcase testcases[] = { > > > > + { FNAME1, 0, FILE_SZ, FILE_SZ, 0, FILE_SZ_MB }, > > > > + { FNAME2, FILE_SZ/4, FILE_SZ/2, FILE_SZ/2, 0, FILE_SZ_MB }, > > > > + { FNAME3, FILE_SZ/4, FILE_SZ/2, FILE_SZ/4, FILE_SZ/2, FILE_SZ_MB/4 }, > > > > +}; > > > > + > > > > +static void run(unsigned int i) > > > > +{ > > > > + verify_sync_file_range(&testcases[i]); > > > > } > > > > > > > > static void setup(void) > > > > { > > > > if (!check_sync_file_range()) > > > > tst_brk(TCONF, "sync_file_range() not supported"); > > > > + > > > > + if (!strcmp(tst_device->fs_type, "vfat")) { > > > > + tst_res(TINFO, "Pre-filling file"); > > > > + tst_fill_file(FNAME3, 0, TST_MB, FILE_SZ_MB); > > > > + sync(); > > > > + } > > > > } > > > > > > > > static struct tst_test test = { > > > > + .tcnt = ARRAY_SIZE(testcases), > > > > .needs_root = 1, > > > > .mount_device = 1, > > > > .all_filesystems = 1, > > > > .mntpoint = MNTPOINT, > > > > .setup = setup, > > > > - .test_all = verify_sync_file_range, > > > > + .test = run, > > > > }; > > > > -- > > > > 2.7.4 > > > > > > > > > > > > -- > > > > Mailing list info: https://lists.linux.it/listinfo/ltp > > > > > > -- > > > Thanks, > > > Caspar > > -- > Thanks, > Caspar -- Mailing list info: https://lists.linux.it/listinfo/ltp
On Wed, Oct 16, 2019 at 11:53:48AM +0530, Sumit Garg wrote: > Hi Caspar, > > On Wed, 16 Oct 2019 at 08:24, Caspar Zhang <caspar@casparzhang.com> wrote: > > > > Hi Sumit, > > > > On Fri, Oct 04, 2019 at 01:03:19PM +0530, Sumit Garg wrote: > > > Hi Caspar, > > > > > > On Sun, 29 Sep 2019 at 18:58, Caspar Zhang <caspar@casparzhang.com> wrote: > > > > > > > > Hi Sumit, > > > > > > > > On Mon, Jun 10, 2019 at 03:43:16PM +0530, Sumit Garg wrote: > > > > > > > > <snip> > > > > > > > > > > > > > > - if (written >= FILE_SIZE) > > > > > + if ((written >= tc->exp_sync_size) && > > > > > + (written <= (tc->exp_sync_size + tc->exp_sync_size/10))) > > > > > > > > May I ask why it is +1/10 of expected sync_size as upper bound here, > > > > since it looks like a magic number to me. > > > > > > It was an outcome of discussion here [1]. The reason being to test > > > that only particular portion of file is written to device for whom > > > sync has been invoked and +1/10 as upper bound to incorporate for any > > > metadata. > > > > I see, thanks for explanation. > > > > > > > > [1] https://patchwork.ozlabs.org/patch/1051647/ > > > > > > > > > > > We encountered test failure in the second case in a debug kernel, > > > > reproducible about once out of 20 times run. > > > > > > Interesting case. Can you share results after applying below patch? > > > > Tested this patch, no TFAIL occured in debug kernel after 200+ times > > run, looks good to me. Thanks! Please add my > > From these results, the reason for the failure that you reported > earlier seems to be writes to the device during "tst_fill_fd()" > operation (they were found negligible/zero with normal kernel). But > it's strange to know that you didn't get any TFAIL after the patch as > I expected "Sync equals write" to fail. > > So can you also put following debug print and share logs of your test run? Retested with debug print, during my 1000-times run, pre-sync remains 0 in all the other fs types except only ext4. For ext4 cases, pre-sync could be non-zero, e.g.: Sync equals write: Synced 33554432, expected 33554432, pre-sync 0 Sync inside of write: Synced 17301504, expected 16777216, pre-sync 1308672 Sync overlaps with write: Synced 8650752, expected 8388608, pre-sync 1310720 Note that pre-sync could be non-zero in `equals writes` case sometimes too, like another round below: Sync equals write: Synced 34078720, expected 33554432, pre-sync 260096 Sync inside of write: Synced 17039360, expected 16777216, pre-sync 4980736 Sync overlaps with write: Synced 8912896, expected 8388608, pre-sync 1048576 Such non-zero situation in ext4 case is reproducible ~10% of my 1000-times run. Thanks, Caspar > > --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > @@ -68,6 +68,8 @@ static void verify_sync_file_range(struct testcase *tc) > > SAFE_CLOSE(fd); > > + printf("%s: Synced %li, expected %li, pre-sync %li\n", > + tc->desc, written, tc->exp_sync_size, written_pre); > if ((written >= tc->exp_sync_size) && > (written <= (tc->exp_sync_size + tc->exp_sync_size/10))) > tst_res(TPASS, "%s", tc->desc); > > -Sumit > > > > > Reviewed-by: Caspar Zhang <caspar@linux.alibaba.com> > > > > directly if you're going to make a formal patch later. > > > > Thanks, > > Caspar > > > > > > > > diff --git a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > > > b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > > > index eb08143..1bc1a44 100644 > > > --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > > > +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > > > @@ -42,7 +42,7 @@ struct testcase { > > > static void verify_sync_file_range(struct testcase *tc) > > > { > > > int fd; > > > - unsigned long written; > > > + unsigned long written, written_pre; > > > > > > fd = SAFE_OPEN(tc->fname, O_RDWR|O_CREAT, MODE); > > > > > > @@ -52,6 +52,8 @@ static void verify_sync_file_range(struct testcase *tc) > > > > > > tst_fill_fd(fd, 0, TST_MB, tc->write_size_mb); > > > > > > + written_pre = tst_dev_bytes_written(tst_device->dev); > > > + > > > TEST(sync_file_range(fd, tc->sync_off, tc->sync_size, > > > SYNC_FILE_RANGE_WAIT_BEFORE | > > > SYNC_FILE_RANGE_WRITE | > > > @@ -70,8 +72,8 @@ static void verify_sync_file_range(struct testcase *tc) > > > (written <= (tc->exp_sync_size + tc->exp_sync_size/10))) > > > tst_res(TPASS, "%s", tc->desc); > > > else > > > - tst_res(TFAIL, "%s: Synced %li, expected %li", tc->desc, > > > - written, tc->exp_sync_size); > > > + tst_res(TFAIL, "%s: Synced %li, expected %li, pre-sync %li", > > > + tc->desc, written, tc->exp_sync_size, written_pre); > > > } > > > > > > static struct testcase testcases[] = { > > > > > > -Sumit > > > > > > > > > > > The reason is unclear yet, however my guess is that more pages could be > > > > written to disk in a debug kernel than a release kernel. > > > > > > > > My codes and config as below: > > > > > > > > tree: https://github.com/alibaba/cloud-kernel :: ck-4.19.67 branch; > > > > config: https://github.com/alibaba/cloud-kernel/blob/master/config-4.19.y-x86_64-debug > > > > > > > > If you like you can build a test kernel on a KVM guest and try to > > > > reproduce, or just run a RHEL8 kernel I guess (@Li Wang, you can have a > > > > try on RHEL8 debug kernel if possible). > > > > > > > > a sample output: > > > > > > > > tst_device.c:87: INFO: Found free device 0 '/dev/loop0' > > > > tst_supported_fs_types.c:60: INFO: Kernel supports ext2 > > > > tst_supported_fs_types.c:44: INFO: mkfs.ext2 does exist > > > > tst_supported_fs_types.c:60: INFO: Kernel supports ext3 > > > > tst_supported_fs_types.c:44: INFO: mkfs.ext3 does exist > > > > tst_supported_fs_types.c:60: INFO: Kernel supports ext4 > > > > tst_supported_fs_types.c:44: INFO: mkfs.ext4 does exist > > > > tst_supported_fs_types.c:60: INFO: Kernel supports xfs > > > > tst_supported_fs_types.c:44: INFO: mkfs.xfs does exist > > > > tst_supported_fs_types.c:60: INFO: Kernel supports btrfs > > > > tst_supported_fs_types.c:44: INFO: mkfs.btrfs does exist > > > > tst_supported_fs_types.c:60: INFO: Kernel supports vfat > > > > tst_supported_fs_types.c:44: INFO: mkfs.vfat does exist > > > > tst_supported_fs_types.c:83: INFO: Filesystem exfat is not supported > > > > tst_supported_fs_types.c:83: INFO: Filesystem ntfs is not supported > > > > tst_test.c:1179: INFO: Testing on ext2 > > > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext2 opts='' extra opts='' > > > > mke2fs 1.43.5 (04-Aug-2017) > > > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > > > sync_file_range02.c:71: PASS: Sync equals write > > > > sync_file_range02.c:71: PASS: Sync inside of write > > > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > > tst_test.c:1179: INFO: Testing on ext3 > > > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext3 opts='' extra opts='' > > > > mke2fs 1.43.5 (04-Aug-2017) > > > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > > > sync_file_range02.c:71: PASS: Sync equals write > > > > sync_file_range02.c:71: PASS: Sync inside of write > > > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > > tst_test.c:1179: INFO: Testing on ext4 > > > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext4 opts='' extra opts='' > > > > mke2fs 1.43.5 (04-Aug-2017) > > > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > > > sync_file_range02.c:71: PASS: Sync equals write > > > > sync_file_range02.c:74: FAIL: Sync inside of write: Synced 19658752, expected 16777216 > > > > ^^^^ > > > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > > tst_test.c:1179: INFO: Testing on xfs > > > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with xfs opts='' extra opts='' > > > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > > > sync_file_range02.c:71: PASS: Sync equals write > > > > sync_file_range02.c:71: PASS: Sync inside of write > > > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > > tst_test.c:1179: INFO: Testing on btrfs > > > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with btrfs opts='' extra opts='' > > > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > > > sync_file_range02.c:71: PASS: Sync equals write > > > > sync_file_range02.c:71: PASS: Sync inside of write > > > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > > tst_test.c:1179: INFO: Testing on vfat > > > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with vfat opts='' extra opts='' > > > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > > > sync_file_range02.c:111: INFO: Pre-filling file > > > > sync_file_range02.c:71: PASS: Sync equals write > > > > sync_file_range02.c:71: PASS: Sync inside of write > > > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > > > > > > Summary: > > > > passed 17 > > > > failed 1 > > > > skipped 0 > > > > warnings 0 > > > > > > > > Any thoughts would be appreicated. > > > > > > > > Thanks, > > > > Caspar > > > > > > > > > > > > > tst_res(TPASS, "Test file range synced to device"); > > > > > else > > > > > - tst_res(TFAIL, "Synced %li, expected %i", written, FILE_SIZE); > > > > > + tst_res(TFAIL, "Synced %li, expected %li", written, > > > > > + tc->exp_sync_size); > > > > > +} > > > > > + > > > > > +static struct testcase testcases[] = { > > > > > + { FNAME1, 0, FILE_SZ, FILE_SZ, 0, FILE_SZ_MB }, > > > > > + { FNAME2, FILE_SZ/4, FILE_SZ/2, FILE_SZ/2, 0, FILE_SZ_MB }, > > > > > + { FNAME3, FILE_SZ/4, FILE_SZ/2, FILE_SZ/4, FILE_SZ/2, FILE_SZ_MB/4 }, > > > > > +}; > > > > > + > > > > > +static void run(unsigned int i) > > > > > +{ > > > > > + verify_sync_file_range(&testcases[i]); > > > > > } > > > > > > > > > > static void setup(void) > > > > > { > > > > > if (!check_sync_file_range()) > > > > > tst_brk(TCONF, "sync_file_range() not supported"); > > > > > + > > > > > + if (!strcmp(tst_device->fs_type, "vfat")) { > > > > > + tst_res(TINFO, "Pre-filling file"); > > > > > + tst_fill_file(FNAME3, 0, TST_MB, FILE_SZ_MB); > > > > > + sync(); > > > > > + } > > > > > } > > > > > > > > > > static struct tst_test test = { > > > > > + .tcnt = ARRAY_SIZE(testcases), > > > > > .needs_root = 1, > > > > > .mount_device = 1, > > > > > .all_filesystems = 1, > > > > > .mntpoint = MNTPOINT, > > > > > .setup = setup, > > > > > - .test_all = verify_sync_file_range, > > > > > + .test = run, > > > > > }; > > > > > -- > > > > > 2.7.4 > > > > > > > > > > > > > > > -- > > > > > Mailing list info: https://lists.linux.it/listinfo/ltp > > > > > > > > -- > > > > Thanks, > > > > Caspar > > > > -- > > Thanks, > > Caspar -- Thanks, Caspar -- Mailing list info: https://lists.linux.it/listinfo/ltp
Hi Caspar and Sumit, > > > > > We encountered test failure in the second case in a debug kernel, > > > > > reproducible about once out of 20 times run. > > > > > > > > Interesting case. Can you share results after applying below patch? > > > > > > Tested this patch, no TFAIL occured in debug kernel after 200+ times > > > run, looks good to me. Thanks! Please add my > > > > From these results, the reason for the failure that you reported > > earlier seems to be writes to the device during "tst_fill_fd()" > > operation (they were found negligible/zero with normal kernel). But > > it's strange to know that you didn't get any TFAIL after the patch as > > I expected "Sync equals write" to fail. > > > > So can you also put following debug print and share logs of your test run? > > Retested with debug print, during my 1000-times run, pre-sync remains 0 > in all the other fs types except only ext4. For ext4 cases, pre-sync > could be non-zero, e.g.: > > Sync equals write: Synced 33554432, expected 33554432, pre-sync 0 > Sync inside of write: Synced 17301504, expected 16777216, pre-sync 1308672 > Sync overlaps with write: Synced 8650752, expected 8388608, pre-sync 1310720 > > Note that pre-sync could be non-zero in `equals writes` case sometimes > too, like another round below: > > Sync equals write: Synced 34078720, expected 33554432, pre-sync 260096 > Sync inside of write: Synced 17039360, expected 16777216, pre-sync 4980736 > Sync overlaps with write: Synced 8912896, expected 8388608, pre-sync 1048576 > > Such non-zero situation in ext4 case is reproducible ~10% of my > 1000-times run. sync_file_range02 test failure reproduced on mainline and stable rc branches 5.3, 4.19, 4.14 on arm64 and arm devices while testing on ext4. output log, -------------- tst_test.c:1179: INFO: Testing on ext2 tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext2 opts='' extra opts='' mke2fs 1.43.8 (1-Jan-2018) tst_test.c:1118: INFO: Timeout per run is 0h 15m 00s sync_file_range02.c:71: PASS: Sync equals write sync_file_range02.c:71: PASS: Sync inside of write sync_file_range02.c:71: PASS: Sync overlaps with write tst_test.c:1179: INFO: Testing on ext3 tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext3 opts='' extra opts='' mke2fs 1.43.8 (1-Jan-2018) Listened to connection for namespace 'tlxc' done [ 1349.061989] EXT4-fs (loop0): mounting ext3 file system using the ext4 subsystem [ 1349.099564] EXT4-fs (loop0): mounted filesystem with ordered data mode. Opts: (null) tst_test.c:1118: INFO: Timeout per run is 0h 15m 00s sync_file_range02.c:71: PASS: Sync equals write Listened to connection for namespace 'tlxc' done sync_file_range02.c:71: PASS: Sync inside of write sync_file_range02.c:71: PASS: Sync overlaps with write tst_test.c:1179: INFO: Testing on ext4 tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext4 opts='' extra opts='' mke2fs 1.43.8 (1-Jan-2018) [ 1362.579639] EXT4-fs (loop0): mounted filesystem with ordered data mode. Opts: (null) tst_test.c:1118: INFO: Timeout per run is 0h 15m 00s sync_file_range02.c:74: FAIL: Sync equals write: Synced 36960256, expected 33554432 Listened to connection for namespace 'tlxc' done sync_file_range02.c:74: FAIL: Sync inside of write: Synced 20185088, expected 16777216 sync_file_range02.c:71: PASS: Sync overlaps with write Summary: passed 7 failed 2 Full output log, -------------------- https://lkft.validation.linaro.org/scheduler/job/983166#L15067 - Naresh -- Mailing list info: https://lists.linux.it/listinfo/ltp
Hi Caspar, Sorry for the late reply. I was busy with some other stuff along with Diwali holidays last week in India. So now I got a chance to dig deeper into this issue. On Thu, 17 Oct 2019 at 13:07, Caspar Zhang <caspar@casparzhang.com> wrote: > > On Wed, Oct 16, 2019 at 11:53:48AM +0530, Sumit Garg wrote: > > Hi Caspar, > > > > On Wed, 16 Oct 2019 at 08:24, Caspar Zhang <caspar@casparzhang.com> wrote: > > > > > > Hi Sumit, > > > > > > On Fri, Oct 04, 2019 at 01:03:19PM +0530, Sumit Garg wrote: > > > > Hi Caspar, > > > > > > > > On Sun, 29 Sep 2019 at 18:58, Caspar Zhang <caspar@casparzhang.com> wrote: > > > > > > > > > > Hi Sumit, > > > > > > > > > > On Mon, Jun 10, 2019 at 03:43:16PM +0530, Sumit Garg wrote: > > > > > > > > > > <snip> > > > > > > > > > > > > > > > > > - if (written >= FILE_SIZE) > > > > > > + if ((written >= tc->exp_sync_size) && > > > > > > + (written <= (tc->exp_sync_size + tc->exp_sync_size/10))) > > > > > > > > > > May I ask why it is +1/10 of expected sync_size as upper bound here, > > > > > since it looks like a magic number to me. > > > > > > > > It was an outcome of discussion here [1]. The reason being to test > > > > that only particular portion of file is written to device for whom > > > > sync has been invoked and +1/10 as upper bound to incorporate for any > > > > metadata. > > > > > > I see, thanks for explanation. > > > > > > > > > > > [1] https://patchwork.ozlabs.org/patch/1051647/ > > > > > > > > > > > > > > We encountered test failure in the second case in a debug kernel, > > > > > reproducible about once out of 20 times run. > > > > > > > > Interesting case. Can you share results after applying below patch? > > > > > > Tested this patch, no TFAIL occured in debug kernel after 200+ times > > > run, looks good to me. Thanks! Please add my > > > > From these results, the reason for the failure that you reported > > earlier seems to be writes to the device during "tst_fill_fd()" > > operation (they were found negligible/zero with normal kernel). But > > it's strange to know that you didn't get any TFAIL after the patch as > > I expected "Sync equals write" to fail. > > > > So can you also put following debug print and share logs of your test run? > > Retested with debug print, during my 1000-times run, pre-sync remains 0 > in all the other fs types except only ext4. For ext4 cases, pre-sync > could be non-zero, e.g.: > > Sync equals write: Synced 33554432, expected 33554432, pre-sync 0 > Sync inside of write: Synced 17301504, expected 16777216, pre-sync 1308672 > Sync overlaps with write: Synced 8650752, expected 8388608, pre-sync 1310720 > > Note that pre-sync could be non-zero in `equals writes` case sometimes > too, like another round below: > > Sync equals write: Synced 34078720, expected 33554432, pre-sync 260096 > Sync inside of write: Synced 17039360, expected 16777216, pre-sync 4980736 > Sync overlaps with write: Synced 8912896, expected 8388608, pre-sync 1048576 > > Such non-zero situation in ext4 case is reproducible ~10% of my > 1000-times run. > Thanks for your testing results. I am able to root-cause this issue. This issue seems to be caused by left over writes from previous test runs that are queued up for writes to the device during current test run leading to over-estimation of synced data to the device causing the failures that you have reported. So to avoid those left over writes from estimation, following additional call to "sync()" is required: diff --git a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c index eb08143c3..c66dbd8d2 100644 --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c @@ -48,6 +48,8 @@ static void verify_sync_file_range(struct testcase *tc) lseek(fd, tc->write_off, SEEK_SET); + sync(); + tst_dev_bytes_written(tst_device->dev); tst_fill_fd(fd, 0, TST_MB, tc->write_size_mb); Try to apply this fix at your end and let me know if you still observe any further failures with this test-case. -Sumit > Thanks, > Caspar > > > > > > > > --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > > +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > > @@ -68,6 +68,8 @@ static void verify_sync_file_range(struct testcase *tc) > > > > SAFE_CLOSE(fd); > > > > + printf("%s: Synced %li, expected %li, pre-sync %li\n", > > + tc->desc, written, tc->exp_sync_size, written_pre); > > if ((written >= tc->exp_sync_size) && > > (written <= (tc->exp_sync_size + tc->exp_sync_size/10))) > > tst_res(TPASS, "%s", tc->desc); > > > > -Sumit > > > > > > > > Reviewed-by: Caspar Zhang <caspar@linux.alibaba.com> > > > > > > directly if you're going to make a formal patch later. > > > > > > Thanks, > > > Caspar > > > > > > > > > > > diff --git a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > > > > b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > > > > index eb08143..1bc1a44 100644 > > > > --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > > > > +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c > > > > @@ -42,7 +42,7 @@ struct testcase { > > > > static void verify_sync_file_range(struct testcase *tc) > > > > { > > > > int fd; > > > > - unsigned long written; > > > > + unsigned long written, written_pre; > > > > > > > > fd = SAFE_OPEN(tc->fname, O_RDWR|O_CREAT, MODE); > > > > > > > > @@ -52,6 +52,8 @@ static void verify_sync_file_range(struct testcase *tc) > > > > > > > > tst_fill_fd(fd, 0, TST_MB, tc->write_size_mb); > > > > > > > > + written_pre = tst_dev_bytes_written(tst_device->dev); > > > > + > > > > TEST(sync_file_range(fd, tc->sync_off, tc->sync_size, > > > > SYNC_FILE_RANGE_WAIT_BEFORE | > > > > SYNC_FILE_RANGE_WRITE | > > > > @@ -70,8 +72,8 @@ static void verify_sync_file_range(struct testcase *tc) > > > > (written <= (tc->exp_sync_size + tc->exp_sync_size/10))) > > > > tst_res(TPASS, "%s", tc->desc); > > > > else > > > > - tst_res(TFAIL, "%s: Synced %li, expected %li", tc->desc, > > > > - written, tc->exp_sync_size); > > > > + tst_res(TFAIL, "%s: Synced %li, expected %li, pre-sync %li", > > > > + tc->desc, written, tc->exp_sync_size, written_pre); > > > > } > > > > > > > > static struct testcase testcases[] = { > > > > > > > > -Sumit > > > > > > > > > > > > > > The reason is unclear yet, however my guess is that more pages could be > > > > > written to disk in a debug kernel than a release kernel. > > > > > > > > > > My codes and config as below: > > > > > > > > > > tree: https://github.com/alibaba/cloud-kernel :: ck-4.19.67 branch; > > > > > config: https://github.com/alibaba/cloud-kernel/blob/master/config-4.19.y-x86_64-debug > > > > > > > > > > If you like you can build a test kernel on a KVM guest and try to > > > > > reproduce, or just run a RHEL8 kernel I guess (@Li Wang, you can have a > > > > > try on RHEL8 debug kernel if possible). > > > > > > > > > > a sample output: > > > > > > > > > > tst_device.c:87: INFO: Found free device 0 '/dev/loop0' > > > > > tst_supported_fs_types.c:60: INFO: Kernel supports ext2 > > > > > tst_supported_fs_types.c:44: INFO: mkfs.ext2 does exist > > > > > tst_supported_fs_types.c:60: INFO: Kernel supports ext3 > > > > > tst_supported_fs_types.c:44: INFO: mkfs.ext3 does exist > > > > > tst_supported_fs_types.c:60: INFO: Kernel supports ext4 > > > > > tst_supported_fs_types.c:44: INFO: mkfs.ext4 does exist > > > > > tst_supported_fs_types.c:60: INFO: Kernel supports xfs > > > > > tst_supported_fs_types.c:44: INFO: mkfs.xfs does exist > > > > > tst_supported_fs_types.c:60: INFO: Kernel supports btrfs > > > > > tst_supported_fs_types.c:44: INFO: mkfs.btrfs does exist > > > > > tst_supported_fs_types.c:60: INFO: Kernel supports vfat > > > > > tst_supported_fs_types.c:44: INFO: mkfs.vfat does exist > > > > > tst_supported_fs_types.c:83: INFO: Filesystem exfat is not supported > > > > > tst_supported_fs_types.c:83: INFO: Filesystem ntfs is not supported > > > > > tst_test.c:1179: INFO: Testing on ext2 > > > > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext2 opts='' extra opts='' > > > > > mke2fs 1.43.5 (04-Aug-2017) > > > > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > > > > sync_file_range02.c:71: PASS: Sync equals write > > > > > sync_file_range02.c:71: PASS: Sync inside of write > > > > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > > > tst_test.c:1179: INFO: Testing on ext3 > > > > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext3 opts='' extra opts='' > > > > > mke2fs 1.43.5 (04-Aug-2017) > > > > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > > > > sync_file_range02.c:71: PASS: Sync equals write > > > > > sync_file_range02.c:71: PASS: Sync inside of write > > > > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > > > tst_test.c:1179: INFO: Testing on ext4 > > > > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext4 opts='' extra opts='' > > > > > mke2fs 1.43.5 (04-Aug-2017) > > > > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > > > > sync_file_range02.c:71: PASS: Sync equals write > > > > > sync_file_range02.c:74: FAIL: Sync inside of write: Synced 19658752, expected 16777216 > > > > > ^^^^ > > > > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > > > tst_test.c:1179: INFO: Testing on xfs > > > > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with xfs opts='' extra opts='' > > > > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > > > > sync_file_range02.c:71: PASS: Sync equals write > > > > > sync_file_range02.c:71: PASS: Sync inside of write > > > > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > > > tst_test.c:1179: INFO: Testing on btrfs > > > > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with btrfs opts='' extra opts='' > > > > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > > > > sync_file_range02.c:71: PASS: Sync equals write > > > > > sync_file_range02.c:71: PASS: Sync inside of write > > > > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > > > tst_test.c:1179: INFO: Testing on vfat > > > > > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with vfat opts='' extra opts='' > > > > > tst_test.c:1118: INFO: Timeout per run is 0h 05m 00s > > > > > sync_file_range02.c:111: INFO: Pre-filling file > > > > > sync_file_range02.c:71: PASS: Sync equals write > > > > > sync_file_range02.c:71: PASS: Sync inside of write > > > > > sync_file_range02.c:71: PASS: Sync overlaps with write > > > > > > > > > > Summary: > > > > > passed 17 > > > > > failed 1 > > > > > skipped 0 > > > > > warnings 0 > > > > > > > > > > Any thoughts would be appreicated. > > > > > > > > > > Thanks, > > > > > Caspar > > > > > > > > > > > > > > > > tst_res(TPASS, "Test file range synced to device"); > > > > > > else > > > > > > - tst_res(TFAIL, "Synced %li, expected %i", written, FILE_SIZE); > > > > > > + tst_res(TFAIL, "Synced %li, expected %li", written, > > > > > > + tc->exp_sync_size); > > > > > > +} > > > > > > + > > > > > > +static struct testcase testcases[] = { > > > > > > + { FNAME1, 0, FILE_SZ, FILE_SZ, 0, FILE_SZ_MB }, > > > > > > + { FNAME2, FILE_SZ/4, FILE_SZ/2, FILE_SZ/2, 0, FILE_SZ_MB }, > > > > > > + { FNAME3, FILE_SZ/4, FILE_SZ/2, FILE_SZ/4, FILE_SZ/2, FILE_SZ_MB/4 }, > > > > > > +}; > > > > > > + > > > > > > +static void run(unsigned int i) > > > > > > +{ > > > > > > + verify_sync_file_range(&testcases[i]); > > > > > > } > > > > > > > > > > > > static void setup(void) > > > > > > { > > > > > > if (!check_sync_file_range()) > > > > > > tst_brk(TCONF, "sync_file_range() not supported"); > > > > > > + > > > > > > + if (!strcmp(tst_device->fs_type, "vfat")) { > > > > > > + tst_res(TINFO, "Pre-filling file"); > > > > > > + tst_fill_file(FNAME3, 0, TST_MB, FILE_SZ_MB); > > > > > > + sync(); > > > > > > + } > > > > > > } > > > > > > > > > > > > static struct tst_test test = { > > > > > > + .tcnt = ARRAY_SIZE(testcases), > > > > > > .needs_root = 1, > > > > > > .mount_device = 1, > > > > > > .all_filesystems = 1, > > > > > > .mntpoint = MNTPOINT, > > > > > > .setup = setup, > > > > > > - .test_all = verify_sync_file_range, > > > > > > + .test = run, > > > > > > }; > > > > > > -- > > > > > > 2.7.4 > > > > > > > > > > > > > > > > > > -- > > > > > > Mailing list info: https://lists.linux.it/listinfo/ltp > > > > > > > > > > -- > > > > > Thanks, > > > > > Caspar > > > > > > -- > > > Thanks, > > > Caspar > > -- > Thanks, > Caspar -- Mailing list info: https://lists.linux.it/listinfo/ltp
Hi Naresh, On Tue, 29 Oct 2019 at 11:39, Naresh Kamboju <naresh.kamboju@linaro.org> wrote: > > Hi Caspar and Sumit, > > > > > > > We encountered test failure in the second case in a debug kernel, > > > > > > reproducible about once out of 20 times run. > > > > > > > > > > Interesting case. Can you share results after applying below patch? > > > > > > > > Tested this patch, no TFAIL occured in debug kernel after 200+ times > > > > run, looks good to me. Thanks! Please add my > > > > > > From these results, the reason for the failure that you reported > > > earlier seems to be writes to the device during "tst_fill_fd()" > > > operation (they were found negligible/zero with normal kernel). But > > > it's strange to know that you didn't get any TFAIL after the patch as > > > I expected "Sync equals write" to fail. > > > > > > So can you also put following debug print and share logs of your test run? > > > > Retested with debug print, during my 1000-times run, pre-sync remains 0 > > in all the other fs types except only ext4. For ext4 cases, pre-sync > > could be non-zero, e.g.: > > > > Sync equals write: Synced 33554432, expected 33554432, pre-sync 0 > > Sync inside of write: Synced 17301504, expected 16777216, pre-sync 1308672 > > Sync overlaps with write: Synced 8650752, expected 8388608, pre-sync 1310720 > > > > Note that pre-sync could be non-zero in `equals writes` case sometimes > > too, like another round below: > > > > Sync equals write: Synced 34078720, expected 33554432, pre-sync 260096 > > Sync inside of write: Synced 17039360, expected 16777216, pre-sync 4980736 > > Sync overlaps with write: Synced 8912896, expected 8388608, pre-sync 1048576 > > > > Such non-zero situation in ext4 case is reproducible ~10% of my > > 1000-times run. > > sync_file_range02 test failure reproduced on mainline and stable rc branches > 5.3, 4.19, 4.14 on arm64 and arm devices while testing on ext4. > Can you test again with the fix proposed here [1]? [1] http://lists.linux.it/pipermail/ltp/2019-October/014157.html -Sumit > output log, > -------------- > tst_test.c:1179: INFO: Testing on ext2 > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext2 opts='' extra opts='' > mke2fs 1.43.8 (1-Jan-2018) > tst_test.c:1118: INFO: Timeout per run is 0h 15m 00s > sync_file_range02.c:71: PASS: Sync equals write > sync_file_range02.c:71: PASS: Sync inside of write > sync_file_range02.c:71: PASS: Sync overlaps with write > tst_test.c:1179: INFO: Testing on ext3 > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext3 opts='' extra opts='' > mke2fs 1.43.8 (1-Jan-2018) > Listened to connection for namespace 'tlxc' done > [ 1349.061989] EXT4-fs (loop0): mounting ext3 file system using the > ext4 subsystem > [ 1349.099564] EXT4-fs (loop0): mounted filesystem with ordered data > mode. Opts: (null) > tst_test.c:1118: INFO: Timeout per run is 0h 15m 00s > sync_file_range02.c:71: PASS: Sync equals write > Listened to connection for namespace 'tlxc' done > sync_file_range02.c:71: PASS: Sync inside of write > sync_file_range02.c:71: PASS: Sync overlaps with write > tst_test.c:1179: INFO: Testing on ext4 > tst_mkfs.c:90: INFO: Formatting /dev/loop0 with ext4 opts='' extra opts='' > mke2fs 1.43.8 (1-Jan-2018) > [ 1362.579639] EXT4-fs (loop0): mounted filesystem with ordered data > mode. Opts: (null) > tst_test.c:1118: INFO: Timeout per run is 0h 15m 00s > sync_file_range02.c:74: FAIL: Sync equals write: Synced 36960256, > expected 33554432 > Listened to connection for namespace 'tlxc' done > sync_file_range02.c:74: FAIL: Sync inside of write: Synced 20185088, > expected 16777216 > sync_file_range02.c:71: PASS: Sync overlaps with write > Summary: > passed 7 > failed 2 > > Full output log, > -------------------- > https://lkft.validation.linaro.org/scheduler/job/983166#L15067 > > - Naresh -- Mailing list info: https://lists.linux.it/listinfo/ltp
Hi Sumit, On Wed, 30 Oct 2019 at 15:15, Sumit Garg <sumit.garg@linaro.org> wrote: > > Hi Naresh, > > Can you test again with the fix proposed here [1]? I have tested with patch applied but still test fails intermittently. https://lkft.validation.linaro.org/scheduler/job/991437#L3683 > > [1] http://lists.linux.it/pipermail/ltp/2019-October/014157.html > > -Sumit - Naresh -- Mailing list info: https://lists.linux.it/listinfo/ltp
diff --git a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c index 82d77f7..9454a56 100644 --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c @@ -22,23 +22,36 @@ #include "check_sync_file_range.h" #define MNTPOINT "mnt_point" -#define FNAME MNTPOINT"/test" -#define FILE_SIZE_MB 32 -#define FILE_SIZE (FILE_SIZE_MB * TST_MB) +#define FNAME1 MNTPOINT"/test1" +#define FNAME2 MNTPOINT"/test2" +#define FNAME3 MNTPOINT"/test3" +#define FILE_SZ_MB 32 +#define FILE_SZ (FILE_SZ_MB * TST_MB) #define MODE 0644 -static void verify_sync_file_range(void) +struct testcase { + char *fname; + off64_t sync_off; + off64_t sync_size; + size_t exp_sync_size; + off64_t write_off; + size_t write_size_mb; +}; + +static void verify_sync_file_range(struct testcase *tc) { int fd; unsigned long written; - fd = SAFE_OPEN(FNAME, O_RDWR|O_CREAT, MODE); + fd = SAFE_OPEN(tc->fname, O_RDWR|O_CREAT, MODE); + + lseek(fd, tc->write_off, SEEK_SET); tst_dev_bytes_written(tst_device->dev); - tst_fill_fd(fd, 0, TST_MB, FILE_SIZE_MB); + tst_fill_fd(fd, 0, TST_MB, tc->write_size_mb); - TEST(sync_file_range(fd, 0, FILE_SIZE, + TEST(sync_file_range(fd, tc->sync_off, tc->sync_size, SYNC_FILE_RANGE_WAIT_BEFORE | SYNC_FILE_RANGE_WRITE | SYNC_FILE_RANGE_WAIT_AFTER)); @@ -50,23 +63,43 @@ static void verify_sync_file_range(void) SAFE_CLOSE(fd); - if (written >= FILE_SIZE) + if ((written >= tc->exp_sync_size) && + (written <= (tc->exp_sync_size + tc->exp_sync_size/10))) tst_res(TPASS, "Test file range synced to device"); else - tst_res(TFAIL, "Synced %li, expected %i", written, FILE_SIZE); + tst_res(TFAIL, "Synced %li, expected %li", written, + tc->exp_sync_size); +} + +static struct testcase testcases[] = { + { FNAME1, 0, FILE_SZ, FILE_SZ, 0, FILE_SZ_MB }, + { FNAME2, FILE_SZ/4, FILE_SZ/2, FILE_SZ/2, 0, FILE_SZ_MB }, + { FNAME3, FILE_SZ/4, FILE_SZ/2, FILE_SZ/4, FILE_SZ/2, FILE_SZ_MB/4 }, +}; + +static void run(unsigned int i) +{ + verify_sync_file_range(&testcases[i]); } static void setup(void) { if (!check_sync_file_range()) tst_brk(TCONF, "sync_file_range() not supported"); + + if (!strcmp(tst_device->fs_type, "vfat")) { + tst_res(TINFO, "Pre-filling file"); + tst_fill_file(FNAME3, 0, TST_MB, FILE_SZ_MB); + sync(); + } } static struct tst_test test = { + .tcnt = ARRAY_SIZE(testcases), .needs_root = 1, .mount_device = 1, .all_filesystems = 1, .mntpoint = MNTPOINT, .setup = setup, - .test_all = verify_sync_file_range, + .test = run, };
Add partial file sync tests as part of sync_file_range02 test-case. Signed-off-by: Sumit Garg <sumit.garg@linaro.org> --- Changes in v4: vfat FS doesn't support sparse files. So handle this via pre-filling the test file in case of "test3". Changes in v3: 1. Add upper bound check for synced size to device. 2. Refactor tests for more code reuse. 3. Add another test to check sync over partial write. Changes in v2: 1. Do full file write instead of partial and test sync partial file. .../syscalls/sync_file_range/sync_file_range02.c | 53 ++++++++++++++++++---- 1 file changed, 43 insertions(+), 10 deletions(-)