All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/6] Tests for readahead() and fadvise() on overlayfs
@ 2018-11-28 16:46 ` Amir Goldstein
  0 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-11-28 16:46 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp, Jan Stancek, Miklos Szeredi, linux-unionfs

Cyril,

The following series adds test covergae for readahead() syscall
over overlayfs file and adds test coverage for a specific
posix_fadvise() syscall advice (POSIX_FADV_WILLNEED).

So far, the posix_fadvise syscall tests in LTP only test for error
conditions, but not functionality.

The functionality of the advice POSIX_FADV_WILLNEED is identical
to readahead() and since kernel commit 3d8f7615319b ("vfs: implement
readahead(2) using POSIX_FADV_WILLNEED"), the implementations are
also bound together.

To add test coverage of the advice POSIX_FADV_WILLNEED, I decided not
to duplicate the readahead() functional test and add test cases to
readahead02 that use the POSIX_FADV_WILLNEED implementation.

Thanks,
Amir.

Changed since v3:
- Get rid of check_ret() helper
- Use tst_timer helpers instead of gettimeofday

Changes since v2:
- Runtime check for readahead/fadvise support
- Abort test case immediately if syscall fail

Changes since v1:
- Fix bugs in loop invocation of test (i.e. -i 2)
- Address review comments from Jan Stancek
- Make cached_max a global maximum over all test cases
- Improve reliability of overlayfs readahead test case failure

Amir Goldstein (6):
  syscalls/readahead02: Convert to newlib and cleanup
  syscalls/readahead02: abort test if readahead syscall fails
  syscalls/readahead02: fail test if readahead did not use any cache
  syscalls/readahead02: Convert to tst_timer helpers
  syscalls/readahead02: test readahead() on an overlayfs file
  syscalls/readahead02: test readahead using posix_fadvise()

 .../kernel/syscalls/readahead/readahead02.c   | 355 ++++++++----------
 1 file changed, 164 insertions(+), 191 deletions(-)

-- 
2.17.1

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [LTP] [PATCH v4 0/6] Tests for readahead() and fadvise() on overlayfs
@ 2018-11-28 16:46 ` Amir Goldstein
  0 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-11-28 16:46 UTC (permalink / raw)
  To: ltp

Cyril,

The following series adds test covergae for readahead() syscall
over overlayfs file and adds test coverage for a specific
posix_fadvise() syscall advice (POSIX_FADV_WILLNEED).

So far, the posix_fadvise syscall tests in LTP only test for error
conditions, but not functionality.

The functionality of the advice POSIX_FADV_WILLNEED is identical
to readahead() and since kernel commit 3d8f7615319b ("vfs: implement
readahead(2) using POSIX_FADV_WILLNEED"), the implementations are
also bound together.

To add test coverage of the advice POSIX_FADV_WILLNEED, I decided not
to duplicate the readahead() functional test and add test cases to
readahead02 that use the POSIX_FADV_WILLNEED implementation.

Thanks,
Amir.

Changed since v3:
- Get rid of check_ret() helper
- Use tst_timer helpers instead of gettimeofday

Changes since v2:
- Runtime check for readahead/fadvise support
- Abort test case immediately if syscall fail

Changes since v1:
- Fix bugs in loop invocation of test (i.e. -i 2)
- Address review comments from Jan Stancek
- Make cached_max a global maximum over all test cases
- Improve reliability of overlayfs readahead test case failure

Amir Goldstein (6):
  syscalls/readahead02: Convert to newlib and cleanup
  syscalls/readahead02: abort test if readahead syscall fails
  syscalls/readahead02: fail test if readahead did not use any cache
  syscalls/readahead02: Convert to tst_timer helpers
  syscalls/readahead02: test readahead() on an overlayfs file
  syscalls/readahead02: test readahead using posix_fadvise()

 .../kernel/syscalls/readahead/readahead02.c   | 355 ++++++++----------
 1 file changed, 164 insertions(+), 191 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH v4 1/6] syscalls/readahead02: Convert to newlib and cleanup
  2018-11-28 16:46 ` [LTP] " Amir Goldstein
@ 2018-11-28 16:46   ` Amir Goldstein
  -1 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-11-28 16:46 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp, Jan Stancek, Miklos Szeredi, linux-unionfs

* Use SAFE macros

* Use SPDX-License-Identifier

* No need to cleanup test file from temp dir

* No need to check if __NR_readahead is defined

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .../kernel/syscalls/readahead/readahead02.c   | 251 +++++-------------
 1 file changed, 71 insertions(+), 180 deletions(-)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index c9d92a6a4..956a1d5e5 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -1,25 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Copyright (C) 2012 Linux Test Project, Inc.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of version 2 of the GNU General Public
- * License as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * Further, this software is distributed without any warranty that it
- * is free of the rightful claim of any third person regarding
- * infringement or the like.  Any license provided herein, whether
- * implied or otherwise, applies only to this software file.  Patent
- * licenses, if any, provided herein do not apply to combinations of
- * this program with other software, or any other product whatsoever.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
- * 02110-1301, USA.
  */
 
 /*
@@ -43,86 +24,56 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include "config.h"
-#include "test.h"
-#include "safe_macros.h"
+#include "tst_test.h"
 #include "lapi/syscalls.h"
 
-char *TCID = "readahead02";
-int TST_TOTAL = 1;
-
-#if defined(__NR_readahead)
 static char testfile[PATH_MAX] = "testfile";
 static const char drop_caches_fname[] = "/proc/sys/vm/drop_caches";
 static const char meminfo_fname[] = "/proc/meminfo";
 static size_t testfile_size = 64 * 1024 * 1024;
-static int opt_fsize;
 static char *opt_fsizestr;
 static int pagesize;
-static const char *fs_type;
-static const char *device;
-static int mount_flag;
 
 #define MNTPOINT        "mntpoint"
 #define MIN_SANE_READAHEAD (4u * 1024u)
 
-option_t options[] = {
-	{"s:", &opt_fsize, &opt_fsizestr},
+static const char mntpoint[] = MNTPOINT;
+
+static struct tst_option options[] = {
+	{"s:", &opt_fsizestr, "-s    testfile size (default 64MB)"},
 	{NULL, NULL, NULL}
 };
 
-static void setup(void);
-static void cleanup(void);
-
-static void help(void)
-{
-	printf("  -s x    testfile size (default 64MB)\n");
-}
-
 static int check_ret(long expected_ret)
 {
-	if (expected_ret == TEST_RETURN) {
-		tst_resm(TPASS, "expected ret success - "
-			 "returned value = %ld", TEST_RETURN);
+	if (expected_ret == TST_RET) {
+		tst_res(TPASS, "expected ret success - "
+			"returned value = %ld", TST_RET);
 		return 0;
 	}
-	tst_resm(TFAIL | TTERRNO, "unexpected failure - "
-		 "returned value = %ld, expected: %ld",
-		 TEST_RETURN, expected_ret);
+	tst_res(TFAIL | TTERRNO, "unexpected failure - "
+		"returned value = %ld, expected: %ld",
+		TST_RET, expected_ret);
 	return 1;
 }
 
 static int has_file(const char *fname, int required)
 {
-	int ret;
 	struct stat buf;
-	ret = stat(fname, &buf);
-	if (ret == -1) {
-		if (errno == ENOENT)
-			if (required)
-				tst_brkm(TCONF, cleanup, "%s not available",
-					 fname);
-			else
-				return 0;
-		else
-			tst_brkm(TBROK | TERRNO, cleanup, "stat %s", fname);
+
+	if (stat(fname, &buf) == -1) {
+		if (errno != ENOENT)
+			tst_brk(TBROK | TERRNO, "stat %s", fname);
+		if (required)
+			tst_brk(TCONF, "%s not available", fname);
+		return 0;
 	}
 	return 1;
 }
 
 static void drop_caches(void)
 {
-	int ret;
-	FILE *f;
-
-	f = fopen(drop_caches_fname, "w");
-	if (f) {
-		ret = fprintf(f, "1");
-		fclose(f);
-		if (ret < 1)
-			tst_brkm(TBROK, cleanup, "Failed to drop caches");
-	} else {
-		tst_brkm(TBROK, cleanup, "Failed to open drop_caches");
-	}
+	SAFE_FILE_PRINTF(drop_caches_fname, "1");
 }
 
 static unsigned long parse_entry(const char *fname, const char *entry)
@@ -161,32 +112,21 @@ static unsigned long get_cached_size(void)
 
 static void create_testfile(void)
 {
-	FILE *f;
+	int fd;
 	char *tmp;
 	size_t i;
 
-	tst_resm(TINFO, "creating test file of size: %zu", testfile_size);
-	tmp = SAFE_MALLOC(cleanup, pagesize);
+	tst_res(TINFO, "creating test file of size: %zu", testfile_size);
+	tmp = SAFE_MALLOC(pagesize);
 
 	/* round to page size */
 	testfile_size = testfile_size & ~((long)pagesize - 1);
 
-	f = fopen(testfile, "w");
-	if (!f) {
-		free(tmp);
-		tst_brkm(TBROK | TERRNO, cleanup, "Failed to create %s",
-			 testfile);
-	}
-
+	fd = SAFE_CREAT(testfile, 0644);
 	for (i = 0; i < testfile_size; i += pagesize)
-		if (fwrite(tmp, pagesize, 1, f) < 1) {
-			free(tmp);
-			tst_brkm(TBROK, cleanup, "Failed to create %s",
-				 testfile);
-		}
-	fflush(f);
-	fsync(fileno(f));
-	fclose(f);
+		SAFE_WRITE(1, fd, tmp, pagesize);
+	SAFE_FSYNC(fd);
+	SAFE_CLOSE(fd);
 	free(tmp);
 }
 
@@ -215,13 +155,13 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 	off_t offset = 0;
 	struct timeval now;
 
