[LTP] [PATCH v3 0/7] syscalls: add sync device test-cases

[LTP] [PATCH v3 0/7] syscalls: add sync device test-cases

[Sumit Garg]

This patch-set adds common test for minimal sectors written on device
for various sync related syscalls:
1. syncfs()
2. sync()
3. fsync()
4. fdatasync()
5. sync_file_range()

Also this test is run on all supported filesystems on a test block
device.

Changes in v3:
1. Scope increased from single patch to this patch-set to add common
   test for sync(), fsync(), fdatasync() and sync_file_range() syscalls.
2. Move common functionality to library functions.
3. Use %lu rather than %s while parsing sectors written.
4. Add config check for C library wrapper and fallback definition for
   syncfs() and sync_file_range() syscalls.

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

Sumit Garg (7):
  lib: Add library functions for sync related syscalls
  syscalls: add syncfs() sync device test-case
  syscalls/sync: add sync device test-case
  syscalls/fsync: add sync device test-case
  syscalls/fdatasync: add sync device test-case
  syscalls/sync_file_range: Use C library wrapper if present
  syscalls/sync_file_range: add sync device test-case

 configure.ac                                       |  2 +
 include/lapi/sync_file_range.h                     | 64 +++++++++++++++++
 include/lapi/syncfs.h                              | 21 ++++++
 include/tst_sync_device.h                          | 17 +++++
 include/tst_test.h                                 |  7 ++
 lib/tst_sync_device.c                              | 80 ++++++++++++++++++++++
 m4/ltp-sync_file_range.m4                          | 10 +++
 m4/ltp-syncfs.m4                                   | 10 +++
 runtest/syscalls                                   |  6 ++
 testcases/kernel/syscalls/fdatasync/.gitignore     |  1 +
 testcases/kernel/syscalls/fdatasync/fdatasync03.c  | 61 +++++++++++++++++
 testcases/kernel/syscalls/fsync/.gitignore         |  1 +
 testcases/kernel/syscalls/fsync/fsync04.c          | 61 +++++++++++++++++
 testcases/kernel/syscalls/sync/.gitignore          |  1 +
 testcases/kernel/syscalls/sync/sync03.c            | 59 ++++++++++++++++
 .../kernel/syscalls/sync_file_range/.gitignore     |  1 +
 .../sync_file_range/check_sync_file_range.h        | 23 +++++++
 .../syscalls/sync_file_range/sync_file_range01.c   | 62 ++---------------
 .../syscalls/sync_file_range/sync_file_range02.c   | 70 +++++++++++++++++++
 testcases/kernel/syscalls/syncfs/.gitignore        |  1 +
 testcases/kernel/syscalls/syncfs/Makefile          |  8 +++
 testcases/kernel/syscalls/syncfs/check_syncfs.h    | 19 +++++
 testcases/kernel/syscalls/syncfs/syncfs01.c        | 64 +++++++++++++++++
 23 files changed, 592 insertions(+), 57 deletions(-)
 create mode 100644 include/lapi/sync_file_range.h
 create mode 100644 include/lapi/syncfs.h
 create mode 100644 include/tst_sync_device.h
 create mode 100644 lib/tst_sync_device.c
 create mode 100644 m4/ltp-sync_file_range.m4
 create mode 100644 m4/ltp-syncfs.m4
 create mode 100644 testcases/kernel/syscalls/fdatasync/fdatasync03.c
 create mode 100644 testcases/kernel/syscalls/fsync/fsync04.c
 create mode 100644 testcases/kernel/syscalls/sync/sync03.c
 create mode 100644 testcases/kernel/syscalls/sync_file_range/check_sync_file_range.h
 create mode 100644 testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
 create mode 100644 testcases/kernel/syscalls/syncfs/.gitignore
 create mode 100644 testcases/kernel/syscalls/syncfs/Makefile
 create mode 100644 testcases/kernel/syscalls/syncfs/check_syncfs.h
 create mode 100644 testcases/kernel/syscalls/syncfs/syncfs01.c

-- 
2.7.4

[LTP] [PATCH v3 1/7] lib: Add library functions for sync related syscalls

[Sumit Garg]

These library functions are used to add common test for minimal sectors
written on device for various sync related syscalls.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 include/tst_sync_device.h | 17 ++++++++++
 lib/tst_sync_device.c     | 80 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+)
 create mode 100644 include/tst_sync_device.h
 create mode 100644 lib/tst_sync_device.c

diff --git a/include/tst_sync_device.h b/include/tst_sync_device.h
new file mode 100644
index 0000000..b07c490
--- /dev/null
+++ b/include/tst_sync_device.h
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Linaro Limited. All rights reserved.
+ * Author: Sumit Garg <sumit.garg@linaro.org>
+ */
+
+#ifndef TST_SYNC_DEVICE_H__
+#define TST_SYNC_DEVICE_H__
+
+#include <stdbool.h>
+
+void tst_sync_device_init(const char *dev);
+int tst_sync_device_write(const char *file, unsigned int size_mb);
+bool tst_sync_device_check(unsigned int size_mb);
+void tst_sync_device_cleanup(void);
+
+#endif
diff --git a/lib/tst_sync_device.c b/lib/tst_sync_device.c
new file mode 100644
index 0000000..5a0a17c
--- /dev/null
+++ b/lib/tst_sync_device.c
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Linaro Limited. All rights reserved.
+ * Author: Sumit Garg <sumit.garg@linaro.org>
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+#include "lapi/stat.h"
+#include "tst_sync_device.h"
+
+#define SIZE_MB			(1024*1024)
+#define MODE			0644
+
+static char dev_stat_path[1024];
+static char *buffer;
+static int fd;
+static unsigned long prev_write_sec;
+
+void tst_sync_device_init(const char *dev)
+{
+	struct stat st;
+
+	snprintf(dev_stat_path, sizeof(dev_stat_path), "/sys/block/%s/stat",
+		 strrchr(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);
+}
+
+int tst_sync_device_write(const char *file, unsigned int size_mb)
+{
+	unsigned int counter;
+
+	SAFE_FILE_SCANF(dev_stat_path, "%*s %*s %*s %*s %*s %*s %lu",
+			&prev_write_sec);
+
+	fd =  SAFE_OPEN(file, O_RDWR|O_CREAT, MODE);
+
+	/* Filling the test file */
+	for (counter = 0; counter < size_mb; counter++)
+		SAFE_WRITE(1, fd, buffer, SIZE_MB);
+
+	return fd;
+}
+
+bool tst_sync_device_check(unsigned int size_mb)
+{
+	unsigned long write_sec = 0;
+	bool res = false;
+
+	SAFE_FILE_SCANF(dev_stat_path, "%*s %*s %*s %*s %*s %*s %lu",
+			&write_sec);
+
+	if ((write_sec - prev_write_sec) * 512 >=
+	    (size_mb * SIZE_MB))
+		res = true;
+
+	SAFE_CLOSE(fd);
+
+	return res;
+}
+
+void tst_sync_device_cleanup(void)
+{
+	if (buffer)
+		free(buffer);
+
+	if (fd > 0)
+		SAFE_CLOSE(fd);
+}
-- 
2.7.4

[LTP] [PATCH v3 2/7] syscalls: add syncfs() sync device test-case

[Sumit Garg]

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>
---
 configure.ac                                    |  1 +
 include/lapi/syncfs.h                           | 21 ++++++++
 m4/ltp-syncfs.m4                                | 10 ++++
 runtest/syscalls                                |  2 +
 testcases/kernel/syscalls/syncfs/.gitignore     |  1 +
 testcases/kernel/syscalls/syncfs/Makefile       |  8 ++++
 testcases/kernel/syscalls/syncfs/check_syncfs.h | 19 ++++++++
 testcases/kernel/syscalls/syncfs/syncfs01.c     | 64 +++++++++++++++++++++++++
 8 files changed, 126 insertions(+)
 create mode 100644 include/lapi/syncfs.h
 create mode 100644 m4/ltp-syncfs.m4
 create mode 100644 testcases/kernel/syscalls/syncfs/.gitignore
 create mode 100644 testcases/kernel/syscalls/syncfs/Makefile
 create mode 100644 testcases/kernel/syscalls/syncfs/check_syncfs.h
 create mode 100644 testcases/kernel/syscalls/syncfs/syncfs01.c

diff --git a/configure.ac b/configure.ac
index caea344..9122b6d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -231,6 +231,7 @@ LTP_CHECK_TPACKET_V3
 LTP_CHECK_RLIMIT64
 LTP_DETECT_HOST_CPU
 LTP_CHECK_PERF_EVENT
+LTP_CHECK_SYNCFS
 
 if test "x$with_numa" = xyes; then
 	LTP_CHECK_SYSCALL_NUMA
diff --git a/include/lapi/syncfs.h b/include/lapi/syncfs.h
new file mode 100644
index 0000000..1341c6b
--- /dev/null
+++ b/include/lapi/syncfs.h
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Linaro Limited. All rights reserved.
+ * Author: Sumit Garg <sumit.garg@linaro.org>
+ */
+
+#ifndef SYNCFS_H
+#define SYNCFS_H
+
+#include <sys/types.h>
+#include "config.h"
+#include "lapi/syscalls.h"
+
+#if !defined(HAVE_SYNCFS)
+int syncfs(int fd)
+{
+	return tst_syscall(__NR_syncfs, fd);
+}
+#endif
+
+#endif /* SYNCFS_H */
diff --git a/m4/ltp-syncfs.m4 b/m4/ltp-syncfs.m4
new file mode 100644
index 0000000..836a055
--- /dev/null
+++ b/m4/ltp-syncfs.m4
@@ -0,0 +1,10 @@
+dnl SPDX-License-Identifier: GPL-2.0-or-later
+dnl Copyright (c) 2019 Linaro Limited. All rights reserved.
+
+dnl
+dnl LTP_CHECK_SYNCFS
+dnl ----------------------------
+dnl
+AC_DEFUN([LTP_CHECK_SYNCFS],[
+AC_CHECK_FUNCS(syncfs,,)
+])
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/check_syncfs.h b/testcases/kernel/syscalls/syncfs/check_syncfs.h
new file mode 100644
index 0000000..26991d2
--- /dev/null
+++ b/testcases/kernel/syscalls/syncfs/check_syncfs.h
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Linaro Limited. All rights reserved.
+ * Author: Sumit Garg <sumit.garg@linaro.org>
+ */
+
+#ifndef CHECK_SYNCFS_H
+#define CHECK_SYNCFS_H
+
+void check_syncfs(void)
+{
+	int ret;
+
+	ret = syncfs(-1);
+	if (ret == -1 && errno == EINVAL)
+		tst_brk(TCONF, "syncfs() not supported");
+}
+
+#endif /* CHECK_SYNCFS_H */
diff --git a/testcases/kernel/syscalls/syncfs/syncfs01.c b/testcases/kernel/syscalls/syncfs/syncfs01.c
new file mode 100644
index 0000000..7c3efb0
--- /dev/null
+++ b/testcases/kernel/syscalls/syncfs/syncfs01.c
@@ -0,0 +1,64 @@
+// 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 "lapi/syncfs.h"
+#include "tst_sync_device.h"
+#include "tst_test.h"
+#include "check_syncfs.h"
+
+#define MNTPOINT		"mnt_point"
+#define TST_FILE		MNTPOINT"/test"
+#define TST_FILE_SIZE_MB	32
+
+static void verify_syncfs(void)
+{
+	int fd;
+
+	fd = tst_sync_device_write(TST_FILE, TST_FILE_SIZE_MB);
+
+	TEST(syncfs(fd));
+	if (TST_RET != 0)
+		tst_brk(TFAIL | TTERRNO, "syncfs(fd) failed");
+
+	if (tst_sync_device_check(TST_FILE_SIZE_MB))
+		tst_res(TPASS, "Test filesystem synced to device");
+	else
+		tst_res(TFAIL, "Failed to sync test filesystem to device");
+}
+
+static void setup(void)
+{
+	check_syncfs();
+
+	tst_sync_device_init(tst_device->dev);
+}
+
+static void cleanup(void)
+{
+	tst_sync_device_cleanup();
+}
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.mount_device = 1,
+	.all_filesystems = 1,
+	.mntpoint = MNTPOINT,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = verify_syncfs,
+};
-- 
2.7.4

