[v2] syscalls: add syscall syncfs test

Message ID 1550215053-6795-1-git-send-email-sumit.garg@linaro.org
State New
Headers show
Series
  • [v2] syscalls: add syscall syncfs test
Related show

Commit Message

Sumit Garg Feb. 15, 2019, 7:17 a.m.
syncfs01 tests to sync filesystem having large dirty file pages to block
device. Also, it tests all supported filesystems on a test block device.

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

Changes in v2:
1. Remove unused header file include.
2. Remove redundant tst_device check.
3. Remove redundant flags from tst_test struct.

Fixes: https://github.com/linux-test-project/ltp/issues/294

 runtest/syscalls                            |   2 +
 testcases/kernel/syscalls/syncfs/.gitignore |   1 +
 testcases/kernel/syscalls/syncfs/Makefile   |   8 +++
 testcases/kernel/syscalls/syncfs/syncfs01.c | 101 ++++++++++++++++++++++++++++
 4 files changed, 112 insertions(+)
 create mode 100644 testcases/kernel/syscalls/syncfs/.gitignore
 create mode 100644 testcases/kernel/syscalls/syncfs/Makefile
 create mode 100644 testcases/kernel/syscalls/syncfs/syncfs01.c

Comments

Xiao Yang Feb. 15, 2019, 7:55 a.m. | #1
Hi Sumit,

Sorry for the late reply. :-)

1) Compilation failed on older kernel(e.g. v2.6.32) because of the
undefined syncfs().
According to manpage, syncfs() was first appeared in Linux 2.6.39 and
library
support was added to glibc in version 2.14. Perhaps, we need to check if
syncfs()
is defined.

2) Running this test got TBROK on older kernel(e.g. v3.10.0), as below:
-------------------------------------------------------------------
syncfs01.c:63: FAIL: Failed to sync test filesystem to device
-------------------------------------------------------------------
It seems that the content of /sys/block/<loopX>/stat is always zero and
doesn't update even though write and sync have been done. I am looking
into it, but i am not sure if this is a known bug on older kernel.