-	fd = SAFE_OPEN(cleanup, fname, O_RDONLY);
+	fd = SAFE_OPEN(fname, O_RDONLY);
 
 	if (do_readahead) {
 		cached_start = get_cached_size();
 		do {
 			TEST(readahead(fd, offset, fsize - offset));
-			if (TEST_RETURN != 0) {
+			if (TST_RET != 0) {
 				check_ret(0);
 				break;
 			}
@@ -232,7 +172,7 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 				if (*cached > cached_start) {
 					max_ra_estimate = (1024 *
 						(*cached - cached_start));
-					tst_resm(TINFO, "max ra estimate: %lu",
+					tst_res(TINFO, "max ra estimate: %lu",
 						max_ra_estimate);
 				}
 				max_ra_estimate = MAX(max_ra_estimate,
@@ -242,27 +182,23 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 			i++;
 			offset += max_ra_estimate;
 		} while ((size_t)offset < fsize);
-		tst_resm(TINFO, "readahead calls made: %zu", i);
+		tst_res(TINFO, "readahead calls made: %zu", i);
 		*cached = get_cached_size();
 
 		/* offset of file shouldn't change after readahead */
-		offset = lseek(fd, 0, SEEK_CUR);
-		if (offset == (off_t) - 1)
-			tst_brkm(TBROK | TERRNO, cleanup, "lseek failed");
+		offset = SAFE_LSEEK(fd, 0, SEEK_CUR);
 		if (offset == 0)
-			tst_resm(TPASS, "offset is still at 0 as expected");
+			tst_res(TPASS, "offset is still at 0 as expected");
 		else
-			tst_resm(TFAIL, "offset has changed to: %lu", offset);
+			tst_res(TFAIL, "offset has changed to: %lu", offset);
 	}
 
 	if (gettimeofday(&now, NULL) == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "gettimeofday failed");
+		tst_brk(TBROK | TERRNO, "gettimeofday failed");
 	time_start_usec = now.tv_sec * 1000000 + now.tv_usec;
 	read_bytes_start = get_bytes_read();
 
-	p = mmap(NULL, fsize, PROT_READ, MAP_SHARED | MAP_POPULATE, fd, 0);
-	if (p == MAP_FAILED)
-		tst_brkm(TBROK | TERRNO, cleanup, "mmap failed");
+	p = SAFE_MMAP(NULL, fsize, PROT_READ, MAP_SHARED | MAP_POPULATE, fd, 0);
 
 	/* for old kernels, where MAP_POPULATE doesn't work, touch each page */
 	tmp = 0;
@@ -270,20 +206,20 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 		tmp = tmp ^ p[i];
 	/* prevent gcc from optimizing out loop above */
 	if (tmp != 0)
-		tst_brkm(TBROK, NULL, "This line should not be reached");
+		tst_brk(TBROK, "This line should not be reached");
 
 	if (!do_readahead)
 		*cached = get_cached_size();
 
-	SAFE_MUNMAP(cleanup, p, fsize);
+	SAFE_MUNMAP(p, fsize);
 
 	*read_bytes = get_bytes_read() - read_bytes_start;
 	if (gettimeofday(&now, NULL) == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "gettimeofday failed");
+		tst_brk(TBROK | TERRNO, "gettimeofday failed");
 	time_end_usec = now.tv_sec * 1000000 + now.tv_usec;
 	*usec = time_end_usec - time_start_usec;
 
-	SAFE_CLOSE(cleanup, fd);
+	SAFE_CLOSE(fd);
 }
 
 static void test_readahead(void)
@@ -302,7 +238,7 @@ static void test_readahead(void)
 	cached_low = get_cached_size();
 	cached_max = cached_max - cached_low;
 
-	tst_resm(TINFO, "read_testfile(0)");
+	tst_res(TINFO, "read_testfile(0)");
 	read_testfile(0, testfile, testfile_size, &read_bytes, &usec, &cached);
 	if (cached > cached_low)
 		cached = cached - cached_low;
@@ -312,7 +248,7 @@ static void test_readahead(void)
 	sync();
 	drop_caches();
 	cached_low = get_cached_size();
-	tst_resm(TINFO, "read_testfile(1)");
+	tst_res(TINFO, "read_testfile(1)");
 	read_testfile(1, testfile, testfile_size, &read_bytes_ra,
 		      &usec_ra, &cached_ra);
 	if (cached_ra > cached_low)
@@ -320,25 +256,25 @@ static void test_readahead(void)
 	else
 		cached_ra = 0;
 
-	tst_resm(TINFO, "read_testfile(0) took: %ld usec", usec);
-	tst_resm(TINFO, "read_testfile(1) took: %ld usec", usec_ra);
+	tst_res(TINFO, "read_testfile(0) took: %ld usec", usec);
+	tst_res(TINFO, "read_testfile(1) took: %ld usec", usec_ra);
 	if (has_file(proc_io_fname, 0)) {
-		tst_resm(TINFO, "read_testfile(0) read: %ld bytes", read_bytes);
-		tst_resm(TINFO, "read_testfile(1) read: %ld bytes",
-			 read_bytes_ra);
+		tst_res(TINFO, "read_testfile(0) read: %ld bytes", read_bytes);
+		tst_res(TINFO, "read_testfile(1) read: %ld bytes",
+			read_bytes_ra);
 		/* actual number of read bytes depends on total RAM */
 		if (read_bytes_ra < read_bytes)
-			tst_resm(TPASS, "readahead saved some I/O");
+			tst_res(TPASS, "readahead saved some I/O");
 		else
-			tst_resm(TFAIL, "readahead failed to save any I/O");
+			tst_res(TFAIL, "readahead failed to save any I/O");
 	} else {
-		tst_resm(TCONF, "Your system doesn't have /proc/pid/io,"
-			 " unable to determine read bytes during test");
+		tst_res(TCONF, "Your system doesn't have /proc/pid/io,"
+			" unable to determine read bytes during test");
 	}
 
-	tst_resm(TINFO, "cache can hold at least: %ld kB", cached_max);
-	tst_resm(TINFO, "read_testfile(0) used cache: %ld kB", cached);
-	tst_resm(TINFO, "read_testfile(1) used cache: %ld kB", cached_ra);
+	tst_res(TINFO, "cache can hold at least: %ld kB", cached_max);
+	tst_res(TINFO, "read_testfile(0) used cache: %ld kB", cached);
+	tst_res(TINFO, "read_testfile(1) used cache: %ld kB", cached_ra);
 
 	if (cached_max * 1024 >= testfile_size) {
 		/*
@@ -346,83 +282,38 @@ static void test_readahead(void)
 		 * for readahead should be at least testfile_size/2
 		 */
 		if (cached_ra * 1024 > testfile_size / 2)
-			tst_resm(TPASS, "using cache as expected");
+			tst_res(TPASS, "using cache as expected");
 		else
-			tst_resm(TWARN, "using less cache than expected");
+			tst_res(TWARN, "using less cache than expected");
 	} else {
-		tst_resm(TCONF, "Page cache on your system is too small "
-			 "to hold whole testfile.");
+		tst_res(TCONF, "Page cache on your system is too small "
+			"to hold whole testfile.");
 	}
 }
 
-int main(int argc, char *argv[])
-{
-	int lc;
-
-	tst_parse_opts(argc, argv, options, help);
-
-	if (opt_fsize)
-		testfile_size = atoi(opt_fsizestr);
-
-	setup();
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-		test_readahead();
-	}
-	cleanup();
-	tst_exit();
-}
-
 static void setup(void)
 {
-	tst_require_root();
-	tst_tmpdir();
-	TEST_PAUSE;
+	if (opt_fsizestr)
+		testfile_size = SAFE_STRTOL(opt_fsizestr, 1, INT_MAX);
 
 	has_file(drop_caches_fname, 1);
 	has_file(meminfo_fname, 1);
 
 	/* check if readahead is supported */
-	ltp_syscall(__NR_readahead, 0, 0, 0);
+	tst_syscall(__NR_readahead, 0, 0, 0);
 
 	pagesize = getpagesize();
 
-	if (tst_fs_type(cleanup, ".") == TST_TMPFS_MAGIC) {
-		tst_resm(TINFO, "TMPFS detected, creating loop device");
-
-		fs_type = tst_dev_fs_type();
-		device = tst_acquire_device(cleanup);
-		if (!device) {
-			tst_brkm(TCONF, cleanup,
-				"Failed to obtain block device");
-		}
-
-		tst_mkfs(cleanup, device, fs_type, NULL, NULL);
-
-		SAFE_MKDIR(cleanup, MNTPOINT, 0755);
-		SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
-		mount_flag = 1;
-
-		sprintf(testfile, MNTPOINT"/testfile");
-	}
+	sprintf(testfile, "%s/testfile", mntpoint);
 	create_testfile();
 }
 