[LTP] [PATCH v3 3/7] syscalls/sync: add sync device test-case

[Sumit Garg]

sync03 tests to sync file having large dirty file pages to block device.
It tests all supported filesystems on a test block device.

Also define TEST_VOID macro in tst_test.h.

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 include/tst_test.h                        |  7 ++++
 runtest/syscalls                          |  1 +
 testcases/kernel/syscalls/sync/.gitignore |  1 +
 testcases/kernel/syscalls/sync/sync03.c   | 59 +++++++++++++++++++++++++++++++
 4 files changed, 68 insertions(+)
 create mode 100644 testcases/kernel/syscalls/sync/sync03.c

diff --git a/include/tst_test.h b/include/tst_test.h
index 12dda2e..03c88e3 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -216,6 +216,13 @@ void tst_reinit(void);
 		TST_ERR = errno; \
 	} while (0)
 
+#define TEST_VOID(SCALL) \
+	do { \
+		errno = 0; \
+		SCALL; \
+		TST_ERR = errno; \
+	} while (0)
+
 extern long TST_RET;
 extern int TST_ERR;
 
diff --git a/runtest/syscalls b/runtest/syscalls
index 9442740..dba0dee 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1345,6 +1345,7 @@ symlinkat01 symlinkat01
 
 sync01 sync01
 sync02 sync02
+sync03 sync03
 
 syncfs01 syncfs01
 
diff --git a/testcases/kernel/syscalls/sync/.gitignore b/testcases/kernel/syscalls/sync/.gitignore
index d8d304d..04f4710 100644
--- a/testcases/kernel/syscalls/sync/.gitignore
+++ b/testcases/kernel/syscalls/sync/.gitignore
@@ -1,2 +1,3 @@
 /sync01
 /sync02
+/sync03
diff --git a/testcases/kernel/syscalls/sync/sync03.c b/testcases/kernel/syscalls/sync/sync03.c
new file mode 100644
index 0000000..419f9b9
--- /dev/null
+++ b/testcases/kernel/syscalls/sync/sync03.c
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Linaro Limited. All rights reserved.
+ * Author: Sumit Garg <sumit.garg@linaro.org>
+ */
+
+/*
+ * sync03
+ *
+ * It basically tests sync() to sync test file having large dirty file pages
+ * to block device. Also, it tests all supported filesystems on a test block
+ * device.
+ */
+
+#define _GNU_SOURCE
+#include <errno.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <sys/types.h>
+#include "tst_sync_device.h"
+#include "tst_test.h"
+
+#define MNTPOINT		"mnt_point"
+#define TST_FILE		MNTPOINT"/test"
+#define TST_FILE_SIZE_MB	32
+
+static void verify_sync(void)
+{
+	tst_sync_device_write(TST_FILE, TST_FILE_SIZE_MB);
+
+	TEST_VOID(sync());
+	if (TST_RET != 0)
+		tst_brk(TFAIL | TTERRNO, "sync() failed");
+
+	if (tst_sync_device_check(TST_FILE_SIZE_MB))
+		tst_res(TPASS, "Test file synced to device");
+	else
+		tst_res(TFAIL, "Failed to sync test file to device");
+}
+
+static void setup(void)
+{
+	tst_sync_device_init(tst_device->dev);
+}
+
+static void cleanup(void)
+{
+	tst_sync_device_cleanup();
+}
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.mount_device = 1,
+	.all_filesystems = 1,
+	.mntpoint = MNTPOINT,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = verify_sync,
+};
-- 
2.7.4

[LTP] [PATCH v3 4/7] syscalls/fsync: add sync device test-case

[Sumit Garg]

fsync04 tests to sync file 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>
---
 runtest/syscalls                           |  1 +
 testcases/kernel/syscalls/fsync/.gitignore |  1 +
 testcases/kernel/syscalls/fsync/fsync04.c  | 61 ++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+)
 create mode 100644 testcases/kernel/syscalls/fsync/fsync04.c

diff --git a/runtest/syscalls b/runtest/syscalls
index dba0dee..0d09509 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -347,6 +347,7 @@ fstatfs02_64 fstatfs02_64
 fsync01 fsync01
 fsync02 fsync02
 fsync03 fsync03
+fsync04 fsync04
 
 ftruncate01 ftruncate01
 ftruncate01_64 ftruncate01_64
diff --git a/testcases/kernel/syscalls/fsync/.gitignore b/testcases/kernel/syscalls/fsync/.gitignore
index 3c694a4..4b5ea83 100644
--- a/testcases/kernel/syscalls/fsync/.gitignore
+++ b/testcases/kernel/syscalls/fsync/.gitignore
@@ -1,3 +1,4 @@
 /fsync01
 /fsync02
 /fsync03
+/fsync04
diff --git a/testcases/kernel/syscalls/fsync/fsync04.c b/testcases/kernel/syscalls/fsync/fsync04.c
new file mode 100644
index 0000000..72c73ce
--- /dev/null
+++ b/testcases/kernel/syscalls/fsync/fsync04.c
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Linaro Limited. All rights reserved.
+ * Author: Sumit Garg <sumit.garg@linaro.org>
+ */
+
+/*
+ * fsync04
+ *
+ * It basically tests fsync() to sync test file having large dirty file pages
+ * to block device. Also, it tests all supported filesystems on a test block
+ * device.
+ */
+
+#define _GNU_SOURCE
+#include <errno.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <sys/types.h>
+#include "tst_sync_device.h"
+#include "tst_test.h"
+
+#define MNTPOINT		"mnt_point"
+#define TST_FILE		MNTPOINT"/test"
+#define TST_FILE_SIZE_MB	32
+
+static void verify_fsync(void)
+{
+	int fd;
+
+	fd = tst_sync_device_write(TST_FILE, TST_FILE_SIZE_MB);
+
+	TEST(fsync(fd));
+	if (TST_RET != 0)
+		tst_brk(TFAIL | TTERRNO, "fsync(fd) failed");
+
+	if (tst_sync_device_check(TST_FILE_SIZE_MB))
+		tst_res(TPASS, "Test file synced to device");
+	else
+		tst_res(TFAIL, "Failed to sync test file to device");
+}
+
+static void setup(void)
+{
+	tst_sync_device_init(tst_device->dev);
+}
+
+static void cleanup(void)
+{
+	tst_sync_device_cleanup();
+}
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.mount_device = 1,
+	.all_filesystems = 1,
+	.mntpoint = MNTPOINT,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = verify_fsync,
+};
-- 
2.7.4

[LTP] [PATCH v3 5/7] syscalls/fdatasync: add sync device test-case

[Sumit Garg]

fdatasync03 tests to sync file data 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>
---
 runtest/syscalls                                  |  1 +
 testcases/kernel/syscalls/fdatasync/.gitignore    |  1 +
 testcases/kernel/syscalls/fdatasync/fdatasync03.c | 61 +++++++++++++++++++++++
 3 files changed, 63 insertions(+)
 create mode 100644 testcases/kernel/syscalls/fdatasync/fdatasync03.c

diff --git a/runtest/syscalls b/runtest/syscalls
index 0d09509..aaad651 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -291,6 +291,7 @@ fcntl36_64 fcntl36_64
 
 fdatasync01 fdatasync01
 fdatasync02 fdatasync02
+fdatasync03 fdatasync03
 
 fgetxattr01 fgetxattr01
 fgetxattr02 fgetxattr02
