[LTP] [PATCH v3] syscalls/sync_file_range: add partial file sync test-cases

[LTP] [PATCH v3] syscalls/sync_file_range: add partial file sync test-cases

[Sumit Garg]

Add partial file sync tests as part of sync_file_range02 test-case.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---

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   | 47 +++++++++++++++++-----
 1 file changed, 37 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..334ea5e 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,10 +63,23 @@ 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)
@@ -63,10 +89,11 @@ static void setup(void)
 }
 
 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

[LTP] [PATCH v3] syscalls/sync_file_range: add partial file sync test-cases

[Cyril Hrubis]

Hi!
Sorry for the long delay.

This is altmost perfect, the only problem is that the third test fails
on vfat. As far as I can tell the reason is that vfat does not support
sparse files, hence seeking to the middle of file and writing there also
schedulles I/O to write zeros from the start of the file to the offset
we started writing to.

Following ugly patch solves the problem:

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 334ea5e88..774524c2f 100644
--- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
+++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
@@ -45,6 +45,12 @@ static void verify_sync_file_range(struct testcase *tc)
 
        fd = SAFE_OPEN(tc->fname, O_RDWR|O_CREAT, MODE);
 
+       if (!strcmp(tst_device->fs_type, "vfat")) {
+               tst_res(TINFO, "Pre-filling file");
+               tst_fill_fd(fd, 0, tc->write_off, 1);
+               fsync(fd);
+       }
+
        lseek(fd, tc->write_off, SEEK_SET);
 

So either we limit the tests so that the sync region does not overlap with the
possible hole at the start of the file and loose some test coverage.

Or we can add a function to the test library that would return true/false if
sparse files are supported for a given FS.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3] syscalls/sync_file_range: add partial file sync test-cases

[Sumit Garg]

On Wed, 27 Mar 2019 at 20:18, Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> Sorry for the long delay.
>
> This is altmost perfect, the only problem is that the third test fails
> on vfat. As far as I can tell the reason is that vfat does not support
> sparse files, hence seeking to the middle of file and writing there also
> schedulles I/O to write zeros from the start of the file to the offset
> we started writing to.
>

Hmm, I see.

> Following ugly patch solves the problem:
>
> 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 334ea5e88..774524c2f 100644
> --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
> +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
> @@ -45,6 +45,12 @@ static void verify_sync_file_range(struct testcase *tc)
>
>         fd = SAFE_OPEN(tc->fname, O_RDWR|O_CREAT, MODE);
>
> +       if (!strcmp(tst_device->fs_type, "vfat")) {
> +               tst_res(TINFO, "Pre-filling file");
> +               tst_fill_fd(fd, 0, tc->write_off, 1);
> +               fsync(fd);
> +       }
> +
>         lseek(fd, tc->write_off, SEEK_SET);
>
>
> So either we limit the tests so that the sync region does not overlap with the
> possible hole at the start of the file and loose some test coverage.
>
> Or we can add a function to the test library that would return true/false if
> sparse files are supported for a given FS.
>

My initial thought behind this test-case was to run sync over a range
which is partially written. The other partial region not being written
could either be a hole or already synced data. So pre-fill file in
case of vfat looks sane option, but how about if we add pre-fill as
part of setup? Something like:

--- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
+++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
@@ -86,6 +86,12 @@ 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();
+       }
 }

-Sumit

> --
> Cyril Hrubis
> chrubis@suse.cz

[LTP] [PATCH v3] syscalls/sync_file_range: add partial file sync test-cases

[Li Wang]

Hi Sumit,

On Thu, Mar 7, 2019 at 8:44 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 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   | 47
> +++++++++++++++++-----
>  1 file changed, 37 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..334ea5e 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);
>

I'm just thinking that is probably more precise if we reverse the order of
tst_fill_fd() and tst_dev_bytes_written()? Because that does counting the
dev_bytes_writen only before and after sync_file_range(), we cann't
garantee system does not wirte back to deviece when do fill_fd(), isn't it?