-static void cleanup(void)
-{
-	unlink(testfile);
-	if (mount_flag && tst_umount(MNTPOINT) < 0)
-		tst_resm(TWARN | TERRNO, "umount device:%s failed", device);
-
-	if (device)
-		tst_release_device(device);
-
-	tst_rmdir();
-}
-
-#else /* __NR_readahead */
-int main(void)
-{
-	tst_brkm(TCONF, NULL, "System doesn't support __NR_readahead");
-}
-#endif
+static struct tst_test test = {
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.mntpoint = mntpoint,
+	.setup = setup,
+	.options = options,
+	.test_all = test_readahead,
+};
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [LTP] [PATCH v4 1/6] syscalls/readahead02: Convert to newlib and cleanup
@ 2018-11-28 16:46   ` Amir Goldstein
  0 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-11-28 16:46 UTC (permalink / raw)
  To: ltp

* Use SAFE macros

* Use SPDX-License-Identifier

* No need to cleanup test file from temp dir

* No need to check if __NR_readahead is defined

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .../kernel/syscalls/readahead/readahead02.c   | 251 +++++-------------
 1 file changed, 71 insertions(+), 180 deletions(-)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index c9d92a6a4..956a1d5e5 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -1,25 +1,6 @@
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Copyright (C) 2012 Linux Test Project, Inc.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of version 2 of the GNU General Public
- * License as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it would be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
- *
- * Further, this software is distributed without any warranty that it
- * is free of the rightful claim of any third person regarding
- * infringement or the like.  Any license provided herein, whether
- * implied or otherwise, applies only to this software file.  Patent
- * licenses, if any, provided herein do not apply to combinations of
- * this program with other software, or any other product whatsoever.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
- * 02110-1301, USA.
  */
 
 /*
@@ -43,86 +24,56 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include "config.h"
-#include "test.h"
-#include "safe_macros.h"
+#include "tst_test.h"
 #include "lapi/syscalls.h"
 
-char *TCID = "readahead02";
-int TST_TOTAL = 1;
-
-#if defined(__NR_readahead)
 static char testfile[PATH_MAX] = "testfile";
 static const char drop_caches_fname[] = "/proc/sys/vm/drop_caches";
 static const char meminfo_fname[] = "/proc/meminfo";
 static size_t testfile_size = 64 * 1024 * 1024;
-static int opt_fsize;
 static char *opt_fsizestr;
 static int pagesize;
-static const char *fs_type;
-static const char *device;
-static int mount_flag;
 
 #define MNTPOINT        "mntpoint"
 #define MIN_SANE_READAHEAD (4u * 1024u)
 
-option_t options[] = {
-	{"s:", &opt_fsize, &opt_fsizestr},
+static const char mntpoint[] = MNTPOINT;
+
+static struct tst_option options[] = {
+	{"s:", &opt_fsizestr, "-s    testfile size (default 64MB)"},
 	{NULL, NULL, NULL}
 };
 
-static void setup(void);
-static void cleanup(void);
-
-static void help(void)
-{
-	printf("  -s x    testfile size (default 64MB)\n");
-}
-
 static int check_ret(long expected_ret)
 {
-	if (expected_ret == TEST_RETURN) {
-		tst_resm(TPASS, "expected ret success - "
-			 "returned value = %ld", TEST_RETURN);
+	if (expected_ret == TST_RET) {
+		tst_res(TPASS, "expected ret success - "
+			"returned value = %ld", TST_RET);
 		return 0;
 	}
-	tst_resm(TFAIL | TTERRNO, "unexpected failure - "
-		 "returned value = %ld, expected: %ld",
-		 TEST_RETURN, expected_ret);
+	tst_res(TFAIL | TTERRNO, "unexpected failure - "
+		"returned value = %ld, expected: %ld",
+		TST_RET, expected_ret);
 	return 1;
 }
 
 static int has_file(const char *fname, int required)
 {
-	int ret;
 	struct stat buf;
-	ret = stat(fname, &buf);
-	if (ret == -1) {
-		if (errno == ENOENT)
-			if (required)
-				tst_brkm(TCONF, cleanup, "%s not available",
-					 fname);
-			else
-				return 0;
-		else
-			tst_brkm(TBROK | TERRNO, cleanup, "stat %s", fname);
+
+	if (stat(fname, &buf) == -1) {
+		if (errno != ENOENT)
+			tst_brk(TBROK | TERRNO, "stat %s", fname);
+		if (required)
+			tst_brk(TCONF, "%s not available", fname);
+		return 0;
 	}
 	return 1;
 }
 
 static void drop_caches(void)
 {
-	int ret;
-	FILE *f;
-
-	f = fopen(drop_caches_fname, "w");
-	if (f) {
-		ret = fprintf(f, "1");
-		fclose(f);
-		if (ret < 1)
-			tst_brkm(TBROK, cleanup, "Failed to drop caches");
-	} else {
-		tst_brkm(TBROK, cleanup, "Failed to open drop_caches");
-	}
+	SAFE_FILE_PRINTF(drop_caches_fname, "1");
 }
 
 static unsigned long parse_entry(const char *fname, const char *entry)
@@ -161,32 +112,21 @@ static unsigned long get_cached_size(void)
 
 static void create_testfile(void)
 {
-	FILE *f;
+	int fd;
 	char *tmp;
 	size_t i;
 
-	tst_resm(TINFO, "creating test file of size: %zu", testfile_size);
-	tmp = SAFE_MALLOC(cleanup, pagesize);
+	tst_res(TINFO, "creating test file of size: %zu", testfile_size);
+	tmp = SAFE_MALLOC(pagesize);
 
 	/* round to page size */
 	testfile_size = testfile_size & ~((long)pagesize - 1);
 
-	f = fopen(testfile, "w");
-	if (!f) {
-		free(tmp);
-		tst_brkm(TBROK | TERRNO, cleanup, "Failed to create %s",
-			 testfile);
-	}
-
+	fd = SAFE_CREAT(testfile, 0644);
 	for (i = 0; i < testfile_size; i += pagesize)
-		if (fwrite(tmp, pagesize, 1, f) < 1) {
-			free(tmp);
-			tst_brkm(TBROK, cleanup, "Failed to create %s",
-				 testfile);
-		}
-	fflush(f);
-	fsync(fileno(f));
-	fclose(f);
+		SAFE_WRITE(1, fd, tmp, pagesize);
+	SAFE_FSYNC(fd);
+	SAFE_CLOSE(fd);
 	free(tmp);
 }
 
@@ -215,13 +155,13 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 	off_t offset = 0;
 	struct timeval now;
 
-	fd = SAFE_OPEN(cleanup, fname, O_RDONLY);
+	fd = SAFE_OPEN(fname, O_RDONLY);
 
 	if (do_readahead) {
 		cached_start = get_cached_size();
 		do {
 			TEST(readahead(fd, offset, fsize - offset));
-			if (TEST_RETURN != 0) {
+			if (TST_RET != 0) {
 				check_ret(0);
 				break;
 			}
@@ -232,7 +172,7 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 				if (*cached > cached_start) {
 					max_ra_estimate = (1024 *
 						(*cached - cached_start));
-					tst_resm(TINFO, "max ra estimate: %lu",
+					tst_res(TINFO, "max ra estimate: %lu",
 						max_ra_estimate);
 				}
 				max_ra_estimate = MAX(max_ra_estimate,
@@ -242,27 +182,23 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 			i++;
 			offset += max_ra_estimate;
 		} while ((size_t)offset < fsize);
-		tst_resm(TINFO, "readahead calls made: %zu", i);
+		tst_res(TINFO, "readahead calls made: %zu", i);
 		*cached = get_cached_size();
 
 		/* offset of file shouldn't change after readahead */
-		offset = lseek(fd, 0, SEEK_CUR);
-		if (offset == (off_t) - 1)
-			tst_brkm(TBROK | TERRNO, cleanup, "lseek failed");
+		offset = SAFE_LSEEK(fd, 0, SEEK_CUR);
 		if (offset == 0)
-			tst_resm(TPASS, "offset is still at 0 as expected");
+			tst_res(TPASS, "offset is still@0 as expected");
 		else
-			tst_resm(TFAIL, "offset has changed to: %lu", offset);
+			tst_res(TFAIL, "offset has changed to: %lu", offset);
 	}
 
 	if (gettimeofday(&now, NULL) == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "gettimeofday failed");
+		tst_brk(TBROK | TERRNO, "gettimeofday failed");
 	time_start_usec = now.tv_sec * 1000000 + now.tv_usec;
 	read_bytes_start = get_bytes_read();
 
-	p = mmap(NULL, fsize, PROT_READ, MAP_SHARED | MAP_POPULATE, fd, 0);
-	if (p == MAP_FAILED)
-		tst_brkm(TBROK | TERRNO, cleanup, "mmap failed");
+	p = SAFE_MMAP(NULL, fsize, PROT_READ, MAP_SHARED | MAP_POPULATE, fd, 0);
 
 	/* for old kernels, where MAP_POPULATE doesn't work, touch each page */
 	tmp = 0;
@@ -270,20 +206,20 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 		tmp = tmp ^ p[i];
 	/* prevent gcc from optimizing out loop above */
 	if (tmp != 0)
-		tst_brkm(TBROK, NULL, "This line should not be reached");
+		tst_brk(TBROK, "This line should not be reached");
 
 	if (!do_readahead)
 		*cached = get_cached_size();
 
-	SAFE_MUNMAP(cleanup, p, fsize);
+	SAFE_MUNMAP(p, fsize);
 
 	*read_bytes = get_bytes_read() - read_bytes_start;
 	if (gettimeofday(&now, NULL) == -1)
-		tst_brkm(TBROK | TERRNO, cleanup, "gettimeofday failed");
+		tst_brk(TBROK | TERRNO, "gettimeofday failed");
 	time_end_usec = now.tv_sec * 1000000 + now.tv_usec;
 	*usec = time_end_usec - time_start_usec;
 
-	SAFE_CLOSE(cleanup, fd);
+	SAFE_CLOSE(fd);
 }
 
 static void test_readahead(void)
@@ -302,7 +238,7 @@ static void test_readahead(void)
 	cached_low = get_cached_size();
 	cached_max = cached_max - cached_low;
 
-	tst_resm(TINFO, "read_testfile(0)");
+	tst_res(TINFO, "read_testfile(0)");
 	read_testfile(0, testfile, testfile_size, &read_bytes, &usec, &cached);
 	if (cached > cached_low)
 		cached = cached - cached_low;
@@ -312,7 +248,7 @@ static void test_readahead(void)
 	sync();
 	drop_caches();
 	cached_low = get_cached_size();
-	tst_resm(TINFO, "read_testfile(1)");
+	tst_res(TINFO, "read_testfile(1)");
 	read_testfile(1, testfile, testfile_size, &read_bytes_ra,
 		      &usec_ra, &cached_ra);
 	if (cached_ra > cached_low)
@@ -320,25 +256,25 @@ static void test_readahead(void)
 	else
 		cached_ra = 0;
 
-	tst_resm(TINFO, "read_testfile(0) took: %ld usec", usec);
-	tst_resm(TINFO, "read_testfile(1) took: %ld usec", usec_ra);
+	tst_res(TINFO, "read_testfile(0) took: %ld usec", usec);
+	tst_res(TINFO, "read_testfile(1) took: %ld usec", usec_ra);
 	if (has_file(proc_io_fname, 0)) {
-		tst_resm(TINFO, "read_testfile(0) read: %ld bytes", read_bytes);
-		tst_resm(TINFO, "read_testfile(1) read: %ld bytes",
-			 read_bytes_ra);
+		tst_res(TINFO, "read_testfile(0) read: %ld bytes", read_bytes);
+		tst_res(TINFO, "read_testfile(1) read: %ld bytes",
+			read_bytes_ra);
 		/* actual number of read bytes depends on total RAM */
 		if (read_bytes_ra < read_bytes)
-			tst_resm(TPASS, "readahead saved some I/O");
+			tst_res(TPASS, "readahead saved some I/O");
 		else
-			tst_resm(TFAIL, "readahead failed to save any I/O");
+			tst_res(TFAIL, "readahead failed to save any I/O");
 	} else {
-		tst_resm(TCONF, "Your system doesn't have /proc/pid/io,"
-			 " unable to determine read bytes during test");
+		tst_res(TCONF, "Your system doesn't have /proc/pid/io,"
+			" unable to determine read bytes during test");
 	}
 
-	tst_resm(TINFO, "cache can hold at least: %ld kB", cached_max);
-	tst_resm(TINFO, "read_testfile(0) used cache: %ld kB", cached);
-	tst_resm(TINFO, "read_testfile(1) used cache: %ld kB", cached_ra);
+	tst_res(TINFO, "cache can hold@least: %ld kB", cached_max);
+	tst_res(TINFO, "read_testfile(0) used cache: %ld kB", cached);
+	tst_res(TINFO, "read_testfile(1) used cache: %ld kB", cached_ra);
 
 	if (cached_max * 1024 >= testfile_size) {
 		/*
@@ -346,83 +282,38 @@ static void test_readahead(void)
 		 * for readahead should be at least testfile_size/2
 		 */
 		if (cached_ra * 1024 > testfile_size / 2)
-			tst_resm(TPASS, "using cache as expected");
+			tst_res(TPASS, "using cache as expected");
 		else
-			tst_resm(TWARN, "using less cache than expected");
+			tst_res(TWARN, "using less cache than expected");
 	} else {
-		tst_resm(TCONF, "Page cache on your system is too small "
-			 "to hold whole testfile.");
+		tst_res(TCONF, "Page cache on your system is too small "
+			"to hold whole testfile.");
 	}
 }
 
-int main(int argc, char *argv[])
-{
-	int lc;
-
-	tst_parse_opts(argc, argv, options, help);
-
-	if (opt_fsize)
-		testfile_size = atoi(opt_fsizestr);
-
-	setup();
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-		test_readahead();
-	}
-	cleanup();
-	tst_exit();
-}
-
 static void setup(void)
 {
-	tst_require_root();
-	tst_tmpdir();
-	TEST_PAUSE;
+	if (opt_fsizestr)
+		testfile_size = SAFE_STRTOL(opt_fsizestr, 1, INT_MAX);
 
 	has_file(drop_caches_fname, 1);
 	has_file(meminfo_fname, 1);
 
 	/* check if readahead is supported */
-	ltp_syscall(__NR_readahead, 0, 0, 0);
+	tst_syscall(__NR_readahead, 0, 0, 0);
 
 	pagesize = getpagesize();
 
-	if (tst_fs_type(cleanup, ".") == TST_TMPFS_MAGIC) {
-		tst_resm(TINFO, "TMPFS detected, creating loop device");
-
-		fs_type = tst_dev_fs_type();
-		device = tst_acquire_device(cleanup);
-		if (!device) {
-			tst_brkm(TCONF, cleanup,
-				"Failed to obtain block device");
-		}
-
-		tst_mkfs(cleanup, device, fs_type, NULL, NULL);
-
-		SAFE_MKDIR(cleanup, MNTPOINT, 0755);
-		SAFE_MOUNT(cleanup, device, MNTPOINT, fs_type, 0, NULL);
-		mount_flag = 1;
-
-		sprintf(testfile, MNTPOINT"/testfile");
-	}
+	sprintf(testfile, "%s/testfile", mntpoint);
 	create_testfile();
 }
 
-static void cleanup(void)
-{
-	unlink(testfile);
-	if (mount_flag && tst_umount(MNTPOINT) < 0)
-		tst_resm(TWARN | TERRNO, "umount device:%s failed", device);
-
-	if (device)
-		tst_release_device(device);
-
-	tst_rmdir();
-}
-
-#else /* __NR_readahead */
-int main(void)
-{
-	tst_brkm(TCONF, NULL, "System doesn't support __NR_readahead");
-}
-#endif
+static struct tst_test test = {
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.mntpoint = mntpoint,
+	.setup = setup,
+	.options = options,
+	.test_all = test_readahead,
+};
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v4 2/6] syscalls/readahead02: abort test if readahead syscall fails
  2018-11-28 16:46 ` [LTP] " Amir Goldstein
@ 2018-11-28 16:46   ` Amir Goldstein
  -1 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-11-28 16:46 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp, Jan Stancek, Miklos Szeredi, linux-unionfs

There is no reason to continue the test if readahead syscall fails
and we can also check and report TCONF if filesystem does not support
readahead.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .../kernel/syscalls/readahead/readahead02.c   | 27 +++++++++----------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 956a1d5e5..88eb5fbff 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -44,19 +44,6 @@ static struct tst_option options[] = {
 	{NULL, NULL, NULL}
 };
 
-static int check_ret(long expected_ret)
-{
-	if (expected_ret == TST_RET) {
-		tst_res(TPASS, "expected ret success - "
-			"returned value = %ld", TST_RET);
-		return 0;
-	}
-	tst_res(TFAIL | TTERRNO, "unexpected failure - "
-		"returned value = %ld, expected: %ld",
-		TST_RET, expected_ret);
-	return 1;
-}
-
 static int has_file(const char *fname, int required)
 {
 	struct stat buf;
@@ -162,8 +149,8 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 		do {
 			TEST(readahead(fd, offset, fsize - offset));
 			if (TST_RET != 0) {
-				check_ret(0);
-				break;
+				SAFE_CLOSE(fd);
+				return;
 			}
 
 			/* estimate max readahead size based on first call */
@@ -251,6 +238,16 @@ static void test_readahead(void)
 	tst_res(TINFO, "read_testfile(1)");
 	read_testfile(1, testfile, testfile_size, &read_bytes_ra,
 		      &usec_ra, &cached_ra);
+	if (TST_RET != 0) {
+		if (TST_ERR == EINVAL) {
+			tst_res(TCONF, "readahead not supported on %s",
+				tst_device->fs_type);
+		} else {
+			tst_res(TFAIL | TTERRNO, "readahead failed on %s",
+				tst_device->fs_type);
+		}
+		return;
+	}
 	if (cached_ra > cached_low)
 		cached_ra = cached_ra - cached_low;
 	else
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [LTP] [PATCH v4 2/6] syscalls/readahead02: abort test if readahead syscall fails
@ 2018-11-28 16:46   ` Amir Goldstein
  0 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-11-28 16:46 UTC (permalink / raw)
  To: ltp

There is no reason to continue the test if readahead syscall fails
and we can also check and report TCONF if filesystem does not support
readahead.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .../kernel/syscalls/readahead/readahead02.c   | 27 +++++++++----------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 956a1d5e5..88eb5fbff 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -44,19 +44,6 @@ static struct tst_option options[] = {
 	{NULL, NULL, NULL}
 };
 
