[v4] syscalls/sync_file_range: add partial file sync test-cases

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
Related show

Commit Message

Sumit Garg June 10, 2019, 10:13 a.m.
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(-)

Comments

Li Wang June 10, 2019, 12:04 p.m. | #1
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 &lt;<a href="mailto:sumit.garg@linaro.org">sumit.garg@linaro.org</a>&gt; 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 &lt;<a href="mailto:sumit.garg@linaro.org" target="_blank">sumit.garg@linaro.org</a>&gt;<br>

---<br>
<br>
Changes in v4:<br>
vfat FS doesn&#39;t support sparse files. So handle this via pre-filling the<br>
test file in case of &quot;test3&quot;.<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 &quot;check_sync_file_range.h&quot;<br>
<br>
 #define MNTPOINT               &quot;mnt_point&quot;<br>
-#define FNAME          MNTPOINT&quot;/test&quot;<br>
-#define FILE_SIZE_MB   32<br>
-#define FILE_SIZE (FILE_SIZE_MB * TST_MB)<br>
+#define FNAME1                 MNTPOINT&quot;/test1&quot;<br>
+#define FNAME2                 MNTPOINT&quot;/test2&quot;<br>
+#define FNAME3                 MNTPOINT&quot;/test3&quot;<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-&gt;fname, O_RDWR|O_CREAT, MODE);<br>
+<br>
+       lseek(fd, tc-&gt;write_off, SEEK_SET);<br>
<br>
        tst_dev_bytes_written(tst_device-&gt;dev);<br>
<br>
-       tst_fill_fd(fd, 0, TST_MB, FILE_SIZE_MB);<br>
+       tst_fill_fd(fd, 0, TST_MB, tc-&gt;write_size_mb);<br>
<br>
-       TEST(sync_file_range(fd, 0, FILE_SIZE,<br>
+       TEST(sync_file_range(fd, tc-&gt;sync_off, tc-&gt;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 &gt;= FILE_SIZE)<br>
+       if ((written &gt;= tc-&gt;exp_sync_size) &amp;&amp;<br>
+           (written &lt;= (tc-&gt;exp_sync_size + tc-&gt;exp_sync_size/10)))<br>
                tst_res(TPASS, &quot;Test file range synced to device&quot;);<br>
        else<br>
-               tst_res(TFAIL, &quot;Synced %li, expected %i&quot;, written, FILE_SIZE);<br>
+               tst_res(TFAIL, &quot;Synced %li, expected %li&quot;, written,<br>
+                       tc-&gt;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(&amp;testcases[i]);<br>
 }<br>
<br>
 static void setup(void)<br>
 {<br>
        if (!check_sync_file_range())<br>
                tst_brk(TCONF, &quot;sync_file_range() not supported&quot;);<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Reviewed-by: Li Wang &lt;<a href="mailto:liwang@redhat.com">liwang@redhat.com</a>&gt;</div><br></div><div><div class="gmail_default" style="font-size:small">That&#39;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&#39;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 &quot;test3&quot;. </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-&gt;fs_type, &quot;vfat&quot;)) {<br>
+               tst_res(TINFO, &quot;Pre-filling file&quot;);<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>
Sumit Garg June 17, 2019, 12:26 p.m. | #2
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
Cyril Hrubis June 17, 2019, 1:06 p.m. | #3
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.
Cyril Hrubis June 17, 2019, 3:06 p.m. | #4
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.

 }
Amir Goldstein June 18, 2019, 3:49 p.m. | #5
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.
Cyril Hrubis June 19, 2019, 9:58 a.m. | #6
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?
Amir Goldstein June 19, 2019, 10:28 a.m. | #7
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.
Caspar Zhang Sept. 29, 2019, 1:27 p.m. | #8
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
Sumit Garg Oct. 4, 2019, 7:33 a.m. | #9
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
Caspar Zhang Oct. 16, 2019, 2:54 a.m. | #10
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
Sumit Garg Oct. 16, 2019, 6:23 a.m. | #11
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
Caspar Zhang Oct. 17, 2019, 7:36 a.m. | #12
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
Naresh Kamboju Oct. 29, 2019, 6:08 a.m. | #13
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
Sumit Garg Oct. 30, 2019, 9:40 a.m. | #14
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
Sumit Garg Oct. 30, 2019, 9:45 a.m. | #15
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
Naresh Kamboju Nov. 5, 2019, 9:05 a.m. | #16
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

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 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,
 };