[PATCH v3 0/6] Tests for readahead() and fadvise() on overlayfs

[PATCH v3 0/6] Tests for readahead() and fadvise() on overlayfs

[Amir Goldstein]

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.

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/readahead01: Convert to newlib
  syscalls/readahead02: Convert to newlib and cleanup
  syscalls/readahead02: abort test if readahead syscall fails
  syscalls/readahead02: test readahead() on an overlayfs file
  syscalls/readahead02: test readahead using posix_fadvise()
  syscalls/readahead02: fail test if readahead did not use any cache

 .../kernel/syscalls/readahead/readahead01.c   | 119 ++----
 .../kernel/syscalls/readahead/readahead02.c   | 339 +++++++++---------
 2 files changed, 198 insertions(+), 260 deletions(-)

-- 
2.17.1

[PATCH v3 1/6] syscalls/readahead01: Convert to newlib

[Amir Goldstein]

* Use SPDX-License-Identifier

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

diff --git a/testcases/kernel/syscalls/readahead/readahead01.c b/testcases/kernel/syscalls/readahead/readahead01.c
index f35019488..682b524b3 100644
--- a/testcases/kernel/syscalls/readahead/readahead01.c
+++ b/testcases/kernel/syscalls/readahead/readahead01.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.
  */
 
 /*
@@ -36,47 +17,36 @@
 #include <sys/syscall.h>
 #include <sys/types.h>
 #include "config.h"
-#include "test.h"
-#include "safe_macros.h"
+#include "tst_test.h"
 #include "lapi/syscalls.h"
 
-char *TCID = "readahead01";
-int TST_TOTAL = 1;
-
-option_t options[] = {
-	{NULL, NULL, NULL}
-};
-
 #if defined(__NR_readahead)
-static void setup(void);
-static void cleanup(void);
-
 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, "unexpected failure - "
-		 "returned value = %ld, expected: %ld",
-		 TEST_RETURN, expected_ret);
+	tst_res(TFAIL, "unexpected failure - "
+		"returned value = %ld, expected: %ld",
+		TST_RET, expected_ret);
 	return 1;
 }
 
 static int check_errno(long expected_errno)
 {
-	if (TEST_ERRNO == expected_errno) {
-		tst_resm(TPASS | TTERRNO, "expected failure");
+	if (TST_ERR == expected_errno) {
+		tst_res(TPASS | TTERRNO, "expected failure");
 		return 0;
 	}
 
-	if (TEST_ERRNO == 0)
-		tst_resm(TFAIL, "call succeeded unexpectedly");
+	if (TST_ERR == 0)
+		tst_res(TFAIL, "call succeeded unexpectedly");
 	else
-		tst_resm(TFAIL | TTERRNO, "unexpected failure - "
-			 "expected = %ld : %s, actual",
-			 expected_errno, strerror(expected_errno));
+		tst_res(TFAIL | TTERRNO, "unexpected failure - "
+			"expected = %ld : %s, actual",
+			expected_errno, strerror(expected_errno));
 	return 1;
 }
 
@@ -85,19 +55,17 @@ static void test_bad_fd(void)
 	char tempname[PATH_MAX] = "readahead01_XXXXXX";
 	int fd;
 
-	tst_resm(TINFO, "test_bad_fd -1");
+	tst_res(TINFO, "test_bad_fd -1");
 	TEST(readahead(-1, 0, getpagesize()));
 	check_ret(-1);
 	check_errno(EBADF);
 
-	tst_resm(TINFO, "test_bad_fd O_WRONLY");
+	tst_res(TINFO, "test_bad_fd O_WRONLY");
 	fd = mkstemp(tempname);
 	if (fd == -1)
-		tst_resm(TBROK | TERRNO, "mkstemp failed");
+		tst_res(TBROK | TERRNO, "mkstemp failed");
 	close(fd);
-	fd = open(tempname, O_WRONLY);
-	if (fd == -1)
-		tst_resm(TBROK | TERRNO, "Failed to open testfile");
+	fd = SAFE_OPEN(tempname, O_WRONLY);
 	TEST(readahead(fd, 0, getpagesize()));
 	check_ret(-1);
 	check_errno(EBADF);
@@ -109,60 +77,41 @@ static void test_invalid_fd(void)
 {
 	int fd[2];
 
-	tst_resm(TINFO, "test_invalid_fd pipe");
-	if (pipe(fd) < 0)
-		tst_resm(TBROK | TERRNO, "Failed to create pipe");
+	tst_res(TINFO, "test_invalid_fd pipe");
+	SAFE_PIPE(fd);
 	TEST(readahead(fd[0], 0, getpagesize()));
 	check_ret(-1);
 	check_errno(EINVAL);
 	close(fd[0]);
 	close(fd[1]);
 
-	tst_resm(TINFO, "test_invalid_fd socket");
-	fd[0] = socket(AF_INET, SOCK_STREAM, 0);
-	if (fd[0] < 0)
-		tst_resm(TBROK | TERRNO, "Failed to create socket");
+	tst_res(TINFO, "test_invalid_fd socket");
+	fd[0] = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0);
 	TEST(readahead(fd[0], 0, getpagesize()));
 	check_ret(-1);
 	check_errno(EINVAL);
 	close(fd[0]);
 }
 
-int main(int argc, char *argv[])
+void test_readahead(void)
 {
-	int lc;
-
-	tst_parse_opts(argc, argv, options, NULL);
-
-	setup();
-	for (lc = 0; TEST_LOOPING(lc); lc++) {
-		tst_count = 0;
-		test_bad_fd();
-		test_invalid_fd();
-	}
-	cleanup();
-	tst_exit();
+	test_bad_fd();
+	test_invalid_fd();
 }
 
 static void setup(void)
 {
-	tst_require_root();
-	tst_tmpdir();
-
 	/* check if readahead syscall is supported */