Best Regards,
Xiao Yang
On 2019/02/15 15:17, Sumit Garg wrote:
> syncfs01 tests to sync filesystem having large dirty file pages to block
> device. Also, it tests all supported filesystems on a test block device.
>
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> ---
>
> Changes in v2:
> 1. Remove unused header file include.
> 2. Remove redundant tst_device check.
> 3. Remove redundant flags from tst_test struct.
>
> Fixes: https://github.com/linux-test-project/ltp/issues/294
>
>  runtest/syscalls                            |   2 +
>  testcases/kernel/syscalls/syncfs/.gitignore |   1 +
>  testcases/kernel/syscalls/syncfs/Makefile   |   8 +++
>  testcases/kernel/syscalls/syncfs/syncfs01.c | 101 ++++++++++++++++++++++++++++
>  4 files changed, 112 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/syncfs/.gitignore
>  create mode 100644 testcases/kernel/syscalls/syncfs/Makefile
>  create mode 100644 testcases/kernel/syscalls/syncfs/syncfs01.c
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 668c87c..9442740 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1346,6 +1346,8 @@ symlinkat01 symlinkat01
>  sync01 sync01
>  sync02 sync02
>  
> +syncfs01 syncfs01
> +
>  #testcases for sync_file_range
>  sync_file_range01 sync_file_range01
>  
> diff --git a/testcases/kernel/syscalls/syncfs/.gitignore b/testcases/kernel/syscalls/syncfs/.gitignore
> new file mode 100644
> index 0000000..6066295
> --- /dev/null
> +++ b/testcases/kernel/syscalls/syncfs/.gitignore
> @@ -0,0 +1 @@
> +syncfs01
> diff --git a/testcases/kernel/syscalls/syncfs/Makefile b/testcases/kernel/syscalls/syncfs/Makefile
> new file mode 100644
> index 0000000..3e6c2f4
> --- /dev/null
> +++ b/testcases/kernel/syscalls/syncfs/Makefile
> @@ -0,0 +1,8 @@
> +# Copyright (c) 2019 - Linaro Limited. All rights reserved.
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +top_srcdir             ?= ../../../..
> +
> +include $(top_srcdir)/include/mk/testcases.mk
> +
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/syscalls/syncfs/syncfs01.c b/testcases/kernel/syscalls/syncfs/syncfs01.c
> new file mode 100644
> index 0000000..e2d3e80
> --- /dev/null
> +++ b/testcases/kernel/syscalls/syncfs/syncfs01.c
> @@ -0,0 +1,101 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019 Linaro Limited. All rights reserved.
> + * Author: Sumit Garg <sumit.garg@linaro.org>
> + */
> +
> +/*
> + * Test syncfs
> + *
> + * It basically tests syncfs() to sync filesystem having large dirty file
> + * pages to block device. Also, it tests all supported filesystems on a test
> + * block device.
> + */
> +
> +#define _GNU_SOURCE
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <sys/types.h>
> +#include "tst_test.h"
> +#include "lapi/fs.h"
> +#include "lapi/stat.h"
> +
> +#define MNTPOINT		"mnt_point"
> +#define TST_FILE		MNTPOINT"/test"
> +#define TST_FILE_SIZE_MB	32
> +#define SIZE_MB			(1024*1024)
> +#define MODE			0644
> +
> +static char dev_stat_path[1024];
> +static char *buffer;
> +static int fd;
> +
> +static void verify_syncfs(void)
> +{
> +	char nwrite_sec_val[BUFSIZ];
> +	int counter;
> +	unsigned long prev_write_sec = 0, write_sec = 0;
> +
> +	SAFE_FILE_SCANF(dev_stat_path, "%*s %*s %*s %*s %*s %*s %s",
> +			nwrite_sec_val);
> +
> +	prev_write_sec = SAFE_STRTOUL(nwrite_sec_val, 0, ULONG_MAX);
> +
> +	fd =  SAFE_OPEN(TST_FILE, O_RDWR|O_CREAT, MODE);
> +
> +	/* Filling the test file */
> +	for (counter = 0; counter < TST_FILE_SIZE_MB; counter++)
> +		SAFE_WRITE(1, fd, buffer, SIZE_MB);
> +
> +	TEST(syncfs(fd));
> +	if (TST_RET != 0)
> +		tst_brk(TFAIL | TTERRNO, "syncfs(fd) failed");
> +
> +	SAFE_FILE_SCANF(dev_stat_path, "%*s %*s %*s %*s %*s %*s %s",
> +			nwrite_sec_val);
> +
> +	write_sec = SAFE_STRTOUL(nwrite_sec_val, 0, ULONG_MAX);
> +
> +	if ((write_sec - prev_write_sec) * 512 >=
> +	    (TST_FILE_SIZE_MB * SIZE_MB))
> +		tst_res(TPASS, "Test filesystem synced to device");
> +	else
> +		tst_res(TFAIL, "Failed to sync test filesystem to device");
> +
> +	SAFE_CLOSE(fd);
> +}
> +
> +static void setup(void)
> +{
> +	struct stat st;
> +
> +	snprintf(dev_stat_path, sizeof(dev_stat_path), "/sys/block/%s/stat",
> +		 strrchr(tst_device->dev, '/') + 1);
> +
> +	if (stat(dev_stat_path, &st) != 0)
> +		tst_brk(TCONF, "Test device stat file: %s not found",
> +			dev_stat_path);
> +
> +	buffer = SAFE_MALLOC(SIZE_MB);
> +
> +	memset(buffer, 0, SIZE_MB);
> +}
> +
> +static void cleanup(void)
> +{
> +	if (buffer)
> +		free(buffer);
> +
> +	if (fd > 0)
> +		SAFE_CLOSE(fd);
> +}
> +
> +static struct tst_test test = {
> +	.needs_root = 1,
> +	.mount_device = 1,
> +	.all_filesystems = 1,
> +	.mntpoint = MNTPOINT,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = verify_syncfs,
> +};
Amir Goldstein Feb. 15, 2019, 8:42 a.m. | #2
On Fri, Feb 15, 2019 at 9:17 AM Sumit Garg <sumit.garg@linaro.org> wrote:
>
> syncfs01 tests to sync filesystem having large dirty file pages to block
> device. Also, it tests all supported filesystems on a test block device.
>
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> ---
>
> Changes in v2:
> 1. Remove unused header file include.
> 2. Remove redundant tst_device check.
> 3. Remove redundant flags from tst_test struct.
>
> Fixes: https://github.com/linux-test-project/ltp/issues/294
>
>  runtest/syscalls                            |   2 +
>  testcases/kernel/syscalls/syncfs/.gitignore |   1 +
>  testcases/kernel/syscalls/syncfs/Makefile   |   8 +++
>  testcases/kernel/syscalls/syncfs/syncfs01.c | 101 ++++++++++++++++++++++++++++
>  4 files changed, 112 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/syncfs/.gitignore
>  create mode 100644 testcases/kernel/syscalls/syncfs/Makefile
>  create mode 100644 testcases/kernel/syscalls/syncfs/syncfs01.c
>
> diff --git a/runtest/syscalls b/runtest/syscalls
> index 668c87c..9442740 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1346,6 +1346,8 @@ symlinkat01 symlinkat01
>  sync01 sync01
>  sync02 sync02
>
> +syncfs01 syncfs01
> +
>  #testcases for sync_file_range
>  sync_file_range01 sync_file_range01
>
> diff --git a/testcases/kernel/syscalls/syncfs/.gitignore b/testcases/kernel/syscalls/syncfs/.gitignore
> new file mode 100644
> index 0000000..6066295
> --- /dev/null
> +++ b/testcases/kernel/syscalls/syncfs/.gitignore
> @@ -0,0 +1 @@
> +syncfs01
> diff --git a/testcases/kernel/syscalls/syncfs/Makefile b/testcases/kernel/syscalls/syncfs/Makefile
> new file mode 100644
> index 0000000..3e6c2f4
> --- /dev/null
> +++ b/testcases/kernel/syscalls/syncfs/Makefile
> @@ -0,0 +1,8 @@
> +# Copyright (c) 2019 - Linaro Limited. All rights reserved.
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +top_srcdir             ?= ../../../..
> +
> +include $(top_srcdir)/include/mk/testcases.mk
> +
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> diff --git a/testcases/kernel/syscalls/syncfs/syncfs01.c b/testcases/kernel/syscalls/syncfs/syncfs01.c
> new file mode 100644
> index 0000000..e2d3e80
> --- /dev/null
> +++ b/testcases/kernel/syscalls/syncfs/syncfs01.c
> @@ -0,0 +1,101 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019 Linaro Limited. All rights reserved.
> + * Author: Sumit Garg <sumit.garg@linaro.org>
> + */
> +
> +/*
> + * Test syncfs
> + *
> + * It basically tests syncfs() to sync filesystem having large dirty file
> + * pages to block device. Also, it tests all supported filesystems on a test
> + * block device.
> + */
> +
> +#define _GNU_SOURCE
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <sys/types.h>
> +#include "tst_test.h"
> +#include "lapi/fs.h"
> +#include "lapi/stat.h"
> +
> +#define MNTPOINT               "mnt_point"
> +#define TST_FILE               MNTPOINT"/test"
> +#define TST_FILE_SIZE_MB       32
> +#define SIZE_MB                        (1024*1024)
> +#define MODE                   0644
> +
> +static char dev_stat_path[1024];
> +static char *buffer;
> +static int fd;
> +
> +static void verify_syncfs(void)
> +{
> +       char nwrite_sec_val[BUFSIZ];
> +       int counter;
> +       unsigned long prev_write_sec = 0, write_sec = 0;
> +
> +       SAFE_FILE_SCANF(dev_stat_path, "%*s %*s %*s %*s %*s %*s %s",
> +                       nwrite_sec_val);
> +
> +       prev_write_sec = SAFE_STRTOUL(nwrite_sec_val, 0, ULONG_MAX);

Why not let scanf read %ul?

> +
> +       fd =  SAFE_OPEN(TST_FILE, O_RDWR|O_CREAT, MODE);
> +
> +       /* Filling the test file */
> +       for (counter = 0; counter < TST_FILE_SIZE_MB; counter++)
> +               SAFE_WRITE(1, fd, buffer, SIZE_MB);
> +
> +       TEST(syncfs(fd));
> +       if (TST_RET != 0)
> +               tst_brk(TFAIL | TTERRNO, "syncfs(fd) failed");
> +
> +       SAFE_FILE_SCANF(dev_stat_path, "%*s %*s %*s %*s %*s %*s %s",
> +                       nwrite_sec_val);
> +
> +       write_sec = SAFE_STRTOUL(nwrite_sec_val, 0, ULONG_MAX);
> +
> +       if ((write_sec - prev_write_sec) * 512 >=
> +           (TST_FILE_SIZE_MB * SIZE_MB))
> +               tst_res(TPASS, "Test filesystem synced to device");
> +       else
> +               tst_res(TFAIL, "Failed to sync test filesystem to device");
> +
> +       SAFE_CLOSE(fd);
> +}
> +