diff --git a/testcases/kernel/syscalls/fdatasync/.gitignore b/testcases/kernel/syscalls/fdatasync/.gitignore
index d1d3cba..dc56ad7 100644
--- a/testcases/kernel/syscalls/fdatasync/.gitignore
+++ b/testcases/kernel/syscalls/fdatasync/.gitignore
@@ -1,2 +1,3 @@
 /fdatasync01
 /fdatasync02
+/fdatasync03
diff --git a/testcases/kernel/syscalls/fdatasync/fdatasync03.c b/testcases/kernel/syscalls/fdatasync/fdatasync03.c
new file mode 100644
index 0000000..3a4f694
--- /dev/null
+++ b/testcases/kernel/syscalls/fdatasync/fdatasync03.c
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Linaro Limited. All rights reserved.
+ * Author: Sumit Garg <sumit.garg@linaro.org>
+ */
+
+/*
+ * fdatasync03
+ *
+ * It basically tests fdatasync() to sync test file data having large dirty
+ * file pages to block device. Also, it tests all supported filesystems on a
+ * test block device.
+ */
+
+#define _GNU_SOURCE
+#include <errno.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <sys/types.h>
+#include "tst_sync_device.h"
+#include "tst_test.h"
+
+#define MNTPOINT		"mnt_point"
+#define TST_FILE		MNTPOINT"/test"
+#define TST_FILE_SIZE_MB	32
+
+static void verify_fdatasync(void)
+{
+	int fd;
+
+	fd = tst_sync_device_write(TST_FILE, TST_FILE_SIZE_MB);
+
+	TEST(fdatasync(fd));
+	if (TST_RET != 0)
+		tst_brk(TFAIL | TTERRNO, "fdatasync(fd) failed");
+
+	if (tst_sync_device_check(TST_FILE_SIZE_MB))
+		tst_res(TPASS, "Test file data synced to device");
+	else
+		tst_res(TFAIL, "Failed to sync test file data to device");
+}
+
+static void setup(void)
+{
+	tst_sync_device_init(tst_device->dev);
+}
+
+static void cleanup(void)
+{
+	tst_sync_device_cleanup();
+}
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.mount_device = 1,
+	.all_filesystems = 1,
+	.mntpoint = MNTPOINT,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = verify_fdatasync,
+};
-- 
2.7.4

[LTP] [PATCH v3 6/7] syscalls/sync_file_range: Use C library wrapper if present

[Sumit Garg]

Add config check for C library wrapper for sync_file_range() syscall.
Also, check for sync_file_range() presence via dummy call rather than
kernel version check. And move fallback api to:
include/lapi/sync_file_range.h

Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
 configure.ac                                       |  1 +
 include/lapi/sync_file_range.h                     | 64 ++++++++++++++++++++++
 m4/ltp-sync_file_range.m4                          | 10 ++++
 .../sync_file_range/check_sync_file_range.h        | 23 ++++++++
 .../syscalls/sync_file_range/sync_file_range01.c   | 62 ++-------------------
 5 files changed, 103 insertions(+), 57 deletions(-)
 create mode 100644 include/lapi/sync_file_range.h
 create mode 100644 m4/ltp-sync_file_range.m4
 create mode 100644 testcases/kernel/syscalls/sync_file_range/check_sync_file_range.h

diff --git a/configure.ac b/configure.ac
index 9122b6d..d15bff3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -232,6 +232,7 @@ LTP_CHECK_RLIMIT64
 LTP_DETECT_HOST_CPU
 LTP_CHECK_PERF_EVENT
 LTP_CHECK_SYNCFS
+LTP_CHECK_SYNC_FILE_RANGE
 
 if test "x$with_numa" = xyes; then
 	LTP_CHECK_SYSCALL_NUMA
diff --git a/include/lapi/sync_file_range.h b/include/lapi/sync_file_range.h
new file mode 100644
index 0000000..7b0ef69
--- /dev/null
+++ b/include/lapi/sync_file_range.h
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) International Business Machines  Corp., 2008
+ */
+
+#ifndef SYNC_FILE_RANGE_H
+#define SYNC_FILE_RANGE_H
+
+#include <sys/types.h>
+#include "config.h"
+#include "lapi/syscalls.h"
+
+#if !defined(HAVE_SYNC_FILE_RANGE)
+
+#ifdef TST_TEST_H__
+# define TST_SYSCALL tst_syscall
+#else
+# define TST_SYSCALL ltp_syscall
+#endif
+
+/*****************************************************************************
+ * Wraper function to call sync_file_range system call
+ ******************************************************************************/
+static inline long sync_file_range(int fd, off64_t offset, off64_t nbytes,
+				   unsigned int flags)
+{
+/* arm and powerpc */
+#if (defined(__arm__) || defined(__powerpc__) || defined(__powerpc64__))
+#if (__WORDSIZE == 32)
+#if __BYTE_ORDER == __BIG_ENDIAN
+	return TST_SYSCALL(__NR_sync_file_range2, fd, flags,
+		(int)(offset >> 32), (int)offset, (int)(nbytes >> 32),
+		(int)nbytes);
+#elif __BYTE_ORDER == __LITTLE_ENDIAN
+	return TST_SYSCALL(__NR_sync_file_range2, fd, flags, (int)offset,
+		       (int)(offset >> 32), nbytes, (int)(nbytes >> 32));
+#endif
+#else
+	return TST_SYSCALL(__NR_sync_file_range2, fd, flags, offset, nbytes);
+#endif
+
+/* s390 */
+#elif (defined(__s390__) || defined(__s390x__)) && __WORDSIZE == 32
+	return TST_SYSCALL(__NR_sync_file_range, fd, (int)(offset >> 32),
+		(int)offset, (int)(nbytes >> 32), (int)nbytes, flags);
+
+/* mips */
+#elif defined(__mips__) && __WORDSIZE == 32
+#if __BYTE_ORDER == __BIG_ENDIAN
+	return TST_SYSCALL(__NR_sync_file_range, fd, 0, (int)(offset >> 32),
+		(int)offset, (int)(nbytes >> 32), (int)nbytes, flags);
+#elif __BYTE_ORDER == __LITTLE_ENDIAN
+	return TST_SYSCALL(__NR_sync_file_range, fd, 0, (int)offset,
+		(int)(offset >> 32), (int)nbytes, (int)(nbytes >> 32), flags);
+#endif
+
+/* other */
+#else
+	return TST_SYSCALL(__NR_sync_file_range, fd, offset, nbytes, flags);
+#endif
+}
+#endif
+
+#endif /* SYNC_FILE_RANGE_H */
diff --git a/m4/ltp-sync_file_range.m4 b/m4/ltp-sync_file_range.m4
new file mode 100644
index 0000000..b47a091
--- /dev/null
+++ b/m4/ltp-sync_file_range.m4
@@ -0,0 +1,10 @@
+dnl SPDX-License-Identifier: GPL-2.0-or-later
+dnl Copyright (c) 2019 Linaro Limited. All rights reserved.
+
+dnl
+dnl LTP_CHECK_SYNC_FILE_RANGE
+dnl ----------------------------
+dnl
+AC_DEFUN([LTP_CHECK_SYNC_FILE_RANGE],[
+AC_CHECK_FUNCS(sync_file_range,,)
+])
diff --git a/testcases/kernel/syscalls/sync_file_range/check_sync_file_range.h b/testcases/kernel/syscalls/sync_file_range/check_sync_file_range.h
new file mode 100644
index 0000000..3d932f6
--- /dev/null
+++ b/testcases/kernel/syscalls/sync_file_range/check_sync_file_range.h
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Linaro Limited. All rights reserved.
+ * Author: Sumit Garg <sumit.garg@linaro.org>
+ */
+
+#ifndef CHECK_SYNC_FILE_RANGE_H
+#define CHECK_SYNC_FILE_RANGE_H
+
+#include <stdbool.h>
+
+bool check_sync_file_range(void)
+{
+	int ret;
+
+	ret = sync_file_range(-1, 0, 0, 0);
+	if (ret == -1 && errno == EINVAL)
+		return false;
+
+	return true;
+}
+
+#endif /* CHECK_SYNC_FILE_RANGE_H */
diff --git a/testcases/kernel/syscalls/sync_file_range/sync_file_range01.c b/testcases/kernel/syscalls/sync_file_range/sync_file_range01.c
index cebb919..3a97183 100644
--- a/testcases/kernel/syscalls/sync_file_range/sync_file_range01.c
+++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range01.c
@@ -92,7 +92,8 @@
 #include <unistd.h>
 
 #include "test.h"
-#include "lapi/syscalls.h"
+#include "lapi/sync_file_range.h"
+#include "check_sync_file_range.h"
 
 #ifndef SYNC_FILE_RANGE_WAIT_BEFORE
 #define SYNC_FILE_RANGE_WAIT_BEFORE 1
@@ -190,48 +191,6 @@ void setup(void)
 	sfd = open(spl_file, O_RDWR | O_CREAT, 0700);
 }
 