-	ltp_syscall(__NR_readahead, 0, 0, 0);
-
-	TEST_PAUSE;
+	tst_syscall(__NR_readahead, 0, 0, 0);
 }
 
-static void cleanup(void)
-{
-	tst_rmdir();
-}
+static struct tst_test test = {
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+	.setup = setup,
+	.test_all = test_readahead,
+};
 
 #else /* __NR_readahead */
-int main(void)
-{
-	tst_brkm(TCONF, NULL, "System doesn't support __NR_readahead");
-}
+	TST_TEST_TCONF("System doesn't support __NR_readahead");
 #endif
-- 
2.17.1

Re: [PATCH v3 1/6] syscalls/readahead01: Convert to newlib

[Cyril Hrubis]

Hi!
Pushed with minor changes:

* close() -> SAFE_CLOSE()

* Changed the check_* functions to void since we do not use the returned
  value

* Removed .needs_root = 1 since the test isn't doing anything priviledged

Thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

[PATCH v3 2/6] syscalls/readahead02: Convert to newlib and cleanup

[Amir Goldstein]

* Use SAFE macros

* Use SPDX-License-Identifier

* No need to cleanup test file from temp dir

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

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index c9d92a6a4..c739d3ba2 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,57 @@
 #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 +113,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 +156,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 +173,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 +183,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 +207,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 +239,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 +249,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 +257,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 +283,42 @@ 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();
-}
+static struct tst_test test = {
+	.needs_root = 1,
+	.needs_tmpdir = 1,
+	.mount_device = 1,
+	.mntpoint = mntpoint,
+	.setup = setup,
+	.options = options,
+	.test_all = test_readahead,
+};
 
 #else /* __NR_readahead */
-int main(void)
-{
-	tst_brkm(TCONF, NULL, "System doesn't support __NR_readahead");
-}
+	TST_TEST_TCONF("System doesn't support __NR_readahead");
 #endif
-- 
2.17.1

Re: [PATCH v3 2/6] syscalls/readahead02: Convert to newlib and cleanup

[Cyril Hrubis]

Hi!
> * Use SAFE macros
> 
> * Use SPDX-License-Identifier
> 
> * No need to cleanup test file from temp dir

Generally looks good, a few comments below.

> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  .../kernel/syscalls/readahead/readahead02.c   | 248 +++++-------------
>  1 file changed, 72 insertions(+), 176 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
> index c9d92a6a4..c739d3ba2 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,57 @@
>  #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;
>  }

I would be happier if we got rid of this function.

Maybe we should just move the loop from read_file() to a separate
function do_readahead() then the indentation would be one level less
there and the code could be put directly there.

>  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 +113,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 +156,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 +173,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 +183,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");

We should really switch to posix timers here, using wall clock for
elapsed time measurements in not a good idea for several reasons.