It's good to have a tests that verified syncfs() actually does what it
is meant to do.
It's awkward that none of the tests for fsync() fdatasync() sync()
sync_file_range()
check that.

It would be very low hanging to have the exact same test that you wrote
iterate on several test cases where the only difference is the op called on
fd. (fsync,fdatasync,syncfs) should all have the same consequence wrt
minimal written sectors.
With a little more effort, sync() and sync_file_range() could also be
added to test cases.

I realize that LTP usually puts syscalls tests under the specific
kernel/syscalls directory, but in this case, I believe code reuse calls
for a single test that exercises all three syscalls.

Thanks,
Amir.
Sumit Garg Feb. 15, 2019, 11:58 a.m. | #3
On Fri, 15 Feb 2019 at 13:25, Xiao Yang <yangx.jy@cn.fujitsu.com> wrote:
>
> Hi Sumit,
>
> Sorry for the late reply. :-)

No worries. :)

>
> 1) Compilation failed on older kernel(e.g. v2.6.32) because of the
> undefined syncfs().
> According to manpage, syncfs() was first appeared in Linux 2.6.39 and
> library
> support was added to glibc in version 2.14. Perhaps, we need to check if
> syncfs()
> is defined.
>

Would configuring .min_kver suffice to avoid this compilation issue?

> 2) Running this test got TBROK on older kernel(e.g. v3.10.0), as below:
> -------------------------------------------------------------------
> syncfs01.c:63: FAIL: Failed to sync test filesystem to device
> -------------------------------------------------------------------
> It seems that the content of /sys/block/<loopX>/stat is always zero and
> doesn't update even though write and sync have been done. I am looking
> into it, but i am not sure if this is a known bug on older kernel.
>

Thanks for looking into this. I will investigate this also.

-Sumit

> Best Regards,
> Xiao Yang
> On 2019/02/15 15:17, Sumit Garg wrote:
> > syncfs01 tests to sync filesystem having large dirty file pages to block
> > device. Also, it tests all supported filesystems on a test block device.
> >
> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > ---
> >
> > Changes in v2:
> > 1. Remove unused header file include.
> > 2. Remove redundant tst_device check.
> > 3. Remove redundant flags from tst_test struct.
> >
> > Fixes: https://github.com/linux-test-project/ltp/issues/294
> >
> >  runtest/syscalls                            |   2 +
> >  testcases/kernel/syscalls/syncfs/.gitignore |   1 +
> >  testcases/kernel/syscalls/syncfs/Makefile   |   8 +++
> >  testcases/kernel/syscalls/syncfs/syncfs01.c | 101 ++++++++++++++++++++++++++++
> >  4 files changed, 112 insertions(+)
> >  create mode 100644 testcases/kernel/syscalls/syncfs/.gitignore
> >  create mode 100644 testcases/kernel/syscalls/syncfs/Makefile
> >  create mode 100644 testcases/kernel/syscalls/syncfs/syncfs01.c
> >
> > diff --git a/runtest/syscalls b/runtest/syscalls
> > index 668c87c..9442740 100644
> > --- a/runtest/syscalls
> > +++ b/runtest/syscalls
> > @@ -1346,6 +1346,8 @@ symlinkat01 symlinkat01
> >  sync01 sync01
> >  sync02 sync02
> >
> > +syncfs01 syncfs01
> > +
> >  #testcases for sync_file_range
> >  sync_file_range01 sync_file_range01
> >
> > diff --git a/testcases/kernel/syscalls/syncfs/.gitignore b/testcases/kernel/syscalls/syncfs/.gitignore
> > new file mode 100644
> > index 0000000..6066295
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/syncfs/.gitignore
> > @@ -0,0 +1 @@
> > +syncfs01
> > diff --git a/testcases/kernel/syscalls/syncfs/Makefile b/testcases/kernel/syscalls/syncfs/Makefile
> > new file mode 100644
> > index 0000000..3e6c2f4
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/syncfs/Makefile
> > @@ -0,0 +1,8 @@
> > +# Copyright (c) 2019 - Linaro Limited. All rights reserved.
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +top_srcdir             ?= ../../../..
> > +
> > +include $(top_srcdir)/include/mk/testcases.mk
> > +
> > +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> > diff --git a/testcases/kernel/syscalls/syncfs/syncfs01.c b/testcases/kernel/syscalls/syncfs/syncfs01.c
> > new file mode 100644
> > index 0000000..e2d3e80
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/syncfs/syncfs01.c
> > @@ -0,0 +1,101 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2019 Linaro Limited. All rights reserved.
> > + * Author: Sumit Garg <sumit.garg@linaro.org>
> > + */
> > +
> > +/*
> > + * Test syncfs
> > + *
> > + * It basically tests syncfs() to sync filesystem having large dirty file
> > + * pages to block device. Also, it tests all supported filesystems on a test
> > + * block device.
> > + */
> > +
> > +#define _GNU_SOURCE
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +#include <sys/types.h>
> > +#include "tst_test.h"
> > +#include "lapi/fs.h"
> > +#include "lapi/stat.h"
> > +
> > +#define MNTPOINT             "mnt_point"
> > +#define TST_FILE             MNTPOINT"/test"
> > +#define TST_FILE_SIZE_MB     32
> > +#define SIZE_MB                      (1024*1024)
> > +#define MODE                 0644
> > +
> > +static char dev_stat_path[1024];
> > +static char *buffer;
> > +static int fd;
> > +
> > +static void verify_syncfs(void)
> > +{
> > +     char nwrite_sec_val[BUFSIZ];
> > +     int counter;
> > +     unsigned long prev_write_sec = 0, write_sec = 0;
> > +
> > +     SAFE_FILE_SCANF(dev_stat_path, "%*s %*s %*s %*s %*s %*s %s",
> > +                     nwrite_sec_val);
> > +
> > +     prev_write_sec = SAFE_STRTOUL(nwrite_sec_val, 0, ULONG_MAX);
> > +
> > +     fd =  SAFE_OPEN(TST_FILE, O_RDWR|O_CREAT, MODE);
> > +
> > +     /* Filling the test file */
> > +     for (counter = 0; counter < TST_FILE_SIZE_MB; counter++)
> > +             SAFE_WRITE(1, fd, buffer, SIZE_MB);
> > +
> > +     TEST(syncfs(fd));
> > +     if (TST_RET != 0)
> > +             tst_brk(TFAIL | TTERRNO, "syncfs(fd) failed");
> > +
> > +     SAFE_FILE_SCANF(dev_stat_path, "%*s %*s %*s %*s %*s %*s %s",
> > +                     nwrite_sec_val);
> > +
> > +     write_sec = SAFE_STRTOUL(nwrite_sec_val, 0, ULONG_MAX);
> > +
> > +     if ((write_sec - prev_write_sec) * 512 >=
> > +         (TST_FILE_SIZE_MB * SIZE_MB))
> > +             tst_res(TPASS, "Test filesystem synced to device");
> > +     else
> > +             tst_res(TFAIL, "Failed to sync test filesystem to device");
> > +
> > +     SAFE_CLOSE(fd);
> > +}
> > +
> > +static void setup(void)
> > +{
> > +     struct stat st;
> > +
> > +     snprintf(dev_stat_path, sizeof(dev_stat_path), "/sys/block/%s/stat",
> > +              strrchr(tst_device->dev, '/') + 1);
> > +
> > +     if (stat(dev_stat_path, &st) != 0)
> > +             tst_brk(TCONF, "Test device stat file: %s not found",
> > +                     dev_stat_path);
> > +
> > +     buffer = SAFE_MALLOC(SIZE_MB);
> > +
> > +     memset(buffer, 0, SIZE_MB);
> > +}
> > +
> > +static void cleanup(void)
> > +{
> > +     if (buffer)
> > +             free(buffer);
> > +
> > +     if (fd > 0)
> > +             SAFE_CLOSE(fd);
> > +}
> > +
> > +static struct tst_test test = {
> > +     .needs_root = 1,
> > +     .mount_device = 1,
> > +     .all_filesystems = 1,
> > +     .mntpoint = MNTPOINT,
> > +     .setup = setup,
> > +     .cleanup = cleanup,
> > +     .test_all = verify_syncfs,
> > +};
>
>
>
Cyril Hrubis Feb. 15, 2019, 12:16 p.m. | #4
Hi!
> > 1) Compilation failed on older kernel(e.g. v2.6.32) because of the
> > undefined syncfs().
> > According to manpage, syncfs() was first appeared in Linux 2.6.39 and
> > library
> > support was added to glibc in version 2.14. Perhaps, we need to check if
> > syncfs()
> > is defined.
> >
> 
> Would configuring .min_kver suffice to avoid this compilation issue?