-static int check_ret(long expected_ret)
-{
-	if (expected_ret == TST_RET) {
-		tst_res(TPASS, "expected ret success - "
-			"returned value = %ld", TST_RET);
-		return 0;
-	}
-	tst_res(TFAIL | TTERRNO, "unexpected failure - "
-		"returned value = %ld, expected: %ld",
-		TST_RET, expected_ret);
-	return 1;
-}
-
 static int has_file(const char *fname, int required)
 {
 	struct stat buf;
@@ -162,8 +149,8 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 		do {
 			TEST(readahead(fd, offset, fsize - offset));
 			if (TST_RET != 0) {
-				check_ret(0);
-				break;
+				SAFE_CLOSE(fd);
+				return;
 			}
 
 			/* estimate max readahead size based on first call */
@@ -251,6 +238,16 @@ static void test_readahead(void)
 	tst_res(TINFO, "read_testfile(1)");
 	read_testfile(1, testfile, testfile_size, &read_bytes_ra,
 		      &usec_ra, &cached_ra);
+	if (TST_RET != 0) {
+		if (TST_ERR == EINVAL) {
+			tst_res(TCONF, "readahead not supported on %s",
+				tst_device->fs_type);
+		} else {
+			tst_res(TFAIL | TTERRNO, "readahead failed on %s",
+				tst_device->fs_type);
+		}
+		return;
+	}
 	if (cached_ra > cached_low)
 		cached_ra = cached_ra - cached_low;
 	else
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v4 3/6] syscalls/readahead02: fail test if readahead did not use any cache
  2018-11-28 16:46 ` [LTP] " Amir Goldstein
@ 2018-11-28 16:46   ` Amir Goldstein
  -1 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-11-28 16:46 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp, Jan Stancek, Miklos Szeredi, linux-unionfs

The heuristic of failing the test only if not saving any io has false
negatives with overlayfs readahead() bug, but readahead() with overlayfs
always warns on "using less cache then expected", when actually, it is
using no cache at all.

Add another condition to fail the test if readahead did not use any
cache at all, which always detected the overlayfs bug.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Acked-by: Jan Stancek <jstancek@redhat.com>
---
 testcases/kernel/syscalls/readahead/readahead02.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 88eb5fbff..0acdad482 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -280,6 +280,8 @@ static void test_readahead(void)
 		 */
 		if (cached_ra * 1024 > testfile_size / 2)
 			tst_res(TPASS, "using cache as expected");
+		else if (!cached_ra)
+			tst_res(TFAIL, "readahead failed to use any cache");
 		else
 			tst_res(TWARN, "using less cache than expected");
 	} else {
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [LTP] [PATCH v4 3/6] syscalls/readahead02: fail test if readahead did not use any cache
@ 2018-11-28 16:46   ` Amir Goldstein
  0 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-11-28 16:46 UTC (permalink / raw)
  To: ltp

The heuristic of failing the test only if not saving any io has false
negatives with overlayfs readahead() bug, but readahead() with overlayfs
always warns on "using less cache then expected", when actually, it is
using no cache at all.

Add another condition to fail the test if readahead did not use any
cache at all, which always detected the overlayfs bug.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Acked-by: Jan Stancek <jstancek@redhat.com>
---
 testcases/kernel/syscalls/readahead/readahead02.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 88eb5fbff..0acdad482 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -280,6 +280,8 @@ static void test_readahead(void)
 		 */
 		if (cached_ra * 1024 > testfile_size / 2)
 			tst_res(TPASS, "using cache as expected");
+		else if (!cached_ra)
+			tst_res(TFAIL, "readahead failed to use any cache");
 		else
 			tst_res(TWARN, "using less cache than expected");
 	} else {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v4 4/6] syscalls/readahead02: Convert to tst_timer helpers
  2018-11-28 16:46 ` [LTP] " Amir Goldstein
@ 2018-11-28 16:46   ` Amir Goldstein
  -1 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-11-28 16:46 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp, Jan Stancek, Miklos Szeredi, linux-unionfs

To avoid usinf wall clock and get rid of gettimeofday boiler plate code.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .../kernel/syscalls/readahead/readahead02.c   | 22 +++++++------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 0acdad482..24f045900 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -16,7 +16,6 @@
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <sys/types.h>
-#include <sys/time.h>
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -25,6 +24,7 @@
 #include <fcntl.h>
 #include "config.h"
 #include "tst_test.h"
+#include "tst_timer.h"
 #include "lapi/syscalls.h"
 
 static char testfile[PATH_MAX] = "testfile";
@@ -130,17 +130,15 @@ static void create_testfile(void)
  * @cached: returns cached kB from /proc/meminfo
  */
 static void read_testfile(int do_readahead, const char *fname, size_t fsize,
-			  unsigned long *read_bytes, long *usec,
+			  unsigned long *read_bytes, long long *usec,
 			  unsigned long *cached)
 {
 	int fd;
 	size_t i = 0;
 	long read_bytes_start;
 	unsigned char *p, tmp;
-	unsigned long time_start_usec, time_end_usec;
 	unsigned long cached_start, max_ra_estimate = 0;
 	off_t offset = 0;
-	struct timeval now;
 
 	fd = SAFE_OPEN(fname, O_RDONLY);
 
@@ -180,9 +178,7 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 			tst_res(TFAIL, "offset has changed to: %lu", offset);
 	}
 
-	if (gettimeofday(&now, NULL) == -1)
-		tst_brk(TBROK | TERRNO, "gettimeofday failed");
-	time_start_usec = now.tv_sec * 1000000 + now.tv_usec;
+	tst_timer_start(CLOCK_MONOTONIC);
 	read_bytes_start = get_bytes_read();
 
 	p = SAFE_MMAP(NULL, fsize, PROT_READ, MAP_SHARED | MAP_POPULATE, fd, 0);
@@ -201,10 +197,8 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 	SAFE_MUNMAP(p, fsize);
 
 	*read_bytes = get_bytes_read() - read_bytes_start;
-	if (gettimeofday(&now, NULL) == -1)
-		tst_brk(TBROK | TERRNO, "gettimeofday failed");
-	time_end_usec = now.tv_sec * 1000000 + now.tv_usec;
-	*usec = time_end_usec - time_start_usec;
+	tst_timer_stop();
+	*usec = tst_timer_elapsed_us();
 
 	SAFE_CLOSE(fd);
 }
@@ -212,7 +206,7 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 static void test_readahead(void)
 {
 	unsigned long read_bytes, read_bytes_ra;
-	long usec, usec_ra;
+	long long usec, usec_ra;
 	unsigned long cached_max, cached_low, cached, cached_ra;
 	char proc_io_fname[128];
 	sprintf(proc_io_fname, "/proc/%u/io", getpid());
@@ -253,8 +247,8 @@ static void test_readahead(void)
 	else
 		cached_ra = 0;
 
-	tst_res(TINFO, "read_testfile(0) took: %ld usec", usec);
-	tst_res(TINFO, "read_testfile(1) took: %ld usec", usec_ra);
+	tst_res(TINFO, "read_testfile(0) took: %lli usec", usec);
+	tst_res(TINFO, "read_testfile(1) took: %lli usec", usec_ra);
 	if (has_file(proc_io_fname, 0)) {
 		tst_res(TINFO, "read_testfile(0) read: %ld bytes", read_bytes);
 		tst_res(TINFO, "read_testfile(1) read: %ld bytes",
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [LTP] [PATCH v4 4/6] syscalls/readahead02: Convert to tst_timer helpers
@ 2018-11-28 16:46   ` Amir Goldstein
  0 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-11-28 16:46 UTC (permalink / raw)
  To: ltp

To avoid usinf wall clock and get rid of gettimeofday boiler plate code.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .../kernel/syscalls/readahead/readahead02.c   | 22 +++++++------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 0acdad482..24f045900 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -16,7 +16,6 @@
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <sys/types.h>
-#include <sys/time.h>
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -25,6 +24,7 @@
 #include <fcntl.h>
 #include "config.h"
 #include "tst_test.h"
+#include "tst_timer.h"
 #include "lapi/syscalls.h"
 
 static char testfile[PATH_MAX] = "testfile";
@@ -130,17 +130,15 @@ static void create_testfile(void)
  * @cached: returns cached kB from /proc/meminfo
  */
 static void read_testfile(int do_readahead, const char *fname, size_t fsize,
-			  unsigned long *read_bytes, long *usec,
+			  unsigned long *read_bytes, long long *usec,
 			  unsigned long *cached)
 {
 	int fd;
 	size_t i = 0;
 	long read_bytes_start;
 	unsigned char *p, tmp;
-	unsigned long time_start_usec, time_end_usec;
 	unsigned long cached_start, max_ra_estimate = 0;
 	off_t offset = 0;
-	struct timeval now;
 
 	fd = SAFE_OPEN(fname, O_RDONLY);
 
@@ -180,9 +178,7 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 			tst_res(TFAIL, "offset has changed to: %lu", offset);
 	}
 
-	if (gettimeofday(&now, NULL) == -1)
-		tst_brk(TBROK | TERRNO, "gettimeofday failed");
-	time_start_usec = now.tv_sec * 1000000 + now.tv_usec;
+	tst_timer_start(CLOCK_MONOTONIC);
 	read_bytes_start = get_bytes_read();
 
 	p = SAFE_MMAP(NULL, fsize, PROT_READ, MAP_SHARED | MAP_POPULATE, fd, 0);
@@ -201,10 +197,8 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 	SAFE_MUNMAP(p, fsize);
 
 	*read_bytes = get_bytes_read() - read_bytes_start;
-	if (gettimeofday(&now, NULL) == -1)
-		tst_brk(TBROK | TERRNO, "gettimeofday failed");
-	time_end_usec = now.tv_sec * 1000000 + now.tv_usec;
-	*usec = time_end_usec - time_start_usec;
+	tst_timer_stop();
+	*usec = tst_timer_elapsed_us();
 
 	SAFE_CLOSE(fd);
 }