And we even do have nice API for this, see:

https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines#2221-measuring-elapsed-time-and-helper-functions

>  	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 +207,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 +239,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 +249,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 +257,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 +283,42 @@ 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);

This just a static string defined on the top of the test.

>  	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();
> -}
> +static struct tst_test test = {
> +	.needs_root = 1,
> +	.needs_tmpdir = 1,
> +	.mount_device = 1,
> +	.mntpoint = mntpoint,
> +	.setup = setup,
> +	.options = options,
> +	.test_all = test_readahead,
> +};
>  
>  #else /* __NR_readahead */
> -int main(void)
> -{
> -	tst_brkm(TCONF, NULL, "System doesn't support __NR_readahead");
> -}
> +	TST_TEST_TCONF("System doesn't support __NR_readahead");
>  #endif
> -- 
> 2.17.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz

Re: [LTP] [PATCH v3 2/6] syscalls/readahead02: Convert to newlib and cleanup

[Cyril Hrubis]

Hi!
> >  #if defined(__NR_readahead)

Also we do have fallback definitions in lapi/syscalls.h, so
__NR_readahead is never undefined, we can safely drop this ifdef.

-- 
Cyril Hrubis
chrubis@suse.cz

Re: [PATCH v3 2/6] syscalls/readahead02: Convert to newlib and cleanup

[Amir Goldstein]

...
> >  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;
> >  }
>
> I would be happier if we got rid of this function.
>

Done. (In next patch)

> Maybe we should just move the loop from read_file() to a separate
> function do_readahead() then the indentation would be one level less
> there and the code could be put directly there.
>
...

> >       if (gettimeofday(&now, NULL) == -1)
> > -             tst_brkm(TBROK | TERRNO, cleanup, "gettimeofday failed");
> > +             tst_brk(TBROK | TERRNO, "gettimeofday failed");
>
> We should really switch to posix timers here, using wall clock for
> elapsed time measurements in not a good idea for several reasons.
>
> And we even do have nice API for this, see:
>
> https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines#2221-measuring-elapsed-time-and-helper-functions
>

Nice! done in a separate patch.

...

> > -             sprintf(testfile, MNTPOINT"/testfile");
> > -     }
> > +     sprintf(testfile, "%s/testfile", mntpoint);
>
> This just a static string defined on the top of the test.
>

It is going to be modified per test case in patch
"test readahead() on an overlayfs file"

v4 posted.

Thanks,
Amir.

[PATCH v3 3/6] syscalls/readahead02: abort test if readahead syscall fails

[Amir Goldstein]

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   | 21 +++++++++++--------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index c739d3ba2..f77e7c66c 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -45,16 +45,17 @@ static struct tst_option options[] = {
 	{NULL, NULL, NULL}
 };
 
-static int check_ret(long expected_ret)
+static int check_ret(void)
 {
-	if (expected_ret == TST_RET) {
-		tst_res(TPASS, "expected ret success - "
-			"returned value = %ld", TST_RET);
+	if (TST_RET == 0)
 		return 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);
 	}
-	tst_res(TFAIL | TTERRNO, "unexpected failure - "
-		"returned value = %ld, expected: %ld",
-		TST_RET, expected_ret);
 	return 1;
 }
 
@@ -163,8 +164,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 */
@@ -252,6 +253,8 @@ 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 (check_ret())
+		return;
 	if (cached_ra > cached_low)
 		cached_ra = cached_ra - cached_low;
 	else
-- 
2.17.1

Re: [PATCH v3 3/6] syscalls/readahead02: abort test if readahead syscall fails

[Cyril Hrubis]

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.

This looks good, but I would like to get rid of the check_ret function
as I said in the first case.

So maybe we can leave it in the cleanup but move the actuall code in
this patch. Does that sounds good to you?

> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  .../kernel/syscalls/readahead/readahead02.c   | 21 +++++++++++--------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
> index c739d3ba2..f77e7c66c 100644
> --- a/testcases/kernel/syscalls/readahead/readahead02.c
> +++ b/testcases/kernel/syscalls/readahead/readahead02.c
> @@ -45,16 +45,17 @@ static struct tst_option options[] = {
>  	{NULL, NULL, NULL}
>  };
>  
> -static int check_ret(long expected_ret)
> +static int check_ret(void)
>  {
> -	if (expected_ret == TST_RET) {
> -		tst_res(TPASS, "expected ret success - "
> -			"returned value = %ld", TST_RET);
> +	if (TST_RET == 0)
>  		return 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);
>  	}
> -	tst_res(TFAIL | TTERRNO, "unexpected failure - "
> -		"returned value = %ld, expected: %ld",
> -		TST_RET, expected_ret);
>  	return 1;
>  }
>  
> @@ -163,8 +164,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 */
> @@ -252,6 +253,8 @@ 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 (check_ret())
> +		return;
>  	if (cached_ra > cached_low)
>  		cached_ra = cached_ra - cached_low;
>  	else
> -- 
> 2.17.1
> 

-- 
Cyril Hrubis
chrubis@suse.cz

Re: [PATCH v3 3/6] syscalls/readahead02: abort test if readahead syscall fails

[Amir Goldstein]

On Tue, Nov 20, 2018 at 6:32 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.
>
> This looks good, but I would like to get rid of the check_ret function
> as I said in the first case.
>
> So maybe we can leave it in the cleanup but move the actuall code in
> this patch. Does that sounds good to you?

Will do.

Thanks,
Amir.

[PATCH v3 4/6] syscalls/readahead02: test readahead() on an overlayfs file

[Amir Goldstein]

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   | 85 ++++++++++++++++---
 1 file changed, 73 insertions(+), 12 deletions(-)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index f77e7c66c..56a8496ff 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 <sys/time.h>
@@ -34,8 +35,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;
@@ -45,16 +52,28 @@ static struct tst_option options[] = {
 	{NULL, NULL, NULL}
 };
 
-static int check_ret(void)
+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 check_ret(struct tcase *tc)
 {
 	if (TST_RET == 0)
 		return 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",
-			tst_device->fs_type);
+			tc->use_overlay ? "overlayfs" : tst_device->fs_type);
 	}
 	return 1;
 }
@@ -112,12 +131,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);
 
@@ -224,21 +244,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 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);
@@ -253,7 +285,7 @@ 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 (check_ret())
+	if (check_ret(tc))
 		return;
 	if (cached_ra > cached_low)
 		cached_ra = cached_ra - cached_low;
@@ -295,6 +327,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)
@@ -308,8 +362,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 = {
@@ -318,8 +377,10 @@ 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),
 };
 
 #else /* __NR_readahead */
-- 
2.17.1

[PATCH v3 5/6] syscalls/readahead02: test readahead using posix_fadvise()

[Amir Goldstein]

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 56a8496ff..d5cc7677b 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -55,24 +55,49 @@ 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 check_ret(struct tcase *tc)
 {
 	if (TST_RET == 0)
 		return 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);
 	}
 	return 1;
@@ -152,7 +177,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.
@@ -182,7 +206,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;
@@ -264,6 +288,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();
-- 
2.17.1

[PATCH v3 6/6] syscalls/readahead02: fail test if readahead did not use any cache

[Amir Goldstein]

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 d5cc7677b..657461905 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -346,6 +346,8 @@ static void test_readahead(unsigned int n)
 		 */
 		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

Re: [PATCH v3 0/6] Tests for readahead() and fadvise() on overlayfs

[Amir Goldstein]

On Thu, Oct 11, 2018 at 2:34 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> Cyril,
>

Ping.

The affected kernel code as well as fix commits are now in v4.19.
Pushed rebased branch to:
https://github.com/amir73il/ltp/commits/overlayfs-devel

Also, there is an unrelated (besides the word "fadvise") cleanup branch
https://github.com/amir73il/ltp/commits/fadvise_cleanup
that converts the fadvise tests to newlib.

Thanks,
Amir.

> 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.
>
> 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/readahead01: Convert to newlib
>   syscalls/readahead02: Convert to newlib and cleanup
>   syscalls/readahead02: abort test if readahead syscall fails
>   syscalls/readahead02: test readahead() on an overlayfs file
>   syscalls/readahead02: test readahead using posix_fadvise()
>   syscalls/readahead02: fail test if readahead did not use any cache
>
>  .../kernel/syscalls/readahead/readahead01.c   | 119 ++----
>  .../kernel/syscalls/readahead/readahead02.c   | 339 +++++++++---------
>  2 files changed, 198 insertions(+), 260 deletions(-)
>
> --
> 2.17.1
>