Not at all, min_kver is runtime check, which could solve the latter
problem.

For this you can either:

* Add a configure check for syscfs()

* Add a fallback syscall definition to header include/lapi/

Fallback definition is preferable solution since with that the test will
still work on old userspace with new kernel.
Cyril Hrubis Feb. 15, 2019, 1:03 p.m. | #5
Hi!
> > +
> > +       fd =  SAFE_OPEN(TST_FILE, O_RDWR|O_CREAT, MODE);
> > +
> > +       /* Filling the test file */
> > +       for (counter = 0; counter < TST_FILE_SIZE_MB; counter++)
> > +               SAFE_WRITE(1, fd, buffer, SIZE_MB);
> > +
> > +       TEST(syncfs(fd));
> > +       if (TST_RET != 0)
> > +               tst_brk(TFAIL | TTERRNO, "syncfs(fd) failed");
> > +
> > +       SAFE_FILE_SCANF(dev_stat_path, "%*s %*s %*s %*s %*s %*s %s",
> > +                       nwrite_sec_val);
> > +
> > +       write_sec = SAFE_STRTOUL(nwrite_sec_val, 0, ULONG_MAX);
> > +
> > +       if ((write_sec - prev_write_sec) * 512 >=
> > +           (TST_FILE_SIZE_MB * SIZE_MB))
> > +               tst_res(TPASS, "Test filesystem synced to device");
> > +       else
> > +               tst_res(TFAIL, "Failed to sync test filesystem to device");
> > +
> > +       SAFE_CLOSE(fd);
> > +}
> > +
> 
> It's good to have a tests that verified syncfs() actually does what it
> is meant to do.
> It's awkward that none of the tests for fsync() fdatasync() sync()
> sync_file_range()
> check that.
> 
> It would be very low hanging to have the exact same test that you wrote
> iterate on several test cases where the only difference is the op called on
> fd. (fsync,fdatasync,syncfs) should all have the same consequence wrt
> minimal written sectors.
> With a little more effort, sync() and sync_file_range() could also be
> added to test cases.
> 
> I realize that LTP usually puts syscalls tests under the specific
> kernel/syscalls directory, but in this case, I believe code reuse calls
> for a single test that exercises all three syscalls.

We can always put the common code into a header/library and still have a
test for each of the syscalls as we usually do in LTP.
Sumit Garg Feb. 15, 2019, 1:18 p.m. | #6
On Fri, 15 Feb 2019 at 17:46, Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > > 1) Compilation failed on older kernel(e.g. v2.6.32) because of the
> > > undefined syncfs().
> > > According to manpage, syncfs() was first appeared in Linux 2.6.39 and
> > > library
> > > support was added to glibc in version 2.14. Perhaps, we need to check if
> > > syncfs()
> > > is defined.
> > >
> >
> > Would configuring .min_kver suffice to avoid this compilation issue?
>
> Not at all, min_kver is runtime check, which could solve the latter
> problem.
>
> For this you can either:
>
> * Add a configure check for syscfs()
>
> * Add a fallback syscall definition to header include/lapi/
>
> Fallback definition is preferable solution since with that the test will
> still work on old userspace with new kernel.
>

Thanks for the pointers. IIUC, you are referring to following change:

-       TEST(syncfs(fd));
+       TEST(tst_syscall(__NR_syncfs, fd));

If yes, then I will incorporate it.

-Sumit

> --
> Cyril Hrubis
> chrubis@suse.cz
Cyril Hrubis Feb. 15, 2019, 1:22 p.m. | #7
Hi!
> Thanks for the pointers. IIUC, you are referring to following change:
> 
> -       TEST(syncfs(fd));
> +       TEST(tst_syscall(__NR_syncfs, fd));
> 
> If yes, then I will incorporate it.

The most complete solution is configure check + fallback definition.

Have a look at:

include/lapi/execveat.h
m4/ltp-execveat.m4
configure.ac
Cyril Hrubis Feb. 15, 2019, 1:25 p.m. | #8
Hi!
> > Thanks for the pointers. IIUC, you are referring to following change:
> > 
> > -       TEST(syncfs(fd));
> > +       TEST(tst_syscall(__NR_syncfs, fd));
> > 
> > If yes, then I will incorporate it.
> 
> The most complete solution is configure check + fallback definition.
> 
> Have a look at:
> 
> include/lapi/execveat.h
> m4/ltp-execveat.m4
> configure.ac