-/*****************************************************************************
- * Wraper function to call sync_file_range system call
- ******************************************************************************/
-static inline long syncfilerange(int fd, off64_t offset, off64_t nbytes,
-				 unsigned int flags)
-{
-/* arm and powerpc */
-#if (defined(__arm__) || defined(__powerpc__) || defined(__powerpc64__))
-#if (__WORDSIZE == 32)
-#if __BYTE_ORDER == __BIG_ENDIAN
-	return ltp_syscall(__NR_sync_file_range2, fd, flags,
-		(int)(offset >> 32), (int)offset, (int)(nbytes >> 32),
-		(int)nbytes);
-#elif __BYTE_ORDER == __LITTLE_ENDIAN
-	return ltp_syscall(__NR_sync_file_range2, fd, flags, (int)offset,
-		       (int)(offset >> 32), nbytes, (int)(nbytes >> 32));
-#endif
-#else
-	return ltp_syscall(__NR_sync_file_range2, fd, flags, offset, nbytes);
-#endif
-
-/* s390 */
-#elif (defined(__s390__) || defined(__s390x__)) && __WORDSIZE == 32
-	return ltp_syscall(__NR_sync_file_range, fd, (int)(offset >> 32),
-		(int)offset, (int)(nbytes >> 32), (int)nbytes, flags);
-
-/* mips */
-#elif defined(__mips__) && __WORDSIZE == 32
-#if __BYTE_ORDER == __BIG_ENDIAN
-	return ltp_syscall(__NR_sync_file_range, fd, 0, (int)(offset >> 32),
-		(int)offset, (int)(nbytes >> 32), (int)nbytes, flags);
-#elif __BYTE_ORDER == __LITTLE_ENDIAN
-	return ltp_syscall(__NR_sync_file_range, fd, 0, (int)offset,
-		(int)(offset >> 32), (int)nbytes, (int)(nbytes >> 32), flags);
-#endif
-
-/* other */
-#else
-	return ltp_syscall(__NR_sync_file_range, fd, offset, nbytes, flags);
-#endif
-}
-
 /******************************************************************************/
 /*									    */
 /* Function:    main							  */
@@ -258,24 +217,13 @@ int main(int ac, char **av)
 
 	tst_parse_opts(ac, av, NULL, NULL);
 