>
> -       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,10 +63,23 @@ 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)
> @@ -63,10 +89,11 @@ static void setup(void)
>  }
>
>  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
>


-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20190401/2c6a5a02/attachment.html>

[LTP] [PATCH v3] syscalls/sync_file_range: add partial file sync test-cases

[Sumit Garg]

Hi Li,

Firstly apologies for the late reply due to travelling for Linaro Connect BKK19.

On Mon, 1 Apr 2019 at 13:54, Li Wang <liwang@redhat.com> wrote:
>
> Hi Sumit,
>
> On Thu, Mar 7, 2019 at 8:44 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 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   | 47 +++++++++++++++++-----
>>  1 file changed, 37 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..334ea5e 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);
>
>
> I'm just thinking that is probably more precise if we reverse the order of tst_fill_fd() and tst_dev_bytes_written()? Because that does counting the dev_bytes_writen only before and after sync_file_range(), we cann't garantee system does not wirte back to deviece when do fill_fd(), isn't it?

There is another aspect to this if we move tst_dev_bytes_written()
after tst_fill_fd() then we may miss count for actual data that may be
written back during tst_fill_fd() operation.

AFAIU, LTP uses test-device for all device specific tests of which it
has full control and also the tests run sequentially. So I think there
are pretty rare chances to have such scenario that you referred too.

-Sumit

>
>>
>>
>> -       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,10 +63,23 @@ 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)
>> @@ -63,10 +89,11 @@ static void setup(void)
>>  }
>>
>>  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
>
>
>
> --
> Regards,
> Li Wang

[LTP] [PATCH v3] syscalls/sync_file_range: add partial file sync test-cases

[Li Wang]

On Wed, Apr 3, 2019 at 7:17 PM Sumit Garg <sumit.garg@linaro.org> wrote:

> Hi Li,
>
> Firstly apologies for the late reply due to travelling for Linaro Connect
> BKK19.
>
> On Mon, 1 Apr 2019 at 13:54, Li Wang <liwang@redhat.com> wrote:
> >
> > Hi Sumit,
> >
> > On Thu, Mar 7, 2019 at 8:44 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 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   | 47
> +++++++++++++++++-----
> >>  1 file changed, 37 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..334ea5e 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);
> >
> >
> > I'm just thinking that is probably more precise if we reverse the order
> of tst_fill_fd() and tst_dev_bytes_written()? Because that does counting
> the dev_bytes_writen only before and after sync_file_range(), we cann't
> garantee system does not wirte back to deviece when do fill_fd(), isn't it?
>
> There is another aspect to this if we move tst_dev_bytes_written()
> after tst_fill_fd() then we may miss count for actual data that may be
> written back during tst_fill_fd() operation.
>
>
Sounds reasonable, thanks for the clarification.


> AFAIU, LTP uses test-device for all device specific tests of which it
> has full control and also the tests run sequentially. So I think there
> are pretty rare chances to have such scenario that you referred too.
>
> -Sumit
>

-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20190418/ec9ddc16/attachment-0001.html>

[LTP] [PATCH v3] syscalls/sync_file_range: add partial file sync test-cases

[Li Wang]

On Thu, Mar 28, 2019 at 12:57 PM Sumit Garg <sumit.garg@linaro.org> wrote:

> On Wed, 27 Mar 2019 at 20:18, Cyril Hrubis <chrubis@suse.cz> wrote:
> >
> > Hi!
> > Sorry for the long delay.
> >
> > This is altmost perfect, the only problem is that the third test fails
> > on vfat. As far as I can tell the reason is that vfat does not support
> > sparse files, hence seeking to the middle of file and writing there also
> > schedulles I/O to write zeros from the start of the file to the offset
> > we started writing to.
> >
>
> Hmm, I see.
>
> > Following ugly patch solves the problem:
> >
> > 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 334ea5e88..774524c2f 100644
> > --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
> > +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
> > @@ -45,6 +45,12 @@ static void verify_sync_file_range(struct testcase
> *tc)
> >
> >         fd = SAFE_OPEN(tc->fname, O_RDWR|O_CREAT, MODE);
> >
> > +       if (!strcmp(tst_device->fs_type, "vfat")) {
> > +               tst_res(TINFO, "Pre-filling file");
> > +               tst_fill_fd(fd, 0, tc->write_off, 1);
> > +               fsync(fd);
> > +       }
> > +
> >         lseek(fd, tc->write_off, SEEK_SET);
> >
> >
> > So either we limit the tests so that the sync region does not overlap
> with the
> > possible hole at the start of the file and loose some test coverage.
> >
> > Or we can add a function to the test library that would return
> true/false if
> > sparse files are supported for a given FS.
> >
>
> My initial thought behind this test-case was to run sync over a range
> which is partially written. The other partial region not being written
> could either be a hole or already synced data. So pre-fill file in
> case of vfat looks sane option, but how about if we add pre-fill as
> part of setup? Something like:
>

I think this is a bit better. Could u send a new patch version?


> --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
> +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
> @@ -86,6 +86,12 @@ 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();
> +       }
>  }
>
> -Sumit
>
> > --
> > Cyril Hrubis
> > chrubis@suse.cz
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>


-- 
Regards,
Li Wang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linux.it/pipermail/ltp/attachments/20190610/e8d4b01a/attachment-0001.html>

[LTP] [PATCH v3] syscalls/sync_file_range: add partial file sync test-cases

[Sumit Garg]

On Mon, 10 Jun 2019 at 09:03, Li Wang <liwang@redhat.com> wrote:
>
>
>
> On Thu, Mar 28, 2019 at 12:57 PM Sumit Garg <sumit.garg@linaro.org> wrote:
>>
>> On Wed, 27 Mar 2019 at 20:18, Cyril Hrubis <chrubis@suse.cz> wrote:
>> >
>> > Hi!
>> > Sorry for the long delay.
>> >
>> > This is altmost perfect, the only problem is that the third test fails
>> > on vfat. As far as I can tell the reason is that vfat does not support
>> > sparse files, hence seeking to the middle of file and writing there also
>> > schedulles I/O to write zeros from the start of the file to the offset
>> > we started writing to.
>> >
>>
>> Hmm, I see.
>>
>> > Following ugly patch solves the problem:
>> >
>> > 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 334ea5e88..774524c2f 100644
>> > --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
>> > +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
>> > @@ -45,6 +45,12 @@ static void verify_sync_file_range(struct testcase *tc)
>> >
>> >         fd = SAFE_OPEN(tc->fname, O_RDWR|O_CREAT, MODE);
>> >
>> > +       if (!strcmp(tst_device->fs_type, "vfat")) {
>> > +               tst_res(TINFO, "Pre-filling file");
>> > +               tst_fill_fd(fd, 0, tc->write_off, 1);
>> > +               fsync(fd);
>> > +       }
>> > +
>> >         lseek(fd, tc->write_off, SEEK_SET);
>> >
>> >
>> > So either we limit the tests so that the sync region does not overlap with the
>> > possible hole at the start of the file and loose some test coverage.
>> >
>> > Or we can add a function to the test library that would return true/false if
>> > sparse files are supported for a given FS.
>> >
>>
>> My initial thought behind this test-case was to run sync over a range
>> which is partially written. The other partial region not being written
>> could either be a hole or already synced data. So pre-fill file in
>> case of vfat looks sane option, but how about if we add pre-fill as
>> part of setup? Something like:
>
>
> I think this is a bit better. Could u send a new patch version?
>

Sure. BTW, apologies for the delay as I was busy with some other tasks.

-Sumit

>>
>> --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
>> +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
>> @@ -86,6 +86,12 @@ 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();
>> +       }
>>  }
>>
>> -Sumit
>>
>> > --
>> > Cyril Hrubis
>> > chrubis@suse.cz
>>
>> --
>> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>
>
> --
> Regards,
> Li Wang