And then you have to also check for the return value to exit the test
with TCONF on missing kernel support, you will most likely get EINVAL in
that case as the syscall number is not known.
Sumit Garg Feb. 15, 2019, 1:38 p.m. | #9
On Fri, 15 Feb 2019 at 14:12, Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Fri, Feb 15, 2019 at 9:17 AM Sumit Garg <sumit.garg@linaro.org> wrote:
> >
> > syncfs01 tests to sync filesystem having large dirty file pages to block
> > device. Also, it tests all supported filesystems on a test block device.
> >
> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > ---
> >
> > Changes in v2:
> > 1. Remove unused header file include.
> > 2. Remove redundant tst_device check.
> > 3. Remove redundant flags from tst_test struct.
> >
> > Fixes: https://github.com/linux-test-project/ltp/issues/294
> >
> >  runtest/syscalls                            |   2 +
> >  testcases/kernel/syscalls/syncfs/.gitignore |   1 +
> >  testcases/kernel/syscalls/syncfs/Makefile   |   8 +++
> >  testcases/kernel/syscalls/syncfs/syncfs01.c | 101 ++++++++++++++++++++++++++++
> >  4 files changed, 112 insertions(+)
> >  create mode 100644 testcases/kernel/syscalls/syncfs/.gitignore
> >  create mode 100644 testcases/kernel/syscalls/syncfs/Makefile
> >  create mode 100644 testcases/kernel/syscalls/syncfs/syncfs01.c
> >
> > diff --git a/runtest/syscalls b/runtest/syscalls
> > index 668c87c..9442740 100644
> > --- a/runtest/syscalls
> > +++ b/runtest/syscalls
> > @@ -1346,6 +1346,8 @@ symlinkat01 symlinkat01
> >  sync01 sync01
> >  sync02 sync02
> >
> > +syncfs01 syncfs01
> > +
> >  #testcases for sync_file_range
> >  sync_file_range01 sync_file_range01
> >
> > diff --git a/testcases/kernel/syscalls/syncfs/.gitignore b/testcases/kernel/syscalls/syncfs/.gitignore
> > new file mode 100644
> > index 0000000..6066295
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/syncfs/.gitignore
> > @@ -0,0 +1 @@
> > +syncfs01
> > diff --git a/testcases/kernel/syscalls/syncfs/Makefile b/testcases/kernel/syscalls/syncfs/Makefile
> > new file mode 100644
> > index 0000000..3e6c2f4
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/syncfs/Makefile
> > @@ -0,0 +1,8 @@
> > +# Copyright (c) 2019 - Linaro Limited. All rights reserved.
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +top_srcdir             ?= ../../../..
> > +
> > +include $(top_srcdir)/include/mk/testcases.mk
> > +
> > +include $(top_srcdir)/include/mk/generic_leaf_target.mk
> > diff --git a/testcases/kernel/syscalls/syncfs/syncfs01.c b/testcases/kernel/syscalls/syncfs/syncfs01.c
> > new file mode 100644
> > index 0000000..e2d3e80
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/syncfs/syncfs01.c
> > @@ -0,0 +1,101 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2019 Linaro Limited. All rights reserved.
> > + * Author: Sumit Garg <sumit.garg@linaro.org>
> > + */
> > +
> > +/*
> > + * Test syncfs
> > + *
> > + * It basically tests syncfs() to sync filesystem having large dirty file
> > + * pages to block device. Also, it tests all supported filesystems on a test
> > + * block device.
> > + */
> > +
> > +#define _GNU_SOURCE
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +#include <sys/types.h>
> > +#include "tst_test.h"
> > +#include "lapi/fs.h"
> > +#include "lapi/stat.h"
> > +
> > +#define MNTPOINT               "mnt_point"
> > +#define TST_FILE               MNTPOINT"/test"
> > +#define TST_FILE_SIZE_MB       32
> > +#define SIZE_MB                        (1024*1024)
> > +#define MODE                   0644
> > +
> > +static char dev_stat_path[1024];
> > +static char *buffer;
> > +static int fd;
> > +
> > +static void verify_syncfs(void)
> > +{
> > +       char nwrite_sec_val[BUFSIZ];
> > +       int counter;
> > +       unsigned long prev_write_sec = 0, write_sec = 0;
> > +
> > +       SAFE_FILE_SCANF(dev_stat_path, "%*s %*s %*s %*s %*s %*s %s",
> > +                       nwrite_sec_val);
> > +
> > +       prev_write_sec = SAFE_STRTOUL(nwrite_sec_val, 0, ULONG_MAX);
>
> Why not let scanf read %ul?
>

Hmm, will use %lu here.

> > +
> > +       fd =  SAFE_OPEN(TST_FILE, O_RDWR|O_CREAT, MODE);
> > +
> > +       /* Filling the test file */
> > +       for (counter = 0; counter < TST_FILE_SIZE_MB; counter++)
> > +               SAFE_WRITE(1, fd, buffer, SIZE_MB);
> > +
> > +       TEST(syncfs(fd));
> > +       if (TST_RET != 0)
> > +               tst_brk(TFAIL | TTERRNO, "syncfs(fd) failed");
> > +
> > +       SAFE_FILE_SCANF(dev_stat_path, "%*s %*s %*s %*s %*s %*s %s",
> > +                       nwrite_sec_val);
> > +
> > +       write_sec = SAFE_STRTOUL(nwrite_sec_val, 0, ULONG_MAX);
> > +
> > +       if ((write_sec - prev_write_sec) * 512 >=
> > +           (TST_FILE_SIZE_MB * SIZE_MB))
> > +               tst_res(TPASS, "Test filesystem synced to device");
> > +       else
> > +               tst_res(TFAIL, "Failed to sync test filesystem to device");
> > +
> > +       SAFE_CLOSE(fd);
> > +}
> > +
>
> It's good to have a tests that verified syncfs() actually does what it
> is meant to do.
> It's awkward that none of the tests for fsync() fdatasync() sync()
> sync_file_range()
> check that.
>

Agree.

> It would be very low hanging to have the exact same test that you wrote
> iterate on several test cases where the only difference is the op called on
> fd. (fsync,fdatasync,syncfs) should all have the same consequence wrt
> minimal written sectors.
> With a little more effort, sync() and sync_file_range() could also be
> added to test cases.
>

Sounds good to me.

> I realize that LTP usually puts syscalls tests under the specific
> kernel/syscalls directory, but in this case, I believe code reuse calls
> for a single test that exercises all three syscalls.
>

As Cyril said, will explore addition of common code in a
header/library and add corresponding test-case for fsync(),
fdatasync(), syncfs(), sync() and sync_file_range().

-Sumit