@@ -212,7 +206,7 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 static void test_readahead(void)
 {
 	unsigned long read_bytes, read_bytes_ra;
-	long usec, usec_ra;
+	long long usec, usec_ra;
 	unsigned long cached_max, cached_low, cached, cached_ra;
 	char proc_io_fname[128];
 	sprintf(proc_io_fname, "/proc/%u/io", getpid());
@@ -253,8 +247,8 @@ static void test_readahead(void)
 	else
 		cached_ra = 0;
 
-	tst_res(TINFO, "read_testfile(0) took: %ld usec", usec);
-	tst_res(TINFO, "read_testfile(1) took: %ld usec", usec_ra);
+	tst_res(TINFO, "read_testfile(0) took: %lli usec", usec);
+	tst_res(TINFO, "read_testfile(1) took: %lli usec", usec_ra);
 	if (has_file(proc_io_fname, 0)) {
 		tst_res(TINFO, "read_testfile(0) read: %ld bytes", read_bytes);
 		tst_res(TINFO, "read_testfile(1) read: %ld bytes",
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v4 5/6] syscalls/readahead02: test readahead() on an overlayfs file
  2018-11-28 16:46 ` [LTP] " Amir Goldstein
@ 2018-11-28 16:46   ` Amir Goldstein
  -1 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-11-28 16:46 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp, Jan Stancek, Miklos Szeredi, linux-unionfs

Repeat the test case on an overlayfs file.

The new test case is a regression test for kernel commit b833a3660394
("ovl: add ovl_fadvise()") which fixes a regression of readahead() on
an overlay file that was introduced by kernel commit 5b910bd615ba
("ovl: fix GPF in swapfile_activate of file from overlayfs over xfs").

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .../kernel/syscalls/readahead/readahead02.c   | 80 ++++++++++++++++---
 1 file changed, 71 insertions(+), 9 deletions(-)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 24f045900..73c8a6ce6 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -14,6 +14,7 @@
 #include <sys/types.h>
 #include <sys/syscall.h>
 #include <sys/mman.h>
+#include <sys/mount.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <errno.h>
@@ -33,8 +34,14 @@ static const char meminfo_fname[] = "/proc/meminfo";
 static size_t testfile_size = 64 * 1024 * 1024;
 static char *opt_fsizestr;
 static int pagesize;
+static unsigned long cached_max;
+static int ovl_mounted;
 
 #define MNTPOINT        "mntpoint"
+#define OVL_LOWER	MNTPOINT"/lower"
+#define OVL_UPPER	MNTPOINT"/upper"
+#define OVL_WORK	MNTPOINT"/work"
+#define OVL_MNT		MNTPOINT"/ovl"
 #define MIN_SANE_READAHEAD (4u * 1024u)
 
 static const char mntpoint[] = MNTPOINT;
@@ -44,6 +51,16 @@ static struct tst_option options[] = {
 	{NULL, NULL, NULL}
 };
 
+static struct tcase {
+	const char *tname;
+	int use_overlay;
+} tcases[] = {
+	{ "readahead on file", 0 },
+	{ "readahead on overlayfs file", 1 },
+};
+
+static int readahead_supported = 1;
+
 static int has_file(const char *fname, int required)
 {
 	struct stat buf;
@@ -97,12 +114,13 @@ static unsigned long get_cached_size(void)
 	return parse_entry(meminfo_fname, entry);
 }
 
-static void create_testfile(void)
+static void create_testfile(int use_overlay)
 {
 	int fd;
 	char *tmp;
 	size_t i;
 
+	sprintf(testfile, "%s/testfile", use_overlay ? OVL_MNT : MNTPOINT);
 	tst_res(TINFO, "creating test file of size: %zu", testfile_size);
 	tmp = SAFE_MALLOC(pagesize);
 
@@ -203,21 +221,33 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 	SAFE_CLOSE(fd);
 }
 
-static void test_readahead(void)
+static void test_readahead(unsigned int n)
 {
 	unsigned long read_bytes, read_bytes_ra;
 	long long usec, usec_ra;
-	unsigned long cached_max, cached_low, cached, cached_ra;
+	unsigned long cached_high, cached_low, cached, cached_ra;
 	char proc_io_fname[128];
+	struct tcase *tc = &tcases[n];
+
+	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
+
 	sprintf(proc_io_fname, "/proc/%u/io", getpid());
 
+	if (tc->use_overlay && !ovl_mounted) {
+		tst_res(TCONF,
+			"overlayfs is not configured in this kernel.");
+		return;
+	}
+
+	create_testfile(tc->use_overlay);
+
 	/* find out how much can cache hold if we read whole file */
 	read_testfile(0, testfile, testfile_size, &read_bytes, &usec, &cached);
-	cached_max = get_cached_size();
+	cached_high = get_cached_size();
 	sync();
 	drop_caches();
 	cached_low = get_cached_size();
-	cached_max = cached_max - cached_low;
+	cached_max = MAX(cached_max, cached_high - cached_low);
 
 	tst_res(TINFO, "read_testfile(0)");
 	read_testfile(0, testfile, testfile_size, &read_bytes, &usec, &cached);
@@ -233,11 +263,14 @@ static void test_readahead(void)
 	read_testfile(1, testfile, testfile_size, &read_bytes_ra,
 		      &usec_ra, &cached_ra);
 	if (TST_RET != 0) {
-		if (TST_ERR == EINVAL) {
+		if (TST_ERR == EINVAL &&
+		    (!tc->use_overlay || !readahead_supported)) {
+			readahead_supported = 0;
 			tst_res(TCONF, "readahead not supported on %s",
 				tst_device->fs_type);
 		} else {
 			tst_res(TFAIL | TTERRNO, "readahead failed on %s",
+				tc->use_overlay ? "overlayfs" :
 				tst_device->fs_type);
 		}
 		return;
@@ -284,6 +317,28 @@ static void test_readahead(void)
 	}
 }
 
+static void setup_overlay(void)
+{
+	int ret;
+
+	/* Setup an overlay mount with lower dir and file */
+	SAFE_MKDIR(OVL_LOWER, 0755);
+	SAFE_MKDIR(OVL_UPPER, 0755);
+	SAFE_MKDIR(OVL_WORK, 0755);
+	SAFE_MKDIR(OVL_MNT, 0755);
+	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
+		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
+	if (ret < 0) {
+		if (errno == ENODEV) {
+			tst_res(TINFO,
+				"overlayfs is not configured in this kernel.");
+			return;
+		}
+		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
+	}
+	ovl_mounted = 1;
+}
+
 static void setup(void)
 {
 	if (opt_fsizestr)
@@ -297,8 +352,13 @@ static void setup(void)
 
 	pagesize = getpagesize();
 
-	sprintf(testfile, "%s/testfile", mntpoint);
-	create_testfile();
+	setup_overlay();
+}
+
+static void cleanup(void)
+{
+	if (ovl_mounted)
+		SAFE_UMOUNT(OVL_MNT);
 }
 
 static struct tst_test test = {
@@ -307,6 +367,8 @@ static struct tst_test test = {
 	.mount_device = 1,
 	.mntpoint = mntpoint,
 	.setup = setup,
+	.cleanup = cleanup,
 	.options = options,
-	.test_all = test_readahead,
+	.test = test_readahead,
+	.tcnt = ARRAY_SIZE(tcases),
 };
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [LTP] [PATCH v4 5/6] syscalls/readahead02: test readahead() on an overlayfs file
@ 2018-11-28 16:46   ` Amir Goldstein
  0 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-11-28 16:46 UTC (permalink / raw)
  To: ltp

Repeat the test case on an overlayfs file.

The new test case is a regression test for kernel commit b833a3660394
("ovl: add ovl_fadvise()") which fixes a regression of readahead() on
an overlay file that was introduced by kernel commit 5b910bd615ba
("ovl: fix GPF in swapfile_activate of file from overlayfs over xfs").

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .../kernel/syscalls/readahead/readahead02.c   | 80 ++++++++++++++++---
 1 file changed, 71 insertions(+), 9 deletions(-)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 24f045900..73c8a6ce6 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -14,6 +14,7 @@
 #include <sys/types.h>
 #include <sys/syscall.h>
 #include <sys/mman.h>
+#include <sys/mount.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <errno.h>
@@ -33,8 +34,14 @@ static const char meminfo_fname[] = "/proc/meminfo";
 static size_t testfile_size = 64 * 1024 * 1024;
 static char *opt_fsizestr;
 static int pagesize;
+static unsigned long cached_max;
+static int ovl_mounted;
 
 #define MNTPOINT        "mntpoint"
+#define OVL_LOWER	MNTPOINT"/lower"
+#define OVL_UPPER	MNTPOINT"/upper"
+#define OVL_WORK	MNTPOINT"/work"
+#define OVL_MNT		MNTPOINT"/ovl"
 #define MIN_SANE_READAHEAD (4u * 1024u)
 
 static const char mntpoint[] = MNTPOINT;
@@ -44,6 +51,16 @@ static struct tst_option options[] = {
 	{NULL, NULL, NULL}
 };
 
+static struct tcase {
+	const char *tname;
+	int use_overlay;
+} tcases[] = {
+	{ "readahead on file", 0 },
+	{ "readahead on overlayfs file", 1 },
+};
+
+static int readahead_supported = 1;
+
 static int has_file(const char *fname, int required)
 {
 	struct stat buf;
@@ -97,12 +114,13 @@ static unsigned long get_cached_size(void)
 	return parse_entry(meminfo_fname, entry);
 }
 
-static void create_testfile(void)
+static void create_testfile(int use_overlay)
 {
 	int fd;
 	char *tmp;
 	size_t i;
 
+	sprintf(testfile, "%s/testfile", use_overlay ? OVL_MNT : MNTPOINT);
 	tst_res(TINFO, "creating test file of size: %zu", testfile_size);
 	tmp = SAFE_MALLOC(pagesize);
 
@@ -203,21 +221,33 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 	SAFE_CLOSE(fd);
 }
 
-static void test_readahead(void)
+static void test_readahead(unsigned int n)
 {
 	unsigned long read_bytes, read_bytes_ra;
 	long long usec, usec_ra;
-	unsigned long cached_max, cached_low, cached, cached_ra;
+	unsigned long cached_high, cached_low, cached, cached_ra;
 	char proc_io_fname[128];
+	struct tcase *tc = &tcases[n];
+
+	tst_res(TINFO, "Test #%d: %s", n, tc->tname);
+
 	sprintf(proc_io_fname, "/proc/%u/io", getpid());
 
+	if (tc->use_overlay && !ovl_mounted) {
+		tst_res(TCONF,
+			"overlayfs is not configured in this kernel.");
+		return;
+	}
+
+	create_testfile(tc->use_overlay);
+
 	/* find out how much can cache hold if we read whole file */
 	read_testfile(0, testfile, testfile_size, &read_bytes, &usec, &cached);
-	cached_max = get_cached_size();
+	cached_high = get_cached_size();
 	sync();
 	drop_caches();
 	cached_low = get_cached_size();
-	cached_max = cached_max - cached_low;
+	cached_max = MAX(cached_max, cached_high - cached_low);
 
 	tst_res(TINFO, "read_testfile(0)");
 	read_testfile(0, testfile, testfile_size, &read_bytes, &usec, &cached);
@@ -233,11 +263,14 @@ static void test_readahead(void)
 	read_testfile(1, testfile, testfile_size, &read_bytes_ra,
 		      &usec_ra, &cached_ra);
 	if (TST_RET != 0) {
-		if (TST_ERR == EINVAL) {
+		if (TST_ERR == EINVAL &&
+		    (!tc->use_overlay || !readahead_supported)) {
+			readahead_supported = 0;
 			tst_res(TCONF, "readahead not supported on %s",
 				tst_device->fs_type);
 		} else {
 			tst_res(TFAIL | TTERRNO, "readahead failed on %s",
+				tc->use_overlay ? "overlayfs" :
 				tst_device->fs_type);
 		}
 		return;
@@ -284,6 +317,28 @@ static void test_readahead(void)
 	}
 }
 
+static void setup_overlay(void)
+{
+	int ret;
+
+	/* Setup an overlay mount with lower dir and file */
+	SAFE_MKDIR(OVL_LOWER, 0755);
+	SAFE_MKDIR(OVL_UPPER, 0755);
+	SAFE_MKDIR(OVL_WORK, 0755);
+	SAFE_MKDIR(OVL_MNT, 0755);
+	ret = mount("overlay", OVL_MNT, "overlay", 0, "lowerdir="OVL_LOWER
+		    ",upperdir="OVL_UPPER",workdir="OVL_WORK);
+	if (ret < 0) {
+		if (errno == ENODEV) {
+			tst_res(TINFO,
+				"overlayfs is not configured in this kernel.");
+			return;
+		}
+		tst_brk(TBROK | TERRNO, "overlayfs mount failed");
+	}
+	ovl_mounted = 1;
+}
+
 static void setup(void)
 {
 	if (opt_fsizestr)
@@ -297,8 +352,13 @@ static void setup(void)
 
 	pagesize = getpagesize();
 
-	sprintf(testfile, "%s/testfile", mntpoint);
-	create_testfile();
+	setup_overlay();
+}
+
+static void cleanup(void)
+{
+	if (ovl_mounted)
+		SAFE_UMOUNT(OVL_MNT);
 }
 
 static struct tst_test test = {
@@ -307,6 +367,8 @@ static struct tst_test test = {
 	.mount_device = 1,
 	.mntpoint = mntpoint,
 	.setup = setup,
+	.cleanup = cleanup,
 	.options = options,
-	.test_all = test_readahead,
+	.test = test_readahead,
+	.tcnt = ARRAY_SIZE(tcases),
 };
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH v4 6/6] syscalls/readahead02: test readahead using posix_fadvise()
  2018-11-28 16:46 ` [LTP] " Amir Goldstein
@ 2018-11-28 16:46   ` Amir Goldstein
  -1 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-11-28 16:46 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp, Jan Stancek, Miklos Szeredi, linux-unionfs

The call to posix_fadvise(POSIX_FADV_WILLNEED) should have the same
effect as the call to readahead() syscall.

Repeat the test cases for local file and overlayfs file with
posix_fadvise().

The new test case is a regression test for kernel commit b833a3660394
("ovl: add ovl_fadvise()") which fixes a regression of fadvise() on
an overlay file that was introduced by kernel commit 5b910bd615ba
("ovl: fix GPF in swapfile_activate of file from overlayfs over xfs").

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .../kernel/syscalls/readahead/readahead02.c   | 41 +++++++++++++++----
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 73c8a6ce6..44475487e 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -54,12 +54,30 @@ static struct tst_option options[] = {
 static struct tcase {
 	const char *tname;
 	int use_overlay;
+	int use_fadvise;
 } tcases[] = {
-	{ "readahead on file", 0 },
-	{ "readahead on overlayfs file", 1 },
+	{ "readahead on file", 0, 0 },
+	{ "readahead on overlayfs file", 1, 0 },
+	{ "POSIX_FADV_WILLNEED on file", 0, 1 },
+	{ "POSIX_FADV_WILLNEED on overlayfs file", 1, 1 },
 };
 
+static int fadvise_willneed(int fd, off_t offset, size_t len)
+{
+	/* Should have the same effect as readahead() syscall */
+	return posix_fadvise(fd, offset, len, POSIX_FADV_WILLNEED);
+}
+
+static int libc_readahead(int fd, off_t offset, size_t len)
+{
+	return readahead(fd, offset, len);
+}
+
+typedef int (*readahead_func_t)(int, off_t, size_t);
+static readahead_func_t readahead_func = libc_readahead;
+
 static int readahead_supported = 1;
+static int fadvise_supported = 1;
 
 static int has_file(const char *fname, int required)
 {
@@ -135,7 +153,6 @@ static void create_testfile(int use_overlay)
 	free(tmp);
 }
 
-
 /* read_testfile - mmap testfile and read every page.
  * This functions measures how many I/O and time it takes to fully
  * read contents of test file.
@@ -163,7 +180,7 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 	if (do_readahead) {
 		cached_start = get_cached_size();
 		do {
-			TEST(readahead(fd, offset, fsize - offset));
+			TEST(readahead_func(fd, offset, fsize - offset));
 			if (TST_RET != 0) {
 				SAFE_CLOSE(fd);
 				return;
@@ -241,6 +258,9 @@ static void test_readahead(unsigned int n)
 
 	create_testfile(tc->use_overlay);
 
+	/* Use either readahead() syscall or POSIX_FADV_WILLNEED */
+	readahead_func = tc->use_fadvise ? fadvise_willneed : libc_readahead;
+
 	/* find out how much can cache hold if we read whole file */
 	read_testfile(0, testfile, testfile_size, &read_bytes, &usec, &cached);
 	cached_high = get_cached_size();
@@ -263,13 +283,20 @@ static void test_readahead(unsigned int n)
 	read_testfile(1, testfile, testfile_size, &read_bytes_ra,
 		      &usec_ra, &cached_ra);
 	if (TST_RET != 0) {
-		if (TST_ERR == EINVAL &&
-		    (!tc->use_overlay || !readahead_supported)) {
+		/* posix_fadvise returns error number (not in errno) */
+		if (tc->use_fadvise && (TST_ERR = TST_RET) == EINVAL &&
+		    (!tc->use_overlay || !fadvise_supported)) {
+			fadvise_supported = 0;
+			tst_res(TCONF, "fadvise not supported on %s",
+				tst_device->fs_type);
+		} else if (TST_ERR == EINVAL &&
+			   (!tc->use_overlay || !readahead_supported)) {
 			readahead_supported = 0;
 			tst_res(TCONF, "readahead not supported on %s",
 				tst_device->fs_type);
 		} else {
-			tst_res(TFAIL | TTERRNO, "readahead failed on %s",
+			tst_res(TFAIL | TTERRNO, "%s failed on %s",
+				tc->use_fadvise ? "fadvise" : "readahead",
 				tc->use_overlay ? "overlayfs" :
 				tst_device->fs_type);
 		}
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [LTP] [PATCH v4 6/6] syscalls/readahead02: test readahead using posix_fadvise()
@ 2018-11-28 16:46   ` Amir Goldstein
  0 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-11-28 16:46 UTC (permalink / raw)
  To: ltp

The call to posix_fadvise(POSIX_FADV_WILLNEED) should have the same
effect as the call to readahead() syscall.

Repeat the test cases for local file and overlayfs file with
posix_fadvise().

The new test case is a regression test for kernel commit b833a3660394
("ovl: add ovl_fadvise()") which fixes a regression of fadvise() on
an overlay file that was introduced by kernel commit 5b910bd615ba
("ovl: fix GPF in swapfile_activate of file from overlayfs over xfs").

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .../kernel/syscalls/readahead/readahead02.c   | 41 +++++++++++++++----
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 73c8a6ce6..44475487e 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -54,12 +54,30 @@ static struct tst_option options[] = {
 static struct tcase {
 	const char *tname;
 	int use_overlay;
+	int use_fadvise;
 } tcases[] = {
-	{ "readahead on file", 0 },
-	{ "readahead on overlayfs file", 1 },
+	{ "readahead on file", 0, 0 },
+	{ "readahead on overlayfs file", 1, 0 },
+	{ "POSIX_FADV_WILLNEED on file", 0, 1 },
+	{ "POSIX_FADV_WILLNEED on overlayfs file", 1, 1 },
 };
 
+static int fadvise_willneed(int fd, off_t offset, size_t len)
+{
+	/* Should have the same effect as readahead() syscall */
+	return posix_fadvise(fd, offset, len, POSIX_FADV_WILLNEED);
+}
+
+static int libc_readahead(int fd, off_t offset, size_t len)
+{
+	return readahead(fd, offset, len);
+}
+
+typedef int (*readahead_func_t)(int, off_t, size_t);
+static readahead_func_t readahead_func = libc_readahead;
+
 static int readahead_supported = 1;
+static int fadvise_supported = 1;
 
 static int has_file(const char *fname, int required)
 {
@@ -135,7 +153,6 @@ static void create_testfile(int use_overlay)
 	free(tmp);
 }
 
-
 /* read_testfile - mmap testfile and read every page.
  * This functions measures how many I/O and time it takes to fully
  * read contents of test file.
@@ -163,7 +180,7 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
 	if (do_readahead) {
 		cached_start = get_cached_size();
 		do {
-			TEST(readahead(fd, offset, fsize - offset));
+			TEST(readahead_func(fd, offset, fsize - offset));
 			if (TST_RET != 0) {
 				SAFE_CLOSE(fd);
 				return;
@@ -241,6 +258,9 @@ static void test_readahead(unsigned int n)
 
 	create_testfile(tc->use_overlay);
 
+	/* Use either readahead() syscall or POSIX_FADV_WILLNEED */
+	readahead_func = tc->use_fadvise ? fadvise_willneed : libc_readahead;
+
 	/* find out how much can cache hold if we read whole file */
 	read_testfile(0, testfile, testfile_size, &read_bytes, &usec, &cached);
 	cached_high = get_cached_size();
@@ -263,13 +283,20 @@ static void test_readahead(unsigned int n)
 	read_testfile(1, testfile, testfile_size, &read_bytes_ra,
 		      &usec_ra, &cached_ra);
 	if (TST_RET != 0) {
-		if (TST_ERR == EINVAL &&
-		    (!tc->use_overlay || !readahead_supported)) {
+		/* posix_fadvise returns error number (not in errno) */
+		if (tc->use_fadvise && (TST_ERR = TST_RET) == EINVAL &&
+		    (!tc->use_overlay || !fadvise_supported)) {
+			fadvise_supported = 0;
+			tst_res(TCONF, "fadvise not supported on %s",
+				tst_device->fs_type);
+		} else if (TST_ERR == EINVAL &&
+			   (!tc->use_overlay || !readahead_supported)) {
 			readahead_supported = 0;
 			tst_res(TCONF, "readahead not supported on %s",
 				tst_device->fs_type);
 		} else {
-			tst_res(TFAIL | TTERRNO, "readahead failed on %s",
+			tst_res(TFAIL | TTERRNO, "%s failed on %s",
+				tc->use_fadvise ? "fadvise" : "readahead",
 				tc->use_overlay ? "overlayfs" :
 				tst_device->fs_type);
 		}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH v4 2/6] syscalls/readahead02: abort test if readahead syscall fails
  2018-11-28 16:46   ` [LTP] " Amir Goldstein
@ 2018-12-03 14:27     ` Cyril Hrubis
  -1 siblings, 0 replies; 26+ messages in thread
From: Cyril Hrubis @ 2018-12-03 14:27 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: ltp, Jan Stancek, Miklos Szeredi, linux-unionfs

Hi!
> There is no reason to continue the test if readahead syscall fails
> and we can also check and report TCONF if filesystem does not support
> readahead.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  .../kernel/syscalls/readahead/readahead02.c   | 27 +++++++++----------
>  1 file changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
> index 956a1d5e5..88eb5fbff 100644
> --- a/testcases/kernel/syscalls/readahead/readahead02.c
> +++ b/testcases/kernel/syscalls/readahead/readahead02.c
> @@ -44,19 +44,6 @@ static struct tst_option options[] = {
>  	{NULL, NULL, NULL}
>  };
>  
> -static int check_ret(long expected_ret)
> -{
> -	if (expected_ret == TST_RET) {
> -		tst_res(TPASS, "expected ret success - "
> -			"returned value = %ld", TST_RET);
> -		return 0;
> -	}
> -	tst_res(TFAIL | TTERRNO, "unexpected failure - "
> -		"returned value = %ld, expected: %ld",
> -		TST_RET, expected_ret);
> -	return 1;
> -}
> -
>  static int has_file(const char *fname, int required)
>  {
>  	struct stat buf;
> @@ -162,8 +149,8 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
>  		do {
>  			TEST(readahead(fd, offset, fsize - offset));
>  			if (TST_RET != 0) {
> -				check_ret(0);
> -				break;
> +				SAFE_CLOSE(fd);
> +				return;
>  			}
>  
>  			/* estimate max readahead size based on first call */
> @@ -251,6 +238,16 @@ static void test_readahead(void)
>  	tst_res(TINFO, "read_testfile(1)");
>  	read_testfile(1, testfile, testfile_size, &read_bytes_ra,
>  		      &usec_ra, &cached_ra);
> +	if (TST_RET != 0) {
> +		if (TST_ERR == EINVAL) {
> +			tst_res(TCONF, "readahead not supported on %s",
> +				tst_device->fs_type);
> +		} else {
> +			tst_res(TFAIL | TTERRNO, "readahead failed on %s",
> +				tst_device->fs_type);
> +		}
> +		return;
> +	}

I do not like that we depend on the fact that TST_RET is not set
read_testfile() function. Can we rather than that explicitely return
the TST_ERR from the read_testfile() function instead? As it is zeroed
before the call in the TEST() macro we can just do return TST_ERR in the
read_testfile() and then ret = read_testfile() if (ret) ...

Also no need to resend, if you agree with the change I will fix that
before applying.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [LTP] [PATCH v4 2/6] syscalls/readahead02: abort test if readahead syscall fails
@ 2018-12-03 14:27     ` Cyril Hrubis
  0 siblings, 0 replies; 26+ messages in thread
From: Cyril Hrubis @ 2018-12-03 14:27 UTC (permalink / raw)
  To: ltp

Hi!
> There is no reason to continue the test if readahead syscall fails
> and we can also check and report TCONF if filesystem does not support
> readahead.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  .../kernel/syscalls/readahead/readahead02.c   | 27 +++++++++----------
>  1 file changed, 12 insertions(+), 15 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
> index 956a1d5e5..88eb5fbff 100644
> --- a/testcases/kernel/syscalls/readahead/readahead02.c
> +++ b/testcases/kernel/syscalls/readahead/readahead02.c
> @@ -44,19 +44,6 @@ static struct tst_option options[] = {
>  	{NULL, NULL, NULL}
>  };
>  
> -static int check_ret(long expected_ret)
> -{
> -	if (expected_ret == TST_RET) {
> -		tst_res(TPASS, "expected ret success - "
> -			"returned value = %ld", TST_RET);
> -		return 0;
> -	}
> -	tst_res(TFAIL | TTERRNO, "unexpected failure - "
> -		"returned value = %ld, expected: %ld",
> -		TST_RET, expected_ret);
> -	return 1;
> -}
> -
>  static int has_file(const char *fname, int required)
>  {
>  	struct stat buf;
> @@ -162,8 +149,8 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
>  		do {
>  			TEST(readahead(fd, offset, fsize - offset));
>  			if (TST_RET != 0) {
> -				check_ret(0);
> -				break;
> +				SAFE_CLOSE(fd);
> +				return;
>  			}
>  
>  			/* estimate max readahead size based on first call */
> @@ -251,6 +238,16 @@ static void test_readahead(void)
>  	tst_res(TINFO, "read_testfile(1)");
>  	read_testfile(1, testfile, testfile_size, &read_bytes_ra,
>  		      &usec_ra, &cached_ra);
> +	if (TST_RET != 0) {
> +		if (TST_ERR == EINVAL) {
> +			tst_res(TCONF, "readahead not supported on %s",
> +				tst_device->fs_type);
> +		} else {
> +			tst_res(TFAIL | TTERRNO, "readahead failed on %s",
> +				tst_device->fs_type);
> +		}
> +		return;
> +	}

I do not like that we depend on the fact that TST_RET is not set
read_testfile() function. Can we rather than that explicitely return
the TST_ERR from the read_testfile() function instead? As it is zeroed
before the call in the TEST() macro we can just do return TST_ERR in the
read_testfile() and then ret = read_testfile() if (ret) ...

Also no need to resend, if you agree with the change I will fix that
before applying.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v4 1/6] syscalls/readahead02: Convert to newlib and cleanup
  2018-11-28 16:46   ` [LTP] " Amir Goldstein
@ 2018-12-03 14:39     ` Cyril Hrubis
  -1 siblings, 0 replies; 26+ messages in thread
From: Cyril Hrubis @ 2018-12-03 14:39 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: ltp, Jan Stancek, Miklos Szeredi, linux-unionfs

Hi!
> +static struct tst_test test = {
> +	.needs_root = 1,
> +	.needs_tmpdir = 1,
> +	.mount_device = 1,
> +	.mntpoint = mntpoint,
> +	.setup = setup,
> +	.options = options,
> +	.test_all = test_readahead,
> +};

I do wonder if it makes sense to set the all_filesystems flag now.

Also does it make any sense to run the overlay test against all
filesystems? I would expect that it does not, so we will have to figure
out how to run the tests. Maybe it would make most sense to treat the
overlayfs as another filesystem in all_filesystem tests...

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [LTP] [PATCH v4 1/6] syscalls/readahead02: Convert to newlib and cleanup
@ 2018-12-03 14:39     ` Cyril Hrubis
  0 siblings, 0 replies; 26+ messages in thread
From: Cyril Hrubis @ 2018-12-03 14:39 UTC (permalink / raw)
  To: ltp

Hi!
> +static struct tst_test test = {
> +	.needs_root = 1,
> +	.needs_tmpdir = 1,
> +	.mount_device = 1,
> +	.mntpoint = mntpoint,
> +	.setup = setup,
> +	.options = options,
> +	.test_all = test_readahead,
> +};

I do wonder if it makes sense to set the all_filesystems flag now.

Also does it make any sense to run the overlay test against all
filesystems? I would expect that it does not, so we will have to figure
out how to run the tests. Maybe it would make most sense to treat the
overlayfs as another filesystem in all_filesystem tests...

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v4 2/6] syscalls/readahead02: abort test if readahead syscall fails
  2018-12-03 14:27     ` [LTP] " Cyril Hrubis
@ 2018-12-03 14:52       ` Amir Goldstein
  -1 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-12-03 14:52 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp, Jan Stancek, Miklos Szeredi, overlayfs

On Mon, Dec 3, 2018 at 4:29 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > There is no reason to continue the test if readahead syscall fails
> > and we can also check and report TCONF if filesystem does not support
> > readahead.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >  .../kernel/syscalls/readahead/readahead02.c   | 27 +++++++++----------
> >  1 file changed, 12 insertions(+), 15 deletions(-)
> >
> > diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
> > index 956a1d5e5..88eb5fbff 100644
> > --- a/testcases/kernel/syscalls/readahead/readahead02.c
> > +++ b/testcases/kernel/syscalls/readahead/readahead02.c
> > @@ -44,19 +44,6 @@ static struct tst_option options[] = {
> >       {NULL, NULL, NULL}
> >  };
> >
> > -static int check_ret(long expected_ret)
> > -{
> > -     if (expected_ret == TST_RET) {
> > -             tst_res(TPASS, "expected ret success - "
> > -                     "returned value = %ld", TST_RET);
> > -             return 0;
> > -     }
> > -     tst_res(TFAIL | TTERRNO, "unexpected failure - "
> > -             "returned value = %ld, expected: %ld",
> > -             TST_RET, expected_ret);
> > -     return 1;
> > -}
> > -
> >  static int has_file(const char *fname, int required)
> >  {
> >       struct stat buf;
> > @@ -162,8 +149,8 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
> >               do {
> >                       TEST(readahead(fd, offset, fsize - offset));
> >                       if (TST_RET != 0) {
> > -                             check_ret(0);
> > -                             break;
> > +                             SAFE_CLOSE(fd);
> > +                             return;
> >                       }
> >
> >                       /* estimate max readahead size based on first call */
> > @@ -251,6 +238,16 @@ static void test_readahead(void)
> >       tst_res(TINFO, "read_testfile(1)");
> >       read_testfile(1, testfile, testfile_size, &read_bytes_ra,
> >                     &usec_ra, &cached_ra);
> > +     if (TST_RET != 0) {
> > +             if (TST_ERR == EINVAL) {
> > +                     tst_res(TCONF, "readahead not supported on %s",
> > +                             tst_device->fs_type);
> > +             } else {
> > +                     tst_res(TFAIL | TTERRNO, "readahead failed on %s",
> > +                             tst_device->fs_type);
> > +             }
> > +             return;
> > +     }
>
> I do not like that we depend on the fact that TST_RET is not set
> read_testfile() function. Can we rather than that explicitely return
> the TST_ERR from the read_testfile() function instead? As it is zeroed
> before the call in the TEST() macro we can just do return TST_ERR in the
> read_testfile() and then ret = read_testfile() if (ret) ...
>
> Also no need to resend, if you agree with the change I will fix that
> before applying.

Fine by me.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [LTP] [PATCH v4 2/6] syscalls/readahead02: abort test if readahead syscall fails
@ 2018-12-03 14:52       ` Amir Goldstein
  0 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-12-03 14:52 UTC (permalink / raw)
  To: ltp

On Mon, Dec 3, 2018 at 4:29 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > There is no reason to continue the test if readahead syscall fails
> > and we can also check and report TCONF if filesystem does not support
> > readahead.
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >  .../kernel/syscalls/readahead/readahead02.c   | 27 +++++++++----------
> >  1 file changed, 12 insertions(+), 15 deletions(-)
> >
> > diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
> > index 956a1d5e5..88eb5fbff 100644
> > --- a/testcases/kernel/syscalls/readahead/readahead02.c
> > +++ b/testcases/kernel/syscalls/readahead/readahead02.c
> > @@ -44,19 +44,6 @@ static struct tst_option options[] = {
> >       {NULL, NULL, NULL}
> >  };
> >
> > -static int check_ret(long expected_ret)
> > -{
> > -     if (expected_ret == TST_RET) {
> > -             tst_res(TPASS, "expected ret success - "
> > -                     "returned value = %ld", TST_RET);
> > -             return 0;
> > -     }
> > -     tst_res(TFAIL | TTERRNO, "unexpected failure - "
> > -             "returned value = %ld, expected: %ld",
> > -             TST_RET, expected_ret);
> > -     return 1;
> > -}
> > -
> >  static int has_file(const char *fname, int required)
> >  {
> >       struct stat buf;
> > @@ -162,8 +149,8 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
> >               do {
> >                       TEST(readahead(fd, offset, fsize - offset));
> >                       if (TST_RET != 0) {
> > -                             check_ret(0);
> > -                             break;
> > +                             SAFE_CLOSE(fd);
> > +                             return;
> >                       }
> >
> >                       /* estimate max readahead size based on first call */
> > @@ -251,6 +238,16 @@ static void test_readahead(void)
> >       tst_res(TINFO, "read_testfile(1)");
> >       read_testfile(1, testfile, testfile_size, &read_bytes_ra,
> >                     &usec_ra, &cached_ra);
> > +     if (TST_RET != 0) {
> > +             if (TST_ERR == EINVAL) {
> > +                     tst_res(TCONF, "readahead not supported on %s",
> > +                             tst_device->fs_type);
> > +             } else {
> > +                     tst_res(TFAIL | TTERRNO, "readahead failed on %s",
> > +                             tst_device->fs_type);
> > +             }
> > +             return;
> > +     }
>
> I do not like that we depend on the fact that TST_RET is not set
> read_testfile() function. Can we rather than that explicitely return
> the TST_ERR from the read_testfile() function instead? As it is zeroed
> before the call in the TEST() macro we can just do return TST_ERR in the
> read_testfile() and then ret = read_testfile() if (ret) ...
>
> Also no need to resend, if you agree with the change I will fix that
> before applying.

Fine by me.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v4 1/6] syscalls/readahead02: Convert to newlib and cleanup
  2018-12-03 14:39     ` [LTP] " Cyril Hrubis
@ 2018-12-03 14:59       ` Amir Goldstein
  -1 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-12-03 14:59 UTC (permalink / raw)
  To: Cyril Hrubis; +Cc: ltp, Jan Stancek, Miklos Szeredi, overlayfs

On Mon, Dec 3, 2018 at 4:41 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > +static struct tst_test test = {
> > +     .needs_root = 1,
> > +     .needs_tmpdir = 1,
> > +     .mount_device = 1,
> > +     .mntpoint = mntpoint,
> > +     .setup = setup,
> > +     .options = options,
> > +     .test_all = test_readahead,
> > +};
>
> I do wonder if it makes sense to set the all_filesystems flag now.
>

That would be fine by me.

> Also does it make any sense to run the overlay test against all
> filesystems?

It does.
Overlayfs may behave differently on different underlying filesystems
depending on their characteristics.

> I would expect that it does not, so we will have to figure
> out how to run the tests. Maybe it would make most sense to treat the
> overlayfs as another filesystem in all_filesystem tests...
>

It does not make sense to treat overlayfs as another filesystem in
all_filesystem tests IMO because:
- "overlayfs" is not a well defined setup, you will need to define the
  underlying filesystem as well, and which one will it be?
- For good test coverage it is best to test, for each fs
  test over fs directly, test over overlayfs + with fs as underlying fs

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [LTP] [PATCH v4 1/6] syscalls/readahead02: Convert to newlib and cleanup
@ 2018-12-03 14:59       ` Amir Goldstein
  0 siblings, 0 replies; 26+ messages in thread
From: Amir Goldstein @ 2018-12-03 14:59 UTC (permalink / raw)
  To: ltp

On Mon, Dec 3, 2018 at 4:41 PM Cyril Hrubis <chrubis@suse.cz> wrote:
>
> Hi!
> > +static struct tst_test test = {
> > +     .needs_root = 1,
> > +     .needs_tmpdir = 1,
> > +     .mount_device = 1,
> > +     .mntpoint = mntpoint,
> > +     .setup = setup,
> > +     .options = options,
> > +     .test_all = test_readahead,
> > +};
>
> I do wonder if it makes sense to set the all_filesystems flag now.
>

That would be fine by me.

> Also does it make any sense to run the overlay test against all
> filesystems?

It does.
Overlayfs may behave differently on different underlying filesystems
depending on their characteristics.

> I would expect that it does not, so we will have to figure
> out how to run the tests. Maybe it would make most sense to treat the
> overlayfs as another filesystem in all_filesystem tests...
>

It does not make sense to treat overlayfs as another filesystem in
all_filesystem tests IMO because:
- "overlayfs" is not a well defined setup, you will need to define the
  underlying filesystem as well, and which one will it be?
- For good test coverage it is best to test, for each fs
  test over fs directly, test over overlayfs + with fs as underlying fs

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v4 5/6] syscalls/readahead02: test readahead() on an overlayfs file
  2018-11-28 16:46   ` [LTP] " Amir Goldstein
@ 2018-12-04 14:02     ` Cyril Hrubis
  -1 siblings, 0 replies; 26+ messages in thread
From: Cyril Hrubis @ 2018-12-04 14:02 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: ltp, Jan Stancek, Miklos Szeredi, linux-unionfs

Hi!
> The new test case is a regression test for kernel commit b833a3660394
> ("ovl: add ovl_fadvise()") which fixes a regression of readahead() on
> an overlay file that was introduced by kernel commit 5b910bd615ba
> ("ovl: fix GPF in swapfile_activate of file from overlayfs over xfs").

I've pushed the patches up to this patch with the change to the error
propagation as we agreed and:

* Added commit hashes of the two commits mentioned in this changelog
  to the top level comment of this test, since this is the place where
  testers look first for this kind of information.

+ Added one simplification patch on the top of this

I will look closer at the last one now.

Thanks for your work on the test.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [LTP] [PATCH v4 5/6] syscalls/readahead02: test readahead() on an overlayfs file
@ 2018-12-04 14:02     ` Cyril Hrubis
  0 siblings, 0 replies; 26+ messages in thread
From: Cyril Hrubis @ 2018-12-04 14:02 UTC (permalink / raw)
  To: ltp

Hi!
> The new test case is a regression test for kernel commit b833a3660394
> ("ovl: add ovl_fadvise()") which fixes a regression of readahead() on
> an overlay file that was introduced by kernel commit 5b910bd615ba
> ("ovl: fix GPF in swapfile_activate of file from overlayfs over xfs").

I've pushed the patches up to this patch with the change to the error
propagation as we agreed and:

* Added commit hashes of the two commits mentioned in this changelog
  to the top level comment of this test, since this is the place where
  testers look first for this kind of information.

+ Added one simplification patch on the top of this

I will look closer at the last one now.

Thanks for your work on the test.

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH v4 6/6] syscalls/readahead02: test readahead using posix_fadvise()
  2018-11-28 16:46   ` [LTP] " Amir Goldstein
@ 2018-12-04 14:10     ` Cyril Hrubis
  -1 siblings, 0 replies; 26+ messages in thread
From: Cyril Hrubis @ 2018-12-04 14:10 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: ltp, Jan Stancek, Miklos Szeredi, linux-unionfs

Hi!
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  .../kernel/syscalls/readahead/readahead02.c   | 41 +++++++++++++++----
>  1 file changed, 34 insertions(+), 7 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
> index 73c8a6ce6..44475487e 100644
> --- a/testcases/kernel/syscalls/readahead/readahead02.c
> +++ b/testcases/kernel/syscalls/readahead/readahead02.c
> @@ -54,12 +54,30 @@ static struct tst_option options[] = {
>  static struct tcase {
>  	const char *tname;
>  	int use_overlay;
> +	int use_fadvise;
>  } tcases[] = {
> -	{ "readahead on file", 0 },
> -	{ "readahead on overlayfs file", 1 },
> +	{ "readahead on file", 0, 0 },
> +	{ "readahead on overlayfs file", 1, 0 },
> +	{ "POSIX_FADV_WILLNEED on file", 0, 1 },
> +	{ "POSIX_FADV_WILLNEED on overlayfs file", 1, 1 },
>  };
>  
> +static int fadvise_willneed(int fd, off_t offset, size_t len)
> +{
> +	/* Should have the same effect as readahead() syscall */
> +	return posix_fadvise(fd, offset, len, POSIX_FADV_WILLNEED);
> +}
> +
> +static int libc_readahead(int fd, off_t offset, size_t len)
> +{
> +	return readahead(fd, offset, len);
> +}
> +
> +typedef int (*readahead_func_t)(int, off_t, size_t);
> +static readahead_func_t readahead_func = libc_readahead;

It's kind of useless to typedef a new type just to define one variable.

Also as we are making wrappers around these functions we should really
handle the differencies in error reporting inside of these wrappers,
i.e. change the fadvise_willneed() so that the function sets errno on a
failure to avoid details like that leaking into the test code.

Also if we want to switch later on to the all_filesystems we have to
avoid having preset values so it would make sense to have the readahead
function pointer in the tcase structure.

>  static int readahead_supported = 1;
> +static int fadvise_supported = 1;
>  
>  static int has_file(const char *fname, int required)
>  {
> @@ -135,7 +153,6 @@ static void create_testfile(int use_overlay)
>  	free(tmp);
>  }
>  
> -
>  /* read_testfile - mmap testfile and read every page.
>   * This functions measures how many I/O and time it takes to fully
>   * read contents of test file.
> @@ -163,7 +180,7 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
>  	if (do_readahead) {
>  		cached_start = get_cached_size();
>  		do {
> -			TEST(readahead(fd, offset, fsize - offset));
> +			TEST(readahead_func(fd, offset, fsize - offset));
>  			if (TST_RET != 0) {
>  				SAFE_CLOSE(fd);
>  				return;
> @@ -241,6 +258,9 @@ static void test_readahead(unsigned int n)
>  
>  	create_testfile(tc->use_overlay);
>  
> +	/* Use either readahead() syscall or POSIX_FADV_WILLNEED */
> +	readahead_func = tc->use_fadvise ? fadvise_willneed : libc_readahead;
> +
>  	/* find out how much can cache hold if we read whole file */
>  	read_testfile(0, testfile, testfile_size, &read_bytes, &usec, &cached);
>  	cached_high = get_cached_size();
> @@ -263,13 +283,20 @@ static void test_readahead(unsigned int n)
>  	read_testfile(1, testfile, testfile_size, &read_bytes_ra,
>  		      &usec_ra, &cached_ra);
>  	if (TST_RET != 0) {
> -		if (TST_ERR == EINVAL &&
> -		    (!tc->use_overlay || !readahead_supported)) {
> +		/* posix_fadvise returns error number (not in errno) */
> +		if (tc->use_fadvise && (TST_ERR = TST_RET) == EINVAL &&
> +		    (!tc->use_overlay || !fadvise_supported)) {
> +			fadvise_supported = 0;
> +			tst_res(TCONF, "fadvise not supported on %s",
> +				tst_device->fs_type);
> +		} else if (TST_ERR == EINVAL &&
> +			   (!tc->use_overlay || !readahead_supported)) {
>  			readahead_supported = 0;
>  			tst_res(TCONF, "readahead not supported on %s",
>  				tst_device->fs_type);
>  		} else {
> -			tst_res(TFAIL | TTERRNO, "readahead failed on %s",
> +			tst_res(TFAIL | TTERRNO, "%s failed on %s",
> +				tc->use_fadvise ? "fadvise" : "readahead",
>  				tc->use_overlay ? "overlayfs" :
>  				tst_device->fs_type);
>  		}
> -- 
> 2.17.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [LTP] [PATCH v4 6/6] syscalls/readahead02: test readahead using posix_fadvise()
@ 2018-12-04 14:10     ` Cyril Hrubis
  0 siblings, 0 replies; 26+ messages in thread
From: Cyril Hrubis @ 2018-12-04 14:10 UTC (permalink / raw)
  To: ltp

Hi!
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  .../kernel/syscalls/readahead/readahead02.c   | 41 +++++++++++++++----
>  1 file changed, 34 insertions(+), 7 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
> index 73c8a6ce6..44475487e 100644
> --- a/testcases/kernel/syscalls/readahead/readahead02.c
> +++ b/testcases/kernel/syscalls/readahead/readahead02.c
> @@ -54,12 +54,30 @@ static struct tst_option options[] = {
>  static struct tcase {
>  	const char *tname;
>  	int use_overlay;
> +	int use_fadvise;
>  } tcases[] = {
> -	{ "readahead on file", 0 },
> -	{ "readahead on overlayfs file", 1 },
> +	{ "readahead on file", 0, 0 },
> +	{ "readahead on overlayfs file", 1, 0 },
> +	{ "POSIX_FADV_WILLNEED on file", 0, 1 },
> +	{ "POSIX_FADV_WILLNEED on overlayfs file", 1, 1 },
>  };
>  
> +static int fadvise_willneed(int fd, off_t offset, size_t len)
> +{
> +	/* Should have the same effect as readahead() syscall */
> +	return posix_fadvise(fd, offset, len, POSIX_FADV_WILLNEED);
> +}
> +
> +static int libc_readahead(int fd, off_t offset, size_t len)
> +{
> +	return readahead(fd, offset, len);
> +}
> +
> +typedef int (*readahead_func_t)(int, off_t, size_t);
> +static readahead_func_t readahead_func = libc_readahead;

It's kind of useless to typedef a new type just to define one variable.

Also as we are making wrappers around these functions we should really
handle the differencies in error reporting inside of these wrappers,
i.e. change the fadvise_willneed() so that the function sets errno on a
failure to avoid details like that leaking into the test code.

Also if we want to switch later on to the all_filesystems we have to
avoid having preset values so it would make sense to have the readahead
function pointer in the tcase structure.

>  static int readahead_supported = 1;
> +static int fadvise_supported = 1;
>  
>  static int has_file(const char *fname, int required)
>  {
> @@ -135,7 +153,6 @@ static void create_testfile(int use_overlay)
>  	free(tmp);
>  }
>  
> -
>  /* read_testfile - mmap testfile and read every page.
>   * This functions measures how many I/O and time it takes to fully
>   * read contents of test file.
> @@ -163,7 +180,7 @@ static void read_testfile(int do_readahead, const char *fname, size_t fsize,
>  	if (do_readahead) {
>  		cached_start = get_cached_size();
>  		do {
> -			TEST(readahead(fd, offset, fsize - offset));
> +			TEST(readahead_func(fd, offset, fsize - offset));
>  			if (TST_RET != 0) {
>  				SAFE_CLOSE(fd);
>  				return;
> @@ -241,6 +258,9 @@ static void test_readahead(unsigned int n)
>  
>  	create_testfile(tc->use_overlay);
>  
> +	/* Use either readahead() syscall or POSIX_FADV_WILLNEED */
> +	readahead_func = tc->use_fadvise ? fadvise_willneed : libc_readahead;
> +
>  	/* find out how much can cache hold if we read whole file */
>  	read_testfile(0, testfile, testfile_size, &read_bytes, &usec, &cached);
>  	cached_high = get_cached_size();
> @@ -263,13 +283,20 @@ static void test_readahead(unsigned int n)
>  	read_testfile(1, testfile, testfile_size, &read_bytes_ra,
>  		      &usec_ra, &cached_ra);
>  	if (TST_RET != 0) {
> -		if (TST_ERR == EINVAL &&
> -		    (!tc->use_overlay || !readahead_supported)) {
> +		/* posix_fadvise returns error number (not in errno) */
> +		if (tc->use_fadvise && (TST_ERR = TST_RET) == EINVAL &&
> +		    (!tc->use_overlay || !fadvise_supported)) {
> +			fadvise_supported = 0;
> +			tst_res(TCONF, "fadvise not supported on %s",
> +				tst_device->fs_type);
> +		} else if (TST_ERR == EINVAL &&
> +			   (!tc->use_overlay || !readahead_supported)) {
>  			readahead_supported = 0;
>  			tst_res(TCONF, "readahead not supported on %s",
>  				tst_device->fs_type);
>  		} else {
> -			tst_res(TFAIL | TTERRNO, "readahead failed on %s",
> +			tst_res(TFAIL | TTERRNO, "%s failed on %s",
> +				tc->use_fadvise ? "fadvise" : "readahead",
>  				tc->use_overlay ? "overlayfs" :
>  				tst_device->fs_type);
>  		}
> -- 
> 2.17.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2018-12-04 14:10 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-28 16:46 [PATCH v4 0/6] Tests for readahead() and fadvise() on overlayfs Amir Goldstein
2018-11-28 16:46 ` [LTP] " Amir Goldstein
2018-11-28 16:46 ` [PATCH v4 1/6] syscalls/readahead02: Convert to newlib and cleanup Amir Goldstein
2018-11-28 16:46   ` [LTP] " Amir Goldstein
2018-12-03 14:39   ` Cyril Hrubis
2018-12-03 14:39     ` [LTP] " Cyril Hrubis
2018-12-03 14:59     ` Amir Goldstein
2018-12-03 14:59       ` [LTP] " Amir Goldstein
2018-11-28 16:46 ` [PATCH v4 2/6] syscalls/readahead02: abort test if readahead syscall fails Amir Goldstein
2018-11-28 16:46   ` [LTP] " Amir Goldstein
2018-12-03 14:27   ` Cyril Hrubis
2018-12-03 14:27     ` [LTP] " Cyril Hrubis
2018-12-03 14:52     ` Amir Goldstein
2018-12-03 14:52       ` [LTP] " Amir Goldstein
2018-11-28 16:46 ` [PATCH v4 3/6] syscalls/readahead02: fail test if readahead did not use any cache Amir Goldstein
2018-11-28 16:46   ` [LTP] " Amir Goldstein
2018-11-28 16:46 ` [PATCH v4 4/6] syscalls/readahead02: Convert to tst_timer helpers Amir Goldstein
2018-11-28 16:46   ` [LTP] " Amir Goldstein
2018-11-28 16:46 ` [PATCH v4 5/6] syscalls/readahead02: test readahead() on an overlayfs file Amir Goldstein
2018-11-28 16:46   ` [LTP] " Amir Goldstein
2018-12-04 14:02   ` Cyril Hrubis
2018-12-04 14:02     ` [LTP] " Cyril Hrubis
2018-11-28 16:46 ` [PATCH v4 6/6] syscalls/readahead02: test readahead using posix_fadvise() Amir Goldstein
2018-11-28 16:46   ` [LTP] " Amir Goldstein
2018-12-04 14:10   ` Cyril Hrubis
2018-12-04 14:10     ` [LTP] " Cyril Hrubis

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.