-#if defined(__powerpc__) || defined(__powerpc64__)	/* for PPC, kernel version > 2.6.21 needed */
-	if (tst_kvercmp(2, 16, 22) < 0) {
-		tst_brkm(TCONF, NULL,
-			 "System doesn't support execution of the test");
-	}
-#else
-	/* For other archs, need kernel version > 2.6.16 */
-
-	if (tst_kvercmp(2, 6, 17) < 0) {
-		tst_brkm(TCONF, NULL,
-			 "System doesn't support execution of the test");
-	}
-#endif
+	if (!check_sync_file_range())
+		tst_brkm(TCONF, NULL, "sync_file_range() not supported");
 
 	setup();
 
 	for (test_index = 0; test_index < TST_TOTAL; test_index++) {
-		TEST(syncfilerange
+		TEST(sync_file_range
 		     (*(test_data[test_index].fd),
 		      test_data[test_index].offset,
 		      test_data[test_index].nbytes,
-- 
2.7.4

[LTP] [PATCH v3 7/7] syscalls/sync_file_range: add sync device test-case

[Sumit Garg]

sync_file_range02 tests to sync file data range 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>
---
 runtest/syscalls                                   |  1 +
 .../kernel/syscalls/sync_file_range/.gitignore     |  1 +
 .../syscalls/sync_file_range/sync_file_range02.c   | 70 ++++++++++++++++++++++
 3 files changed, 72 insertions(+)
 create mode 100644 testcases/kernel/syscalls/sync_file_range/sync_file_range02.c

diff --git a/runtest/syscalls b/runtest/syscalls
index aaad651..70d3561 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -1353,6 +1353,7 @@ syncfs01 syncfs01
 
 #testcases for sync_file_range
 sync_file_range01 sync_file_range01
+sync_file_range02 sync_file_range02
 
 syscall01 syscall01
 
diff --git a/testcases/kernel/syscalls/sync_file_range/.gitignore b/testcases/kernel/syscalls/sync_file_range/.gitignore
index 3f6bd75..e6485f7 100644
--- a/testcases/kernel/syscalls/sync_file_range/.gitignore
+++ b/testcases/kernel/syscalls/sync_file_range/.gitignore
@@ -1 +1,2 @@
 /sync_file_range01
+/sync_file_range02
diff --git a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
new file mode 100644
index 0000000..fb7a5f7
--- /dev/null
+++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Linaro Limited. All rights reserved.
+ * Author: Sumit Garg <sumit.garg@linaro.org>
+ */
+
+/*
+ * sync_file_range02
+ *
+ * It basically tests sync_file_range() to sync test file range having large
+ * dirty file pages to block device. Also, it tests all supported filesystems
+ * on a test block device.
+ */
+
+#define _GNU_SOURCE
+#include <errno.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <sys/types.h>
+#include "tst_sync_device.h"
+#include "tst_test.h"
+#include "lapi/sync_file_range.h"
+#include "check_sync_file_range.h"
+
+#define MNTPOINT		"mnt_point"
+#define TST_FILE		MNTPOINT"/test"
+#define TST_FILE_SIZE_MB	32
+#define SIZE_MB			(1024*1024)
+
+static void verify_sync_file_range(void)
+{
+	int fd;
+
+	fd = tst_sync_device_write(TST_FILE, TST_FILE_SIZE_MB);
+
+	TEST(sync_file_range(fd, 0, TST_FILE_SIZE_MB * SIZE_MB,
+			     SYNC_FILE_RANGE_WAIT_BEFORE |
+			     SYNC_FILE_RANGE_WRITE |
+			     SYNC_FILE_RANGE_WAIT_AFTER));
+	if (TST_RET != 0)
+		tst_brk(TFAIL | TTERRNO, "sync_file_range() failed");
+
+	if (tst_sync_device_check(TST_FILE_SIZE_MB))
+		tst_res(TPASS, "Test file range synced to device");
+	else
+		tst_res(TFAIL, "Failed to sync test file range to device");
+}
+
+static void setup(void)
+{
+	tst_sync_device_init(tst_device->dev);
+}
+
+static void cleanup(void)
+{
+	if (!check_sync_file_range())
+		tst_brk(TCONF, "sync_file_range() not supported");
+
+	tst_sync_device_cleanup();
+}
+
+static struct tst_test test = {
+	.needs_root = 1,
+	.mount_device = 1,
+	.all_filesystems = 1,
+	.mntpoint = MNTPOINT,
+	.setup = setup,
+	.cleanup = cleanup,
+	.test_all = verify_sync_file_range,
+};
-- 
2.7.4

[LTP] [PATCH v3 1/7] lib: Add library functions for sync related syscalls

[Cyril Hrubis]

Hi!
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> ---
>  include/tst_sync_device.h | 17 ++++++++++
>  lib/tst_sync_device.c     | 80 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 97 insertions(+)
>  create mode 100644 include/tst_sync_device.h
>  create mode 100644 lib/tst_sync_device.c
> 
> diff --git a/include/tst_sync_device.h b/include/tst_sync_device.h
> new file mode 100644
> index 0000000..b07c490
> --- /dev/null
> +++ b/include/tst_sync_device.h
> @@ -0,0 +1,17 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019 Linaro Limited. All rights reserved.
> + * Author: Sumit Garg <sumit.garg@linaro.org>
> + */
> +
> +#ifndef TST_SYNC_DEVICE_H__
> +#define TST_SYNC_DEVICE_H__
> +
> +#include <stdbool.h>
> +
> +void tst_sync_device_init(const char *dev);
> +int tst_sync_device_write(const char *file, unsigned int size_mb);
> +bool tst_sync_device_check(unsigned int size_mb);
> +void tst_sync_device_cleanup(void);
> +
> +#endif
> diff --git a/lib/tst_sync_device.c b/lib/tst_sync_device.c
> new file mode 100644
> index 0000000..5a0a17c
> --- /dev/null
> +++ b/lib/tst_sync_device.c
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019 Linaro Limited. All rights reserved.
> + * Author: Sumit Garg <sumit.garg@linaro.org>
> + */
> +
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <sys/types.h>
> +
> +#define TST_NO_DEFAULT_MAIN
> +#include "tst_test.h"
> +#include "lapi/stat.h"
> +#include "tst_sync_device.h"
> +
> +#define SIZE_MB			(1024*1024)
> +#define MODE			0644
> +
> +static char dev_stat_path[1024];
> +static char *buffer;
> +static int fd;
> +static unsigned long prev_write_sec;
> +
> +void tst_sync_device_init(const char *dev)
> +{
> +	struct stat st;
> +
> +	snprintf(dev_stat_path, sizeof(dev_stat_path), "/sys/block/%s/stat",
> +		 strrchr(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);
> +}
> +
> +int tst_sync_device_write(const char *file, unsigned int size_mb)
> +{
> +	unsigned int counter;
                     ^
		     It's kind of strange to name this variable anything
		     else but i.

> +	SAFE_FILE_SCANF(dev_stat_path, "%*s %*s %*s %*s %*s %*s %lu",
> +			&prev_write_sec);
> +
> +	fd =  SAFE_OPEN(file, O_RDWR|O_CREAT, MODE);
            ^
	    Extra space.

> +	/* Filling the test file */
         ^
	 Useless comment, it's pretty clear what we do here.

> +	for (counter = 0; counter < size_mb; counter++)
> +		SAFE_WRITE(1, fd, buffer, SIZE_MB);
> +
> +	return fd;
> +}
> +
> +bool tst_sync_device_check(unsigned int size_mb)
> +{
> +	unsigned long write_sec = 0;
> +	bool res = false;
> +
> +	SAFE_FILE_SCANF(dev_stat_path, "%*s %*s %*s %*s %*s %*s %lu",
> +			&write_sec);
> +
> +	if ((write_sec - prev_write_sec) * 512 >=
> +	    (size_mb * SIZE_MB))
> +		res = true;
> +
> +	SAFE_CLOSE(fd);
> +
> +	return res;
> +}

I do not like that this API is tailored just to this specific usecase.

I would rather see the code that writes the file separated from the code
that measures the bytes written.

Maybe we just need a tst_dev_bytes_written() function that would return
the bytes written since the last call of the function so that we could
do:

	fd = SAFE_OPEN(...);

	tst_dev_blocks_written(tst_device.dev);

	tst_fill_fd(fd, 0, TST_MB, size_mb);

	TEST(fdsync(fd));

	if (TST_RET)
		tst_brk(TFAIL | TTERRNO, "syncfd() failed with %i", TST_RET);

	written = tst_dev_blocks_written(tst_device.dev);

	SAFE_CLOSE(fd);

	if (written >= size_mb * TST_DEV_BLOCKS_IN_MB)
		tst_res(TPASS, ...);
	else
		tst_res(TFAIL, ...);


The test a bit longer, but the library functions are more reusable, if
you do 'git grep -B 1 SAFE_WRITE' you can see that the tst_fill_fd
function that loops over SAFE_WRITE could be used in a few places
already.

> +void tst_sync_device_cleanup(void)
> +{
> +	if (buffer)
> +		free(buffer);
> +
> +	if (fd > 0)
> +		SAFE_CLOSE(fd);
> +}
> -- 
> 2.7.4
> 

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 3/7] syscalls/sync: add sync device test-case

[Cyril Hrubis]

Hi!
> Also define TEST_VOID macro in tst_test.h.

This should be in a separate patch, otherwise this all looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 6/7] syscalls/sync_file_range: Use C library wrapper if present

[Cyril Hrubis]

Hi!
> diff --git a/configure.ac b/configure.ac
> index 9122b6d..d15bff3 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -232,6 +232,7 @@ LTP_CHECK_RLIMIT64
>  LTP_DETECT_HOST_CPU
>  LTP_CHECK_PERF_EVENT
>  LTP_CHECK_SYNCFS
> +LTP_CHECK_SYNC_FILE_RANGE
>  
>  if test "x$with_numa" = xyes; then
>  	LTP_CHECK_SYSCALL_NUMA
> diff --git a/include/lapi/sync_file_range.h b/include/lapi/sync_file_range.h
> new file mode 100644
> index 0000000..7b0ef69
> --- /dev/null
> +++ b/include/lapi/sync_file_range.h
> @@ -0,0 +1,64 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) International Business Machines  Corp., 2008
> + */
> +
> +#ifndef SYNC_FILE_RANGE_H
> +#define SYNC_FILE_RANGE_H
> +
> +#include <sys/types.h>
> +#include "config.h"
> +#include "lapi/syscalls.h"
> +
> +#if !defined(HAVE_SYNC_FILE_RANGE)
> +
> +#ifdef TST_TEST_H__
> +# define TST_SYSCALL tst_syscall
> +#else
> +# define TST_SYSCALL ltp_syscall
> +#endif
> +
> +/*****************************************************************************
> + * Wraper function to call sync_file_range system call
> + ******************************************************************************/
> +static inline long sync_file_range(int fd, off64_t offset, off64_t nbytes,
> +				   unsigned int flags)
> +{
> +/* arm and powerpc */
> +#if (defined(__arm__) || defined(__powerpc__) || defined(__powerpc64__))
> +#if (__WORDSIZE == 32)
> +#if __BYTE_ORDER == __BIG_ENDIAN
> +	return TST_SYSCALL(__NR_sync_file_range2, fd, flags,
> +		(int)(offset >> 32), (int)offset, (int)(nbytes >> 32),
> +		(int)nbytes);
> +#elif __BYTE_ORDER == __LITTLE_ENDIAN
> +	return TST_SYSCALL(__NR_sync_file_range2, fd, flags, (int)offset,
> +		       (int)(offset >> 32), nbytes, (int)(nbytes >> 32));
> +#endif
> +#else
> +	return TST_SYSCALL(__NR_sync_file_range2, fd, flags, offset, nbytes);
> +#endif
> +
> +/* s390 */

Instead of comment like this the usuall way how to make this maze easier
to read is to stick spaces after the hash to inner conditions, so this
would become:

#if (defined(__arm__) || defined (__powerpc__) || defined(__powerpc64__))
# if (__WORDSIZE == 32)
#  if __BYTE_ORDER == __BIG_ENDIAN
	return ...;
#  elif __BYTE_ORDER == __LITTLE_ENDIAN
	return ...;
#  endif
# else
	return ...;
#endif


> +#elif (defined(__s390__) || defined(__s390x__)) && __WORDSIZE == 32
> +	return TST_SYSCALL(__NR_sync_file_range, fd, (int)(offset >> 32),
> +		(int)offset, (int)(nbytes >> 32), (int)nbytes, flags);
> +
> +/* mips */
> +#elif defined(__mips__) && __WORDSIZE == 32
> +#if __BYTE_ORDER == __BIG_ENDIAN
> +	return TST_SYSCALL(__NR_sync_file_range, fd, 0, (int)(offset >> 32),
> +		(int)offset, (int)(nbytes >> 32), (int)nbytes, flags);
> +#elif __BYTE_ORDER == __LITTLE_ENDIAN
> +	return TST_SYSCALL(__NR_sync_file_range, fd, 0, (int)offset,
> +		(int)(offset >> 32), (int)nbytes, (int)(nbytes >> 32), flags);
> +#endif
> +
> +/* other */
> +#else
> +	return TST_SYSCALL(__NR_sync_file_range, fd, offset, nbytes, flags);
> +#endif
> +}
> +#endif



> +#endif /* SYNC_FILE_RANGE_H */
> diff --git a/m4/ltp-sync_file_range.m4 b/m4/ltp-sync_file_range.m4
> new file mode 100644
> index 0000000..b47a091
> --- /dev/null
> +++ b/m4/ltp-sync_file_range.m4
> @@ -0,0 +1,10 @@
> +dnl SPDX-License-Identifier: GPL-2.0-or-later
> +dnl Copyright (c) 2019 Linaro Limited. All rights reserved.
> +
> +dnl
> +dnl LTP_CHECK_SYNC_FILE_RANGE
> +dnl ----------------------------
> +dnl
> +AC_DEFUN([LTP_CHECK_SYNC_FILE_RANGE],[
> +AC_CHECK_FUNCS(sync_file_range,,)
> +])
> diff --git a/testcases/kernel/syscalls/sync_file_range/check_sync_file_range.h b/testcases/kernel/syscalls/sync_file_range/check_sync_file_range.h
> new file mode 100644
> index 0000000..3d932f6
> --- /dev/null
> +++ b/testcases/kernel/syscalls/sync_file_range/check_sync_file_range.h
> @@ -0,0 +1,23 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019 Linaro Limited. All rights reserved.
> + * Author: Sumit Garg <sumit.garg@linaro.org>
> + */
> +
> +#ifndef CHECK_SYNC_FILE_RANGE_H
> +#define CHECK_SYNC_FILE_RANGE_H
> +
> +#include <stdbool.h>
> +
> +bool check_sync_file_range(void)
> +{
> +	int ret;
> +
> +	ret = sync_file_range(-1, 0, 0, 0);
> +	if (ret == -1 && errno == EINVAL)
> +		return false;
> +
> +	return true;
> +}

I would rather stick to return 0 and return 1 instead of including the
stdbool, but that is very minor.

> +#endif /* CHECK_SYNC_FILE_RANGE_H */
> diff --git a/testcases/kernel/syscalls/sync_file_range/sync_file_range01.c b/testcases/kernel/syscalls/sync_file_range/sync_file_range01.c
> index cebb919..3a97183 100644
> --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range01.c
> +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range01.c
> @@ -92,7 +92,8 @@
>  #include <unistd.h>
>  
>  #include "test.h"
> -#include "lapi/syscalls.h"
> +#include "lapi/sync_file_range.h"
> +#include "check_sync_file_range.h"
>  
>  #ifndef SYNC_FILE_RANGE_WAIT_BEFORE
>  #define SYNC_FILE_RANGE_WAIT_BEFORE 1
> @@ -190,48 +191,6 @@ void setup(void)
>  	sfd = open(spl_file, O_RDWR | O_CREAT, 0700);
>  }
>  
> -/*****************************************************************************
> - * Wraper function to call sync_file_range system call
> - ******************************************************************************/
> -static inline long syncfilerange(int fd, off64_t offset, off64_t nbytes,
> -				 unsigned int flags)
> -{
> -/* arm and powerpc */
> -#if (defined(__arm__) || defined(__powerpc__) || defined(__powerpc64__))
> -#if (__WORDSIZE == 32)
> -#if __BYTE_ORDER == __BIG_ENDIAN
> -	return ltp_syscall(__NR_sync_file_range2, fd, flags,
> -		(int)(offset >> 32), (int)offset, (int)(nbytes >> 32),
> -		(int)nbytes);
> -#elif __BYTE_ORDER == __LITTLE_ENDIAN
> -	return ltp_syscall(__NR_sync_file_range2, fd, flags, (int)offset,
> -		       (int)(offset >> 32), nbytes, (int)(nbytes >> 32));
> -#endif
> -#else
> -	return ltp_syscall(__NR_sync_file_range2, fd, flags, offset, nbytes);
> -#endif
> -
> -/* s390 */
> -#elif (defined(__s390__) || defined(__s390x__)) && __WORDSIZE == 32
> -	return ltp_syscall(__NR_sync_file_range, fd, (int)(offset >> 32),
> -		(int)offset, (int)(nbytes >> 32), (int)nbytes, flags);
> -
> -/* mips */
> -#elif defined(__mips__) && __WORDSIZE == 32
> -#if __BYTE_ORDER == __BIG_ENDIAN
> -	return ltp_syscall(__NR_sync_file_range, fd, 0, (int)(offset >> 32),
> -		(int)offset, (int)(nbytes >> 32), (int)nbytes, flags);
> -#elif __BYTE_ORDER == __LITTLE_ENDIAN
> -	return ltp_syscall(__NR_sync_file_range, fd, 0, (int)offset,
> -		(int)(offset >> 32), (int)nbytes, (int)(nbytes >> 32), flags);
> -#endif
> -
> -/* other */
> -#else
> -	return ltp_syscall(__NR_sync_file_range, fd, offset, nbytes, flags);
> -#endif
> -}
> -
>  /******************************************************************************/
>  /*									    */
>  /* Function:    main							  */
> @@ -258,24 +217,13 @@ int main(int ac, char **av)
>  
>  	tst_parse_opts(ac, av, NULL, NULL);
>  
> -#if defined(__powerpc__) || defined(__powerpc64__)	/* for PPC, kernel version > 2.6.21 needed */
> -	if (tst_kvercmp(2, 16, 22) < 0) {
> -		tst_brkm(TCONF, NULL,
> -			 "System doesn't support execution of the test");
> -	}
> -#else
> -	/* For other archs, need kernel version > 2.6.16 */
> -
> -	if (tst_kvercmp(2, 6, 17) < 0) {
> -		tst_brkm(TCONF, NULL,
> -			 "System doesn't support execution of the test");
> -	}
> -#endif
> +	if (!check_sync_file_range())
> +		tst_brkm(TCONF, NULL, "sync_file_range() not supported");
>  
>  	setup();
>  
>  	for (test_index = 0; test_index < TST_TOTAL; test_index++) {
> -		TEST(syncfilerange
> +		TEST(sync_file_range
>  		     (*(test_data[test_index].fd),
>  		      test_data[test_index].offset,
>  		      test_data[test_index].nbytes,

Other than the minor things, this is great work.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 7/7] syscalls/sync_file_range: add sync device test-case

[Cyril Hrubis]

Hi!
> sync_file_range02 tests to sync file data range 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>
> ---
>  runtest/syscalls                                   |  1 +
>  .../kernel/syscalls/sync_file_range/.gitignore     |  1 +
>  .../syscalls/sync_file_range/sync_file_range02.c   | 70 ++++++++++++++++++++++
>  3 files changed, 72 insertions(+)
>  create mode 100644 testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
> 
> diff --git a/runtest/syscalls b/runtest/syscalls
> index aaad651..70d3561 100644
> --- a/runtest/syscalls
> +++ b/runtest/syscalls
> @@ -1353,6 +1353,7 @@ syncfs01 syncfs01
>  
>  #testcases for sync_file_range
>  sync_file_range01 sync_file_range01
> +sync_file_range02 sync_file_range02
>  
>  syscall01 syscall01
>  
> diff --git a/testcases/kernel/syscalls/sync_file_range/.gitignore b/testcases/kernel/syscalls/sync_file_range/.gitignore
> index 3f6bd75..e6485f7 100644
> --- a/testcases/kernel/syscalls/sync_file_range/.gitignore
> +++ b/testcases/kernel/syscalls/sync_file_range/.gitignore
> @@ -1 +1,2 @@
>  /sync_file_range01
> +/sync_file_range02
> diff --git a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
> new file mode 100644
> index 0000000..fb7a5f7
> --- /dev/null
> +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2019 Linaro Limited. All rights reserved.
> + * Author: Sumit Garg <sumit.garg@linaro.org>
> + */
> +
> +/*
> + * sync_file_range02
> + *
> + * It basically tests sync_file_range() to sync test file range having large
> + * dirty file pages to block device. Also, it tests all supported filesystems
> + * on a test block device.
> + */
> +
> +#define _GNU_SOURCE
> +#include <errno.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <sys/types.h>
> +#include "tst_sync_device.h"
> +#include "tst_test.h"
> +#include "lapi/sync_file_range.h"
> +#include "check_sync_file_range.h"
> +
> +#define MNTPOINT		"mnt_point"
> +#define TST_FILE		MNTPOINT"/test"
> +#define TST_FILE_SIZE_MB	32
> +#define SIZE_MB			(1024*1024)
> +
> +static void verify_sync_file_range(void)
> +{
> +	int fd;
> +
> +	fd = tst_sync_device_write(TST_FILE, TST_FILE_SIZE_MB);
> +
> +	TEST(sync_file_range(fd, 0, TST_FILE_SIZE_MB * SIZE_MB,
> +			     SYNC_FILE_RANGE_WAIT_BEFORE |
> +			     SYNC_FILE_RANGE_WRITE |
> +			     SYNC_FILE_RANGE_WAIT_AFTER));
> +	if (TST_RET != 0)
> +		tst_brk(TFAIL | TTERRNO, "sync_file_range() failed");
> +
> +	if (tst_sync_device_check(TST_FILE_SIZE_MB))
> +		tst_res(TPASS, "Test file range synced to device");
> +	else
> +		tst_res(TFAIL, "Failed to sync test file range to device");
> +}
> +
> +static void setup(void)
> +{
> +	tst_sync_device_init(tst_device->dev);
> +}
> +
> +static void cleanup(void)
> +{
> +	if (!check_sync_file_range())
> +		tst_brk(TCONF, "sync_file_range() not supported");

This check is supposed to be in the test setup, right?

> +	tst_sync_device_cleanup();
> +}
> +
> +static struct tst_test test = {
> +	.needs_root = 1,
> +	.mount_device = 1,
> +	.all_filesystems = 1,
> +	.mntpoint = MNTPOINT,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.test_all = verify_sync_file_range,
> +};
> -- 
> 2.7.4
> 

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 1/7] lib: Add library functions for sync related syscalls

[Sumit Garg]

On Tue, 19 Feb 2019 at 19:36, Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > ---
> >  include/tst_sync_device.h | 17 ++++++++++
> >  lib/tst_sync_device.c     | 80 +++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 97 insertions(+)
> >  create mode 100644 include/tst_sync_device.h
> >  create mode 100644 lib/tst_sync_device.c
> >
> > diff --git a/include/tst_sync_device.h b/include/tst_sync_device.h
> > new file mode 100644
> > index 0000000..b07c490
> > --- /dev/null
> > +++ b/include/tst_sync_device.h
> > @@ -0,0 +1,17 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2019 Linaro Limited. All rights reserved.
> > + * Author: Sumit Garg <sumit.garg@linaro.org>
> > + */
> > +
> > +#ifndef TST_SYNC_DEVICE_H__
> > +#define TST_SYNC_DEVICE_H__
> > +
> > +#include <stdbool.h>
> > +
> > +void tst_sync_device_init(const char *dev);
> > +int tst_sync_device_write(const char *file, unsigned int size_mb);
> > +bool tst_sync_device_check(unsigned int size_mb);
> > +void tst_sync_device_cleanup(void);
> > +
> > +#endif
> > diff --git a/lib/tst_sync_device.c b/lib/tst_sync_device.c
> > new file mode 100644
> > index 0000000..5a0a17c
> > --- /dev/null
> > +++ b/lib/tst_sync_device.c
> > @@ -0,0 +1,80 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2019 Linaro Limited. All rights reserved.
> > + * Author: Sumit Garg <sumit.garg@linaro.org>
> > + */
> > +
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +#include <sys/types.h>
> > +
> > +#define TST_NO_DEFAULT_MAIN
> > +#include "tst_test.h"
> > +#include "lapi/stat.h"
> > +#include "tst_sync_device.h"
> > +
> > +#define SIZE_MB                      (1024*1024)
> > +#define MODE                 0644
> > +
> > +static char dev_stat_path[1024];
> > +static char *buffer;
> > +static int fd;
> > +static unsigned long prev_write_sec;
> > +
> > +void tst_sync_device_init(const char *dev)
> > +{
> > +     struct stat st;
> > +
> > +     snprintf(dev_stat_path, sizeof(dev_stat_path), "/sys/block/%s/stat",
> > +              strrchr(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);
> > +}
> > +
> > +int tst_sync_device_write(const char *file, unsigned int size_mb)
> > +{
> > +     unsigned int counter;
>                      ^
>                      It's kind of strange to name this variable anything
>                      else but i.
>

Will use i instead.

> > +     SAFE_FILE_SCANF(dev_stat_path, "%*s %*s %*s %*s %*s %*s %lu",
> > +                     &prev_write_sec);
> > +
> > +     fd =  SAFE_OPEN(file, O_RDWR|O_CREAT, MODE);
>             ^
>             Extra space.
>

Will fix.

> > +     /* Filling the test file */
>          ^
>          Useless comment, it's pretty clear what we do here.
>

Will remove this comment.


> > +     for (counter = 0; counter < size_mb; counter++)
> > +             SAFE_WRITE(1, fd, buffer, SIZE_MB);
> > +
> > +     return fd;
> > +}
> > +
> > +bool tst_sync_device_check(unsigned int size_mb)
> > +{
> > +     unsigned long write_sec = 0;
> > +     bool res = false;
> > +
> > +     SAFE_FILE_SCANF(dev_stat_path, "%*s %*s %*s %*s %*s %*s %lu",
> > +                     &write_sec);
> > +
> > +     if ((write_sec - prev_write_sec) * 512 >=
> > +         (size_mb * SIZE_MB))
> > +             res = true;
> > +
> > +     SAFE_CLOSE(fd);
> > +
> > +     return res;
> > +}
>
> I do not like that this API is tailored just to this specific usecase.
>
> I would rather see the code that writes the file separated from the code
> that measures the bytes written.
>

Agree. Actually my initial idea was to tailor it for maximal code re-use.

> Maybe we just need a tst_dev_bytes_written() function that would return
> the bytes written since the last call of the function so that we could
> do:
>
>         fd = SAFE_OPEN(...);
>
>         tst_dev_blocks_written(tst_device.dev);
>
>         tst_fill_fd(fd, 0, TST_MB, size_mb);
>
>         TEST(fdsync(fd));
>
>         if (TST_RET)
>                 tst_brk(TFAIL | TTERRNO, "syncfd() failed with %i", TST_RET);
>
>         written = tst_dev_blocks_written(tst_device.dev);
>
>         SAFE_CLOSE(fd);
>
>         if (written >= size_mb * TST_DEV_BLOCKS_IN_MB)
>                 tst_res(TPASS, ...);
>         else
>                 tst_res(TFAIL, ...);
>

Seems to be nicer approach. So rather than new file should I add
tst_dev_bytes_written() function in "lib/tst_device.c" file?

>
> The test a bit longer, but the library functions are more reusable, if
> you do 'git grep -B 1 SAFE_WRITE' you can see that the tst_fill_fd
> function that loops over SAFE_WRITE could be used in a few places
> already.
>

I am not able to locate tst_fill_fd function. Are you referring to
tst_fill_file function? If yes, then we could split that function to
create tst_fill_fd function.

-Sumit

> > +void tst_sync_device_cleanup(void)
> > +{
> > +     if (buffer)
> > +             free(buffer);
> > +
> > +     if (fd > 0)
> > +             SAFE_CLOSE(fd);
> > +}
> > --
> > 2.7.4
> >
>
> --
> Cyril Hrubis
> chrubis@suse.cz

[LTP] [PATCH v3 3/7] syscalls/sync: add sync device test-case

[Sumit Garg]

On Tue, 19 Feb 2019 at 19:39, Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > Also define TEST_VOID macro in tst_test.h.
>
> This should be in a separate patch, otherwise this all looks good.
>

Sure, will create separate patch for this.

-Sumit

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

[LTP] [PATCH v3 6/7] syscalls/sync_file_range: Use C library wrapper if present

[Sumit Garg]

On Tue, 19 Feb 2019 at 19:49, Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > diff --git a/configure.ac b/configure.ac
> > index 9122b6d..d15bff3 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -232,6 +232,7 @@ LTP_CHECK_RLIMIT64
> >  LTP_DETECT_HOST_CPU
> >  LTP_CHECK_PERF_EVENT
> >  LTP_CHECK_SYNCFS
> > +LTP_CHECK_SYNC_FILE_RANGE
> >
> >  if test "x$with_numa" = xyes; then
> >       LTP_CHECK_SYSCALL_NUMA
> > diff --git a/include/lapi/sync_file_range.h b/include/lapi/sync_file_range.h
> > new file mode 100644
> > index 0000000..7b0ef69
> > --- /dev/null
> > +++ b/include/lapi/sync_file_range.h
> > @@ -0,0 +1,64 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) International Business Machines  Corp., 2008
> > + */
> > +
> > +#ifndef SYNC_FILE_RANGE_H
> > +#define SYNC_FILE_RANGE_H
> > +
> > +#include <sys/types.h>
> > +#include "config.h"
> > +#include "lapi/syscalls.h"
> > +
> > +#if !defined(HAVE_SYNC_FILE_RANGE)
> > +
> > +#ifdef TST_TEST_H__
> > +# define TST_SYSCALL tst_syscall
> > +#else
> > +# define TST_SYSCALL ltp_syscall
> > +#endif
> > +
> > +/*****************************************************************************
> > + * Wraper function to call sync_file_range system call
> > + ******************************************************************************/
> > +static inline long sync_file_range(int fd, off64_t offset, off64_t nbytes,
> > +                                unsigned int flags)
> > +{
> > +/* arm and powerpc */
> > +#if (defined(__arm__) || defined(__powerpc__) || defined(__powerpc64__))
> > +#if (__WORDSIZE == 32)
> > +#if __BYTE_ORDER == __BIG_ENDIAN
> > +     return TST_SYSCALL(__NR_sync_file_range2, fd, flags,
> > +             (int)(offset >> 32), (int)offset, (int)(nbytes >> 32),
> > +             (int)nbytes);
> > +#elif __BYTE_ORDER == __LITTLE_ENDIAN
> > +     return TST_SYSCALL(__NR_sync_file_range2, fd, flags, (int)offset,
> > +                    (int)(offset >> 32), nbytes, (int)(nbytes >> 32));
> > +#endif
> > +#else
> > +     return TST_SYSCALL(__NR_sync_file_range2, fd, flags, offset, nbytes);
> > +#endif
> > +
> > +/* s390 */
>
> Instead of comment like this the usuall way how to make this maze easier
> to read is to stick spaces after the hash to inner conditions, so this
> would become:
>
> #if (defined(__arm__) || defined (__powerpc__) || defined(__powerpc64__))
> # if (__WORDSIZE == 32)
> #  if __BYTE_ORDER == __BIG_ENDIAN
>         return ...;
> #  elif __BYTE_ORDER == __LITTLE_ENDIAN
>         return ...;
> #  endif
> # else
>         return ...;
> #endif
>

Sure, will remove comments and insert spaces.

>
> > +#elif (defined(__s390__) || defined(__s390x__)) && __WORDSIZE == 32
> > +     return TST_SYSCALL(__NR_sync_file_range, fd, (int)(offset >> 32),
> > +             (int)offset, (int)(nbytes >> 32), (int)nbytes, flags);
> > +
> > +/* mips */
> > +#elif defined(__mips__) && __WORDSIZE == 32
> > +#if __BYTE_ORDER == __BIG_ENDIAN
> > +     return TST_SYSCALL(__NR_sync_file_range, fd, 0, (int)(offset >> 32),
> > +             (int)offset, (int)(nbytes >> 32), (int)nbytes, flags);
> > +#elif __BYTE_ORDER == __LITTLE_ENDIAN
> > +     return TST_SYSCALL(__NR_sync_file_range, fd, 0, (int)offset,
> > +             (int)(offset >> 32), (int)nbytes, (int)(nbytes >> 32), flags);
> > +#endif
> > +
> > +/* other */
> > +#else
> > +     return TST_SYSCALL(__NR_sync_file_range, fd, offset, nbytes, flags);
> > +#endif
> > +}
> > +#endif
>
>
>
> > +#endif /* SYNC_FILE_RANGE_H */
> > diff --git a/m4/ltp-sync_file_range.m4 b/m4/ltp-sync_file_range.m4
> > new file mode 100644
> > index 0000000..b47a091
> > --- /dev/null
> > +++ b/m4/ltp-sync_file_range.m4
> > @@ -0,0 +1,10 @@
> > +dnl SPDX-License-Identifier: GPL-2.0-or-later
> > +dnl Copyright (c) 2019 Linaro Limited. All rights reserved.
> > +
> > +dnl
> > +dnl LTP_CHECK_SYNC_FILE_RANGE
> > +dnl ----------------------------
> > +dnl
> > +AC_DEFUN([LTP_CHECK_SYNC_FILE_RANGE],[
> > +AC_CHECK_FUNCS(sync_file_range,,)
> > +])
> > diff --git a/testcases/kernel/syscalls/sync_file_range/check_sync_file_range.h b/testcases/kernel/syscalls/sync_file_range/check_sync_file_range.h
> > new file mode 100644
> > index 0000000..3d932f6
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/sync_file_range/check_sync_file_range.h
> > @@ -0,0 +1,23 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2019 Linaro Limited. All rights reserved.
> > + * Author: Sumit Garg <sumit.garg@linaro.org>
> > + */
> > +
> > +#ifndef CHECK_SYNC_FILE_RANGE_H
> > +#define CHECK_SYNC_FILE_RANGE_H
> > +
> > +#include <stdbool.h>
> > +
> > +bool check_sync_file_range(void)
> > +{
> > +     int ret;
> > +
> > +     ret = sync_file_range(-1, 0, 0, 0);
> > +     if (ret == -1 && errno == EINVAL)
> > +             return false;
> > +
> > +     return true;
> > +}
>
> I would rather stick to return 0 and return 1 instead of including the
> stdbool, but that is very minor.
>

Ok will use 0 and 1 then.

> > +#endif /* CHECK_SYNC_FILE_RANGE_H */
> > diff --git a/testcases/kernel/syscalls/sync_file_range/sync_file_range01.c b/testcases/kernel/syscalls/sync_file_range/sync_file_range01.c
> > index cebb919..3a97183 100644
> > --- a/testcases/kernel/syscalls/sync_file_range/sync_file_range01.c
> > +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range01.c
> > @@ -92,7 +92,8 @@
> >  #include <unistd.h>
> >
> >  #include "test.h"
> > -#include "lapi/syscalls.h"
> > +#include "lapi/sync_file_range.h"
> > +#include "check_sync_file_range.h"
> >
> >  #ifndef SYNC_FILE_RANGE_WAIT_BEFORE
> >  #define SYNC_FILE_RANGE_WAIT_BEFORE 1
> > @@ -190,48 +191,6 @@ void setup(void)
> >       sfd = open(spl_file, O_RDWR | O_CREAT, 0700);
> >  }
> >
> > -/*****************************************************************************
> > - * Wraper function to call sync_file_range system call
> > - ******************************************************************************/
> > -static inline long syncfilerange(int fd, off64_t offset, off64_t nbytes,
> > -                              unsigned int flags)
> > -{
> > -/* arm and powerpc */
> > -#if (defined(__arm__) || defined(__powerpc__) || defined(__powerpc64__))
> > -#if (__WORDSIZE == 32)
> > -#if __BYTE_ORDER == __BIG_ENDIAN
> > -     return ltp_syscall(__NR_sync_file_range2, fd, flags,
> > -             (int)(offset >> 32), (int)offset, (int)(nbytes >> 32),
> > -             (int)nbytes);
> > -#elif __BYTE_ORDER == __LITTLE_ENDIAN
> > -     return ltp_syscall(__NR_sync_file_range2, fd, flags, (int)offset,
> > -                    (int)(offset >> 32), nbytes, (int)(nbytes >> 32));
> > -#endif
> > -#else
> > -     return ltp_syscall(__NR_sync_file_range2, fd, flags, offset, nbytes);
> > -#endif
> > -
> > -/* s390 */
> > -#elif (defined(__s390__) || defined(__s390x__)) && __WORDSIZE == 32
> > -     return ltp_syscall(__NR_sync_file_range, fd, (int)(offset >> 32),
> > -             (int)offset, (int)(nbytes >> 32), (int)nbytes, flags);
> > -
> > -/* mips */
> > -#elif defined(__mips__) && __WORDSIZE == 32
> > -#if __BYTE_ORDER == __BIG_ENDIAN
> > -     return ltp_syscall(__NR_sync_file_range, fd, 0, (int)(offset >> 32),
> > -             (int)offset, (int)(nbytes >> 32), (int)nbytes, flags);
> > -#elif __BYTE_ORDER == __LITTLE_ENDIAN
> > -     return ltp_syscall(__NR_sync_file_range, fd, 0, (int)offset,
> > -             (int)(offset >> 32), (int)nbytes, (int)(nbytes >> 32), flags);
> > -#endif
> > -
> > -/* other */
> > -#else
> > -     return ltp_syscall(__NR_sync_file_range, fd, offset, nbytes, flags);
> > -#endif
> > -}
> > -
> >  /******************************************************************************/
> >  /*                                                                       */
> >  /* Function:    main                                                   */
> > @@ -258,24 +217,13 @@ int main(int ac, char **av)
> >
> >       tst_parse_opts(ac, av, NULL, NULL);
> >
> > -#if defined(__powerpc__) || defined(__powerpc64__)   /* for PPC, kernel version > 2.6.21 needed */
> > -     if (tst_kvercmp(2, 16, 22) < 0) {
> > -             tst_brkm(TCONF, NULL,
> > -                      "System doesn't support execution of the test");
> > -     }
> > -#else
> > -     /* For other archs, need kernel version > 2.6.16 */
> > -
> > -     if (tst_kvercmp(2, 6, 17) < 0) {
> > -             tst_brkm(TCONF, NULL,
> > -                      "System doesn't support execution of the test");
> > -     }
> > -#endif
> > +     if (!check_sync_file_range())
> > +             tst_brkm(TCONF, NULL, "sync_file_range() not supported");
> >
> >       setup();
> >
> >       for (test_index = 0; test_index < TST_TOTAL; test_index++) {
> > -             TEST(syncfilerange
> > +             TEST(sync_file_range
> >                    (*(test_data[test_index].fd),
> >                     test_data[test_index].offset,
> >                     test_data[test_index].nbytes,
>
> Other than the minor things, this is great work.
>

Thanks,
-Sumit

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

[LTP] [PATCH v3 7/7] syscalls/sync_file_range: add sync device test-case

[Sumit Garg]

On Tue, 19 Feb 2019 at 19:51, Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > sync_file_range02 tests to sync file data range 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>
> > ---
> >  runtest/syscalls                                   |  1 +
> >  .../kernel/syscalls/sync_file_range/.gitignore     |  1 +
> >  .../syscalls/sync_file_range/sync_file_range02.c   | 70 ++++++++++++++++++++++
> >  3 files changed, 72 insertions(+)
> >  create mode 100644 testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
> >
> > diff --git a/runtest/syscalls b/runtest/syscalls
> > index aaad651..70d3561 100644
> > --- a/runtest/syscalls
> > +++ b/runtest/syscalls
> > @@ -1353,6 +1353,7 @@ syncfs01 syncfs01
> >
> >  #testcases for sync_file_range
> >  sync_file_range01 sync_file_range01
> > +sync_file_range02 sync_file_range02
> >
> >  syscall01 syscall01
> >
> > diff --git a/testcases/kernel/syscalls/sync_file_range/.gitignore b/testcases/kernel/syscalls/sync_file_range/.gitignore
> > index 3f6bd75..e6485f7 100644
> > --- a/testcases/kernel/syscalls/sync_file_range/.gitignore
> > +++ b/testcases/kernel/syscalls/sync_file_range/.gitignore
> > @@ -1 +1,2 @@
> >  /sync_file_range01
> > +/sync_file_range02
> > diff --git a/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
> > new file mode 100644
> > index 0000000..fb7a5f7
> > --- /dev/null
> > +++ b/testcases/kernel/syscalls/sync_file_range/sync_file_range02.c
> > @@ -0,0 +1,70 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * Copyright (c) 2019 Linaro Limited. All rights reserved.
> > + * Author: Sumit Garg <sumit.garg@linaro.org>
> > + */
> > +
> > +/*
> > + * sync_file_range02
> > + *
> > + * It basically tests sync_file_range() to sync test file range having large
> > + * dirty file pages to block device. Also, it tests all supported filesystems
> > + * on a test block device.
> > + */
> > +
> > +#define _GNU_SOURCE
> > +#include <errno.h>
> > +#include <stdlib.h>
> > +#include <stdio.h>
> > +#include <sys/types.h>
> > +#include "tst_sync_device.h"
> > +#include "tst_test.h"
> > +#include "lapi/sync_file_range.h"
> > +#include "check_sync_file_range.h"
> > +
> > +#define MNTPOINT             "mnt_point"
> > +#define TST_FILE             MNTPOINT"/test"
> > +#define TST_FILE_SIZE_MB     32
> > +#define SIZE_MB                      (1024*1024)
> > +
> > +static void verify_sync_file_range(void)
> > +{
> > +     int fd;
> > +
> > +     fd = tst_sync_device_write(TST_FILE, TST_FILE_SIZE_MB);
> > +
> > +     TEST(sync_file_range(fd, 0, TST_FILE_SIZE_MB * SIZE_MB,
> > +                          SYNC_FILE_RANGE_WAIT_BEFORE |
> > +                          SYNC_FILE_RANGE_WRITE |
> > +                          SYNC_FILE_RANGE_WAIT_AFTER));
> > +     if (TST_RET != 0)
> > +             tst_brk(TFAIL | TTERRNO, "sync_file_range() failed");
> > +
> > +     if (tst_sync_device_check(TST_FILE_SIZE_MB))
> > +             tst_res(TPASS, "Test file range synced to device");
> > +     else
> > +             tst_res(TFAIL, "Failed to sync test file range to device");
> > +}
> > +
> > +static void setup(void)
> > +{
> > +     tst_sync_device_init(tst_device->dev);
> > +}
> > +
> > +static void cleanup(void)
> > +{
> > +     if (!check_sync_file_range())
> > +             tst_brk(TCONF, "sync_file_range() not supported");
>
> This check is supposed to be in the test setup, right?
>

Ah, right. I will correct it.

-Sumit

> > +     tst_sync_device_cleanup();
> > +}
> > +
> > +static struct tst_test test = {
> > +     .needs_root = 1,
> > +     .mount_device = 1,
> > +     .all_filesystems = 1,
> > +     .mntpoint = MNTPOINT,
> > +     .setup = setup,
> > +     .cleanup = cleanup,
> > +     .test_all = verify_sync_file_range,
> > +};
> > --
> > 2.7.4
> >
>
> --
> Cyril Hrubis
> chrubis@suse.cz

[LTP] [PATCH v3 1/7] lib: Add library functions for sync related syscalls

[Cyril Hrubis]

Hi!
> > Maybe we just need a tst_dev_bytes_written() function that would return
> > the bytes written since the last call of the function so that we could
> > do:
> >
> >         fd = SAFE_OPEN(...);
> >
> >         tst_dev_blocks_written(tst_device.dev);
> >
> >         tst_fill_fd(fd, 0, TST_MB, size_mb);
> >
> >         TEST(fdsync(fd));
> >
> >         if (TST_RET)
> >                 tst_brk(TFAIL | TTERRNO, "syncfd() failed with %i", TST_RET);
> >
> >         written = tst_dev_blocks_written(tst_device.dev);
> >
> >         SAFE_CLOSE(fd);
> >
> >         if (written >= size_mb * TST_DEV_BLOCKS_IN_MB)
> >                 tst_res(TPASS, ...);
> >         else
> >                 tst_res(TFAIL, ...);
> >
> 
> Seems to be nicer approach. So rather than new file should I add
> tst_dev_bytes_written() function in "lib/tst_device.c" file?

Sounds good to me.

> > The test a bit longer, but the library functions are more reusable, if
> > you do 'git grep -B 1 SAFE_WRITE' you can see that the tst_fill_fd
> > function that loops over SAFE_WRITE could be used in a few places
> > already.
> >
> 
> I am not able to locate tst_fill_fd function. Are you referring to
> tst_fill_file function? If yes, then we could split that function to
> create tst_fill_fd function.

There is none, I was trying to state that there are several places that
use the same pattern and that such function would be generally useful.

And yes, buiding tst_fill_file on the top of tst_fill_fd sounds like a
good approach.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v3 1/7] lib: Add library functions for sync related syscalls

[Sumit Garg]

On Wed, 20 Feb 2019 at 17:34, Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > > Maybe we just need a tst_dev_bytes_written() function that would return
> > > the bytes written since the last call of the function so that we could
> > > do:
> > >
> > >         fd = SAFE_OPEN(...);
> > >
> > >         tst_dev_blocks_written(tst_device.dev);
> > >
> > >         tst_fill_fd(fd, 0, TST_MB, size_mb);
> > >
> > >         TEST(fdsync(fd));
> > >
> > >         if (TST_RET)
> > >                 tst_brk(TFAIL | TTERRNO, "syncfd() failed with %i", TST_RET);
> > >
> > >         written = tst_dev_blocks_written(tst_device.dev);
> > >
> > >         SAFE_CLOSE(fd);
> > >
> > >         if (written >= size_mb * TST_DEV_BLOCKS_IN_MB)
> > >                 tst_res(TPASS, ...);
> > >         else
> > >                 tst_res(TFAIL, ...);
> > >
> >
> > Seems to be nicer approach. So rather than new file should I add
> > tst_dev_bytes_written() function in "lib/tst_device.c" file?
>
> Sounds good to me.
>
> > > The test a bit longer, but the library functions are more reusable, if
> > > you do 'git grep -B 1 SAFE_WRITE' you can see that the tst_fill_fd
> > > function that loops over SAFE_WRITE could be used in a few places
> > > already.
> > >
> >
> > I am not able to locate tst_fill_fd function. Are you referring to
> > tst_fill_file function? If yes, then we could split that function to
> > create tst_fill_fd function.
>
> There is none, I was trying to state that there are several places that
> use the same pattern and that such function would be generally useful.
>

Ah, ok.

> And yes, buiding tst_fill_file on the top of tst_fill_fd sounds like a
> good approach.
>

Will incorporate in v4.

-Sumit

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