> Thanks,
> Amir.
Sumit Garg Feb. 15, 2019, 1:51 p.m. | #10
On Fri, 15 Feb 2019 at 18:55, Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > > Thanks for the pointers. IIUC, you are referring to following change:
> > >
> > > -       TEST(syncfs(fd));
> > > +       TEST(tst_syscall(__NR_syncfs, fd));
> > >
> > > If yes, then I will incorporate it.
> >
> > The most complete solution is configure check + fallback definition.
> >
> > Have a look at:
> >
> > include/lapi/execveat.h
> > m4/ltp-execveat.m4
> > configure.ac
>
> And then you have to also check for the return value to exit the test
> with TCONF on missing kernel support, you will most likely get EINVAL in
> that case as the syscall number is not known.
>

Ok got it. Will incorporate it.

-Sumit

> --
> Cyril Hrubis
> chrubis@suse.cz
Steve Muckle Feb. 16, 2019, 12:17 a.m. | #11
On 02/15/2019 05:22 AM, Cyril Hrubis wrote:
> Hi!
>> Thanks for the pointers. IIUC, you are referring to following change:
>>
>> -       TEST(syncfs(fd));
>> +       TEST(tst_syscall(__NR_syncfs, fd));
>>
>> If yes, then I will incorporate it.
> 
> The most complete solution is configure check + fallback definition.
> 
> Have a look at:
> 
> include/lapi/execveat.h
> m4/ltp-execveat.m4
> configure.ac

For tests in testcases/kernel/syscalls, should the tests typically 
directly call the syscall? I'd think this is preferable to the use of C 
library wrappers as these tests (by their location in the tree) seem to 
be focused on the syscall functionality in the kernel.

thanks,
steve
Sumit Garg Feb. 18, 2019, 7:52 a.m. | #12
On Sat, 16 Feb 2019 at 05:47, Steve Muckle <smuckle@google.com> wrote:
>
> On 02/15/2019 05:22 AM, Cyril Hrubis wrote:
> > Hi!
> >> Thanks for the pointers. IIUC, you are referring to following change:
> >>
> >> -       TEST(syncfs(fd));
> >> +       TEST(tst_syscall(__NR_syncfs, fd));
> >>
> >> If yes, then I will incorporate it.
> >
> > The most complete solution is configure check + fallback definition.
> >
> > Have a look at:
> >
> > include/lapi/execveat.h
> > m4/ltp-execveat.m4
> > configure.ac
>
> For tests in testcases/kernel/syscalls, should the tests typically
> directly call the syscall? I'd think this is preferable to the use of C
> library wrappers as these tests (by their location in the tree) seem to
> be focused on the syscall functionality in the kernel.
>

Sounds reasonable to me. But as per following manpage text for syscall():

"Each architecture ABI has its own requirements on how system call
arguments are passed to the kernel.  For system calls that have a
glibc  wrapper  (e.g., most  system  calls), glibc handles the details
of copying arguments to the right registers in a manner suitable for
the architecture. However, when using syscall() to make a system call,
the caller might need to handle architecture-dependent details; this
requirement is most commonly encountered  on certain 32-bit
architectures."

So I am not sure how we handle this architecture specific stuff if
required in test-cases.

-Sumit

> thanks,
> steve
Cyril Hrubis Feb. 18, 2019, 11:57 a.m. | #13
Hi!
> >> Thanks for the pointers. IIUC, you are referring to following change:
> >>
> >> -       TEST(syncfs(fd));
> >> +       TEST(tst_syscall(__NR_syncfs, fd));
> >>
> >> If yes, then I will incorporate it.
> > 
> > The most complete solution is configure check + fallback definition.
> > 
> > Have a look at:
> > 
> > include/lapi/execveat.h
> > m4/ltp-execveat.m4
> > configure.ac
> 
> For tests in testcases/kernel/syscalls, should the tests typically 
> directly call the syscall? I'd think this is preferable to the use of C 
> library wrappers as these tests (by their location in the tree) seem to 
> be focused on the syscall functionality in the kernel.

My take on this is that for a functional testing we really have to test
the kernel together with the library that wraps the syscalls because
otherwise bugs are bound to happen such as these:

https://sourceware.org/bugzilla/show_bug.cgi?id=23069
https://sourceware.org/bugzilla/show_bug.cgi?id=23579

And I always cared about syscalls being correct on the C library level.

I guess that for most of the syscalls that are just thin wrappers it
does not matter since these just prepare the parameters and jump to the
kernel, but in certain cases libc does quite a lot of work which is
sometimes complex code I do not want to replicate that unless really
needed. And with that I think that it's actually much easier to go with
the libc API whenever possible rather than reviewing the libc code each
time we write a testcase.

Does that sound reasonable to you?
Cyril Hrubis Feb. 18, 2019, 12:24 p.m. | #14
Hi!
> > >> Thanks for the pointers. IIUC, you are referring to following change:
> > >>
> > >> -       TEST(syncfs(fd));
> > >> +       TEST(tst_syscall(__NR_syncfs, fd));
> > >>
> > >> If yes, then I will incorporate it.
> > >
> > > The most complete solution is configure check + fallback definition.
> > >
> > > Have a look at:
> > >
> > > include/lapi/execveat.h
> > > m4/ltp-execveat.m4
> > > configure.ac
> >
> > For tests in testcases/kernel/syscalls, should the tests typically
> > directly call the syscall? I'd think this is preferable to the use of C
> > library wrappers as these tests (by their location in the tree) seem to
> > be focused on the syscall functionality in the kernel.
> >
> 
> Sounds reasonable to me. But as per following manpage text for syscall():
> 
> "Each architecture ABI has its own requirements on how system call
> arguments are passed to the kernel.  For system calls that have a
> glibc  wrapper  (e.g., most  system  calls), glibc handles the details
> of copying arguments to the right registers in a manner suitable for
> the architecture. However, when using syscall() to make a system call,
> the caller might need to handle architecture-dependent details; this
> requirement is most commonly encountered  on certain 32-bit
> architectures."
> 
> So I am not sure how we handle this architecture specific stuff if
> required in test-cases.

In more than 99% of the cases this is not an issue, as far as the
arguments are passed as long int and we do not have to pass more than
six, which is the common case, we don't have to do anything.

In some cases we have to split 64bit values in half and pass them as two
arguments, then we put the code into the lapi hader such as
include/lapi/fallocate.h.

Then there are cases where we pass a pointer to architecture specific
structure such as the sigaction which sometimes contains a pointer to
function that restores the program state after we return from the signal
handler and this is where calling the kernel syscall directly gets
complex. We do carry that code under include/lapi/rt_sigaction.h.
Sumit Garg Feb. 19, 2019, 6:44 a.m. | #15
Hi Xiao,

On Fri, 15 Feb 2019 at 13:25, Xiao Yang <yangx.jy@cn.fujitsu.com> wrote:
>
> Hi Sumit,
>
> Sorry for the late reply. :-)
>
> 1) Compilation failed on older kernel(e.g. v2.6.32) because of the
> undefined syncfs().
> According to manpage, syncfs() was first appeared in Linux 2.6.39 and
> library
> support was added to glibc in version 2.14. Perhaps, we need to check if
> syncfs()
> is defined.
>
> 2) Running this test got TBROK on older kernel(e.g. v3.10.0), as below:
> -------------------------------------------------------------------
> syncfs01.c:63: FAIL: Failed to sync test filesystem to device
> -------------------------------------------------------------------
> It seems that the content of /sys/block/<loopX>/stat is always zero and
> doesn't update even though write and sync have been done. I am looking
> into it, but i am not sure if this is a known bug on older kernel.
>

Did you get a chance to root cause this issue?

I am not able to reproduce setup running kernel v3.10. If possible,
can you please share your setup details?

As Cyril suggested, we could solve this issue via configuring min_kver
value. But I am not sure regarding appropriate value.

BTW, as this seems to be kernel issue (/sys/block/<loopX>/stat not
updated), do we really don't want to run this test-case on old kernel
where it fails? To me this failure case is value add for user of old
kernel to be aware about this issue and probably fix it.

-Sumit

> Best Regards,
> Xiao Yang
Steve Muckle Feb. 19, 2019, 11:15 p.m. | #16
On 02/18/2019 03:57 AM, Cyril Hrubis wrote:
>> For tests in testcases/kernel/syscalls, should the tests typically
>> directly call the syscall? I'd think this is preferable to the use of C
>> library wrappers as these tests (by their location in the tree) seem to
>> be focused on the syscall functionality in the kernel.
> 
> My take on this is that for a functional testing we really have to test
> the kernel together with the library that wraps the syscalls because
> otherwise bugs are bound to happen such as these:
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=23069
> https://sourceware.org/bugzilla/show_bug.cgi?id=23579
> 
> And I always cared about syscalls being correct on the C library level.
> 
> I guess that for most of the syscalls that are just thin wrappers it
> does not matter since these just prepare the parameters and jump to the
> kernel, but in certain cases libc does quite a lot of work which is
> sometimes complex code I do not want to replicate that unless really
> needed. And with that I think that it's actually much easier to go with
> the libc API whenever possible rather than reviewing the libc code each
> time we write a testcase.
> 
> Does that sound reasonable to you?

Sure. My concern is being able to test syscalls in Android where the C 
library may not have some wrappers. So far these have all been cases 
where one can just replace the library call with the direct syscall, as 
a number of recent patches have done. If I run into a case where more 
substantial library support is needed maybe we'll just have to focus on 
getting that into bionic or look at other options.

thanks,
steve
Cyril Hrubis Feb. 20, 2019, 1:14 p.m. | #17
Hi!
> Sure. My concern is being able to test syscalls in Android where the C 
> library may not have some wrappers. So far these have all been cases 
> where one can just replace the library call with the direct syscall, as 
> a number of recent patches have done. If I run into a case where more 
> substantial library support is needed maybe we'll just have to focus on 
> getting that into bionic or look at other options.

I guess that we can also auto-generate fallback syscall wrappers for
these cases so that we don't have to bother dealing with this in the
actual testcases. Should be as easy as listing the function prototypes
in a file and a few lines of shell.
Cyril Hrubis Feb. 20, 2019, 3:12 p.m. | #18
Hi!
> > Sure. My concern is being able to test syscalls in Android where the C 
> > library may not have some wrappers. So far these have all been cases 
> > where one can just replace the library call with the direct syscall, as 
> > a number of recent patches have done. If I run into a case where more 
> > substantial library support is needed maybe we'll just have to focus on 
> > getting that into bionic or look at other options.
> 
> I guess that we can also auto-generate fallback syscall wrappers for
> these cases so that we don't have to bother dealing with this in the
> actual testcases. Should be as easy as listing the function prototypes
> in a file and a few lines of shell.

Thinking of this, generating fallbacks is the easy part, figuring out
when to use them is the complex one. I wanted to use a trick with weak
function symbols so that the linker would pick up these fallbacks only
if there was no system defintion but that unfortunately does not work
because glibc uses weak symbols for symbol versioning. I guess that we
can use autoconf to generate fallback functions automatically if syscall
wrapper is found to be missing, but that would be a bit ugly code. Also
do you even use the configure script on android builds?
Sumit Garg Feb. 21, 2019, 7:08 a.m. | #19
On Tue, 19 Feb 2019 at 12:14, Sumit Garg <sumit.garg@linaro.org> wrote:
>
> Hi Xiao,
>
> On Fri, 15 Feb 2019 at 13:25, Xiao Yang <yangx.jy@cn.fujitsu.com> wrote:
> >
> > Hi Sumit,
> >
> > Sorry for the late reply. :-)
> >
> > 1) Compilation failed on older kernel(e.g. v2.6.32) because of the
> > undefined syncfs().
> > According to manpage, syncfs() was first appeared in Linux 2.6.39 and
> > library
> > support was added to glibc in version 2.14. Perhaps, we need to check if
> > syncfs()
> > is defined.
> >
> > 2) Running this test got TBROK on older kernel(e.g. v3.10.0), as below:
> > -------------------------------------------------------------------
> > syncfs01.c:63: FAIL: Failed to sync test filesystem to device
> > -------------------------------------------------------------------
> > It seems that the content of /sys/block/<loopX>/stat is always zero and
> > doesn't update even though write and sync have been done. I am looking
> > into it, but i am not sure if this is a known bug on older kernel.
> >
>
> Did you get a chance to root cause this issue?
>
> I am not able to reproduce setup running kernel v3.10. If possible,
> can you please share your setup details?
>
> As Cyril suggested, we could solve this issue via configuring min_kver
> value. But I am not sure regarding appropriate value.
>
> BTW, as this seems to be kernel issue (/sys/block/<loopX>/stat not
> updated), do we really don't want to run this test-case on old kernel
> where it fails? To me this failure case is value add for user of old
> kernel to be aware about this issue and probably fix it.
>

Thinking more about broken /sys/block/<loopX>/stat [1]. It seems that
following param should be a good indicator of block device stat file
being broken:

io_ticks        milliseconds  total time this block device has been active

This param should be non-zero inside test-case as there are already
some file-system operations performed on the device like formatting
etc.

If you could confirm that you see this param as zero on older kernel
(v3.10), then I will add a TCONF check for this param being non-zero
as follows:

-       SAFE_FILE_SCANF(NULL, dev_stat_path, "%*s %*s %*s %*s %*s %*s %lu",
-                       &dev_sec_write);
+       SAFE_FILE_SCANF(NULL, dev_stat_path,
+                       "%*s %*s %*s %*s %*s %*s %lu %*s %*s %lu",
+                       &dev_sec_write, &io_ticks);
+
+       if (!io_ticks)
+               tst_brkm(TCONF, NULL, "Test device stat file: %s broken",
+                        dev_stat_path);

[1] https://www.kernel.org/doc/Documentation/block/stat.txt

-Sumit

>
> > Best Regards,
> > Xiao Yang
Steve Muckle Feb. 22, 2019, 7:58 p.m. | #20
On 02/20/2019 07:12 AM, Cyril Hrubis wrote:
> Hi!
>>> Sure. My concern is being able to test syscalls in Android where the C
>>> library may not have some wrappers. So far these have all been cases
>>> where one can just replace the library call with the direct syscall, as
>>> a number of recent patches have done. If I run into a case where more
>>> substantial library support is needed maybe we'll just have to focus on
>>> getting that into bionic or look at other options.
>>
>> I guess that we can also auto-generate fallback syscall wrappers for
>> these cases so that we don't have to bother dealing with this in the
>> actual testcases. Should be as easy as listing the function prototypes
>> in a file and a few lines of shell.
> 
> Thinking of this, generating fallbacks is the easy part, figuring out
> when to use them is the complex one. I wanted to use a trick with weak
> function symbols so that the linker would pick up these fallbacks only
> if there was no system defintion but that unfortunately does not work
> because glibc uses weak symbols for symbol versioning. I guess that we
> can use autoconf to generate fallback functions automatically if syscall
> wrapper is found to be missing, but that would be a bit ugly code. Also
> do you even use the configure script on android builds?

We do use the configure script. But I guess given the discussion in the 
other thread, hopefully both the libc wrapper and direct syscall can be 
tested.

Patch

diff --git a/runtest/syscalls b/runtest/syscalls
index 668c87c..9442740 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1346,6 +1346,8 @@  symlinkat01 symlinkat01
 sync01 sync01
 sync02 sync02
 
+syncfs01 syncfs01
+
 #testcases for sync_file_range
 sync_file_range01 sync_file_range01
 
diff --git a/testcases/kernel/syscalls/syncfs/.gitignore b/testcases/kernel/syscalls/syncfs/.gitignore
new file mode 100644
index 0000000..6066295
--- /dev/null
+++ b/testcases/kernel/syscalls/syncfs/.gitignore
@@ -0,0 +1 @@ 
+syncfs01
diff --git a/testcases/kernel/syscalls/syncfs/Makefile b/testcases/kernel/syscalls/syncfs/Makefile
new file mode 100644
index 0000000..3e6c2f4
--- /dev/null
+++ b/testcases/kernel/syscalls/syncfs/Makefile
@@ -0,0 +1,8 @@ 
+# Copyright (c) 2019 - Linaro Limited. All rights reserved.
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+top_srcdir             ?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/syncfs/syncfs01.c b/testcases/kernel/syscalls/syncfs/syncfs01.c
new file mode 100644
index 0000000..e2d3e80
--- /dev/null
+++ b/testcases/kernel/syscalls/syncfs/syncfs01.c
@@ -0,0 +1,101 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Linaro Limited. All rights reserved.
+ * Author: Sumit Garg <sumit.garg@linaro.org>
+ */
+
+/*
+ * Test syncfs
+ *
+ * It basically tests syncfs() to sync filesystem having large dirty file
+ * pages to block device. Also, it tests all supported filesystems on a test
+ * block device.
+ */
+
+#define _GNU_SOURCE
+#include <stdlib.h>
+#include <stdio.h>
+#include <sys/types.h>
+#include "tst_test.h"
+#include "lapi/fs.h"
+#include "lapi/stat.h"
+
+#define MNTPOINT		"mnt_point"
+#define TST_FILE		MNTPOINT"/test"
+#define TST_FILE_SIZE_MB	32
+#define SIZE_MB			(1024*1024)
+#define MODE			0644
+
+static char dev_stat_path[1024];
+static char *buffer;
+static int fd;
+
+static void verify_syncfs(void)
+{
+	char nwrite_sec_val[BUFSIZ];
+	int counter;
+	unsigned long prev_write_sec = 0, write_sec = 0;
+
+	SAFE_FILE_SCANF(dev_stat_path, "%*s %*s %*s %*s %*s %*s %s",
+			nwrite_sec_val);
+
+	prev_write_sec = SAFE_STRTOUL(nwrite_sec_val, 0, ULONG_MAX);
+
+	fd =  SAFE_OPEN(TST_FILE, O_RDWR|O_CREAT, MODE);
+
+	/* Filling the test file */
+	for (counter = 0; counter < TST_FILE_SIZE_MB; counter++)
+		SAFE_WRITE(1, fd, buffer, SIZE_MB);
+
+	TEST(syncfs(fd));
+	if (TST_RET != 0)
+		tst_brk(TFAIL | TTERRNO, "syncfs(fd) failed");
+
+	SAFE_FILE_SCANF(dev_stat_path, "%*s %*s %*s %*s %*s %*s %s",
+			nwrite_sec_val);
+
+	write_sec = SAFE_STRTOUL(nwrite_sec_val, 0, ULONG_MAX);
+
+	if ((write_sec - prev_write_sec) * 512 >=
+	    (TST_FILE_SIZE_MB * SIZE_MB))
+		tst_res(TPASS, "Test filesystem synced to device");
+	else
+		tst_res(TFAIL, "Failed to sync test filesystem to device");
+
+	SAFE_CLOSE(fd);
+}
+
+static void setup(void)
+{
+	struct stat st;
+
+	snprintf(dev_stat_path, sizeof(dev_stat_path), "/sys/block/%s/stat",
+		 strrchr(tst_device->dev, '/') + 1);
+
+	if (stat(dev_stat_path, &st) != 0)
+		tst_brk(TCONF, "Test device stat file: %s not found",
+			dev_stat_path);
+
+	buffer = SAFE_MALLOC(SIZE_MB);
+
+	memset(buffer, 0, SIZE_MB);
+}
+
+static void cleanup(void)
+{
+	if (buffer)
+		free(buffer);
+
+	if (fd > 0)
+		SAFE_CLOSE(fd);
+}
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.mount_device = 1,
+	.all_filesystems = 1,
+	.mntpoint = MNTPOINT,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = verify_syncfs,
+};