[LTP] [PATCH v1] readahead02.c fixes: use tst_parse_filesize() so that we can pass sizes with units e.g. -s 128M

[LTP] [PATCH v1] readahead02.c fixes: use tst_parse_filesize() so that we can pass sizes with units e.g. -s 128M

[coolgw]

Signed-off-by: WEI GAO <wegao@suse.com>
---
 testcases/kernel/syscalls/readahead/readahead02.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 3ed88c005..c282b4d68 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -367,8 +367,8 @@ static void setup_readahead_length(void)
 
 static void setup(void)
 {
-	if (opt_fsizestr) {
-		testfile_size = SAFE_STRTOL(opt_fsizestr, 1, INT_MAX);
+        if (tst_parse_filesize(opt_fsizestr, &testfile_size, 1, INT_MAX)) {
+		tst_brk(TBROK, "invalid initial filesize '%s'", opt_fsizestr);
 		tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
 	}
 
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] readahead02.c fixes: use tst_parse_filesize() so that we can pass sizes with units e.g. -s 128M

[Petr Vorel]

Hi Wei,

I set the other 3 mesages as supperseded in patchwor.
Please before sending email, check whether it has been received by ML.
You should get it to your mail you use as Signed-off-by:,
also it's after ~1 min in https://lists.linux.it/pipermail/ltp/
and in https://patchwork.ozlabs.org/project/ltp/list/.
After some time (few minutes) it's also in
https://lore.kernel.org/ltp/

Patch subject is just too long and yet not describing things correctly.
https://chris.beams.io/posts/git-commit/

It could be something like this (patch continues on 3rd line):

readahead02.c: allow to pass human readable sizes

e.g. ./readahead02 -s 128M

---
NOTE: it's not a fix, but enhancement.
The fix would be to fix a real problem, e.g. test allows to define an invalid
size:

$ sudo ./readahead02 -s 100
tst_device.c:93: TINFO: Found free device 4 '/dev/loop4'
tst_test.c:1093: TINFO: Formatting /dev/loop4 with ext2 opts='' extra opts=''
mke2fs 1.46.5 (30-Dec-2021)
tst_test.c:1558: TINFO: Timeout per run is 0h 01m 00s
tst_test.c:1566: TINFO: Updating max runtime to 0h 00m 01s
tst_test.c:1558: TINFO: Timeout per run is 0h 00m 31s
readahead02.c:387: TINFO: readahead length: 4096
readahead02.c:223: TINFO: Test #0: readahead on file
readahead02.c:128: TINFO: creating test file of size: 100
readahead02.c:191: TBROK: mmap((nil),0,1,32769,3,0) failed: EINVAL (22)

=> although I haven't looked if this is a bug elsewhere in a test which should
be fixed, or test just needs more size (likely) and thus size should be required
higher.

> Signed-off-by: WEI GAO <wegao@suse.com>
> ---
>  testcases/kernel/syscalls/readahead/readahead02.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

> diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
> index 3ed88c005..c282b4d68 100644
> --- a/testcases/kernel/syscalls/readahead/readahead02.c
> +++ b/testcases/kernel/syscalls/readahead/readahead02.c
> @@ -367,8 +367,8 @@ static void setup_readahead_length(void)

>  static void setup(void)
>  {
> -	if (opt_fsizestr) {
> -		testfile_size = SAFE_STRTOL(opt_fsizestr, 1, INT_MAX);
> +        if (tst_parse_filesize(opt_fsizestr, &testfile_size, 1, INT_MAX)) {

Running make introduces warnings (we don't want to introduce new warnings):

readahead02.c: In function ‘setup’:
readahead02.c:370:46: warning: passing argument 2 of ‘tst_parse_filesize’ from incompatible pointer type [-Wincompatible-pointer-types]
  370 |         if (tst_parse_filesize(opt_fsizestr, &testfile_size, 1, INT_MAX)) {
      |                                              ^~~~~~~~~~~~~~
      |                                              |
      |                                              size_t * {aka long unsigned int *}
In file included from readahead02.c:33:
../../../../include/tst_test.h:138:52: note: expected ‘long long int *’ but argument is of type ‘size_t *’ {aka ‘long unsigned int *’}
  138 | int tst_parse_filesize(const char *str, long long *val, long long min, long long max);
      |                                         ~~~~~~~~~~~^~~

 tst_parse_filesize would have to be long long.
Therefore it would have to be casted (size_t) on other places
which expected the original size_t type.

Kind regards,
Petr

> +		tst_brk(TBROK, "invalid initial filesize '%s'", opt_fsizestr);
>  		tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
>  	}

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2] readahead02.c: Fix check input fsize

[Wei Gao via ltp]

From: coolgw <wegao@suse.com>

We run the test with a loop device it will fail with ENOSPC if we
pass -s bigger than the loop device, we should at least check if
the device is large enough in the test setup.The test should make
use of use tst_parse_filesize() so that we can pass sizes with
units e.g. -s 128M.

Signed-off-by: WEI GAO <wegao@suse.com>
Suggested-by: Petr Vorel <pvorel@suse.cz>
Suggested-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 include/tst_device.h                          |  2 ++
 lib/tst_device.c                              |  2 +-
 .../kernel/syscalls/readahead/readahead02.c   | 22 +++++++++++++++----
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/include/tst_device.h b/include/tst_device.h
index 977427f1c..67ceb25a4 100644
--- a/include/tst_device.h
+++ b/include/tst_device.h
@@ -6,6 +6,8 @@
 #ifndef TST_DEVICE_H__
 #define TST_DEVICE_H__
 
+#define DEV_SIZE_MB 300u
+
 #include <unistd.h>
 #include <stdint.h>
 #include <sys/stat.h>
diff --git a/lib/tst_device.c b/lib/tst_device.c
index 48d7e3ab6..b098fc80b 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -38,6 +38,7 @@
 #include "lapi/syscalls.h"
 #include "test.h"
 #include "safe_macros.h"
+#include "tst_device.h"
 
 #ifndef LOOP_CTL_GET_FREE
 # define LOOP_CTL_GET_FREE 0x4C82
@@ -46,7 +47,6 @@
 #define LOOP_CONTROL_FILE "/dev/loop-control"
 
 #define DEV_FILE "test_dev.img"
-#define DEV_SIZE_MB 300u
 #define UUID_STR_SZ 37
 #define UUID_FMT "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x"
 
diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 7acf4bb18..a9d0530c2 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -33,6 +33,7 @@
 #include "tst_test.h"
 #include "tst_timer.h"
 #include "lapi/syscalls.h"
+#include "tst_device.h"
 
 static char testfile[PATH_MAX] = "testfile";
 #define DROP_CACHES_FNAME "/proc/sys/vm/drop_caches"
@@ -366,11 +367,25 @@ static void setup_readahead_length(void)
 
 static void setup(void)
 {
-	if (opt_fsizestr) {
-		testfile_size = SAFE_STRTOL(opt_fsizestr, 1, INT_MAX);
-		tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
+	/*
+	 * Acutaly dev size will reduced after create filesystem,
+	 * so use dev_szie * 0.8 as dev real usage size, test case will
+	 * create two files within dev so we need div 2 get max file size
+	 */
+	size_t dev_size = (test.dev_min_size ? test.dev_min_size : DEV_SIZE_MB) * 1024 * 1024;
+	size_t fsize_max = dev_size * 0.8 / 2;
+
+	/* At least two pagesize for test case */
+	pagesize = getpagesize();
+	size_t fsize_min = pagesize * 2;
+
+	if (tst_parse_filesize(opt_fsizestr, (long long *)&testfile_size, fsize_min, fsize_max)) {
+		tst_set_max_runtime(1 + DEFAULT_FILESIZE / (DEFAULT_FILESIZE/32));
+		tst_brk(TBROK, "invalid initial filesize '%s'", opt_fsizestr);
 	}
 
+	tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
+
 	if (access(PROC_IO_FNAME, F_OK))
 		tst_brk(TCONF, "Requires " PROC_IO_FNAME);
 
@@ -380,7 +395,6 @@ static void setup(void)
 	/* check if readahead is supported */
 	tst_syscall(__NR_readahead, 0, 0, 0);
 
-	pagesize = getpagesize();
 
 	setup_readahead_length();
 	tst_res(TINFO, "readahead length: %d", readahead_length);
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3] readahead02.c: Fix check input fsize

[Wei Gao via ltp]

We run the test with a loop device it will fail with ENOSPC if we
pass -s bigger than the loop device, we should at least check if
the device is large enough in the test setup.The test should make
use of use tst_parse_filesize() so that we can pass sizes with
units e.g. -s 128M.

Signed-off-by: Wei Gao <wegao@suse.com>
Suggested-by: Petr Vorel <pvorel@suse.cz>
Suggested-by: Richard Palethorpe <rpalethorpe@suse.com>
Suggested-by: PCyril Hrubis <chrubis@suse.cz>
---
 include/tst_device.h                          |  4 +++-
 lib/tst_device.c                              |  2 +-
 .../kernel/syscalls/readahead/readahead02.c   | 22 +++++++++++++++----
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/include/tst_device.h b/include/tst_device.h
index 977427f1c..f03f17f7d 100644
--- a/include/tst_device.h
+++ b/include/tst_device.h
@@ -6,6 +6,8 @@
 #ifndef TST_DEVICE_H__
 #define TST_DEVICE_H__
 
+#define DEV_SIZE_MB 300u
+
 #include <unistd.h>
 #include <stdint.h>
 #include <sys/stat.h>
@@ -49,7 +51,7 @@ int tst_clear_device(const char *dev);
  * free loopdev). If path is non-NULL, it will be filled with free loopdev path.
  *
  */
-int tst_find_free_loopdev(const char *path, size_t path_len);
+int tst_find_free_loopdev(char *path, size_t path_len);
 
 /*
  * Attaches a file to a loop device.
diff --git a/lib/tst_device.c b/lib/tst_device.c
index 48d7e3ab6..b098fc80b 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -38,6 +38,7 @@
 #include "lapi/syscalls.h"
 #include "test.h"
 #include "safe_macros.h"
+#include "tst_device.h"
 
 #ifndef LOOP_CTL_GET_FREE
 # define LOOP_CTL_GET_FREE 0x4C82
@@ -46,7 +47,6 @@
 #define LOOP_CONTROL_FILE "/dev/loop-control"
 
 #define DEV_FILE "test_dev.img"
-#define DEV_SIZE_MB 300u
 #define UUID_STR_SZ 37
 #define UUID_FMT "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x"
 
diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 7acf4bb18..7cf6b5032 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -33,6 +33,7 @@
 #include "tst_test.h"
 #include "tst_timer.h"
 #include "lapi/syscalls.h"
+#include "tst_device.h"
 
 static char testfile[PATH_MAX] = "testfile";
 #define DROP_CACHES_FNAME "/proc/sys/vm/drop_caches"
@@ -366,11 +367,25 @@ static void setup_readahead_length(void)
 
 static void setup(void)
 {
-	if (opt_fsizestr) {
-		testfile_size = SAFE_STRTOL(opt_fsizestr, 1, INT_MAX);
-		tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
+	/*
+	 * Acutaly dev size will reduced after create filesystem,
+	 * so use dev_szie * 0.8 as dev real usage size, test case will
+	 * create two files within dev so we need div 2 get max file size
+	 */
+	size_t dev_size = (tst_device->size ? tst_device->size : DEV_SIZE_MB) * 1024 * 1024;
+	size_t fsize_max = dev_size * 0.8 / 2;
+
+	/* At least two pagesize for test case */
+	pagesize = getpagesize();
+	size_t fsize_min = pagesize * 2;
+
+	if (tst_parse_filesize(opt_fsizestr, (long long *)&testfile_size, fsize_min, fsize_max)) {
+		tst_set_max_runtime(1 + DEFAULT_FILESIZE / (DEFAULT_FILESIZE/32));
+		tst_brk(TBROK, "invalid initial filesize '%s'", opt_fsizestr);
 	}
 
+	tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
+
 	if (access(PROC_IO_FNAME, F_OK))
 		tst_brk(TCONF, "Requires " PROC_IO_FNAME);
 
@@ -380,7 +395,6 @@ static void setup(void)
 	/* check if readahead is supported */
 	tst_syscall(__NR_readahead, 0, 0, 0);
 
-	pagesize = getpagesize();
 
 	setup_readahead_length();
 	tst_res(TINFO, "readahead length: %d", readahead_length);
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2] readahead02.c: Fix check input fsize

[Cyril Hrubis]

Hi!
> We run the test with a loop device it will fail with ENOSPC if we
> pass -s bigger than the loop device, we should at least check if
> the device is large enough in the test setup.The test should make
> use of use tst_parse_filesize() so that we can pass sizes with
> units e.g. -s 128M.
> 
> Signed-off-by: WEI GAO <wegao@suse.com>
> Suggested-by: Petr Vorel <pvorel@suse.cz>
> Suggested-by: Richard Palethorpe <rpalethorpe@suse.com>
> ---
>  include/tst_device.h                          |  2 ++
>  lib/tst_device.c                              |  2 +-
>  .../kernel/syscalls/readahead/readahead02.c   | 22 +++++++++++++++----
>  3 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/include/tst_device.h b/include/tst_device.h
> index 977427f1c..67ceb25a4 100644
> --- a/include/tst_device.h
> +++ b/include/tst_device.h
> @@ -6,6 +6,8 @@
>  #ifndef TST_DEVICE_H__
>  #define TST_DEVICE_H__
>  
> +#define DEV_SIZE_MB 300u
> +
>  #include <unistd.h>
>  #include <stdint.h>
>  #include <sys/stat.h>
> diff --git a/lib/tst_device.c b/lib/tst_device.c
> index 48d7e3ab6..b098fc80b 100644
> --- a/lib/tst_device.c
> +++ b/lib/tst_device.c
> @@ -38,6 +38,7 @@
>  #include "lapi/syscalls.h"
>  #include "test.h"
>  #include "safe_macros.h"
> +#include "tst_device.h"
>  
>  #ifndef LOOP_CTL_GET_FREE
>  # define LOOP_CTL_GET_FREE 0x4C82
> @@ -46,7 +47,6 @@
>  #define LOOP_CONTROL_FILE "/dev/loop-control"
>  
>  #define DEV_FILE "test_dev.img"
> -#define DEV_SIZE_MB 300u
>  #define UUID_STR_SZ 37
>  #define UUID_FMT "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x"
>  
> diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
> index 7acf4bb18..a9d0530c2 100644
> --- a/testcases/kernel/syscalls/readahead/readahead02.c
> +++ b/testcases/kernel/syscalls/readahead/readahead02.c
> @@ -33,6 +33,7 @@
>  #include "tst_test.h"
>  #include "tst_timer.h"
>  #include "lapi/syscalls.h"
> +#include "tst_device.h"
>  
>  static char testfile[PATH_MAX] = "testfile";
>  #define DROP_CACHES_FNAME "/proc/sys/vm/drop_caches"
> @@ -366,11 +367,25 @@ static void setup_readahead_length(void)
>  
>  static void setup(void)
>  {
> -	if (opt_fsizestr) {
> -		testfile_size = SAFE_STRTOL(opt_fsizestr, 1, INT_MAX);
> -		tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
> +	/*
> +	 * Acutaly dev size will reduced after create filesystem,
> +	 * so use dev_szie * 0.8 as dev real usage size, test case will
> +	 * create two files within dev so we need div 2 get max file size
> +	 */
> +	size_t dev_size = (test.dev_min_size ? test.dev_min_size : DEV_SIZE_MB) * 1024 * 1024;

This is wrong. The actual device size may me completele different. You
have to use tst_device.size instead.

> +	size_t fsize_max = dev_size * 0.8 / 2;
> +
> +	/* At least two pagesize for test case */
> +	pagesize = getpagesize();
> +	size_t fsize_min = pagesize * 2;
> +
> +	if (tst_parse_filesize(opt_fsizestr, (long long *)&testfile_size, fsize_min, fsize_max)) {
> +		tst_set_max_runtime(1 + DEFAULT_FILESIZE / (DEFAULT_FILESIZE/32));
> +		tst_brk(TBROK, "invalid initial filesize '%s'", opt_fsizestr);
>  	}
>  
> +	tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
> +
>  	if (access(PROC_IO_FNAME, F_OK))
>  		tst_brk(TCONF, "Requires " PROC_IO_FNAME);
>  
> @@ -380,7 +395,6 @@ static void setup(void)
>  	/* check if readahead is supported */
>  	tst_syscall(__NR_readahead, 0, 0, 0);
>  
> -	pagesize = getpagesize();
>  
>  	setup_readahead_length();
>  	tst_res(TINFO, "readahead length: %d", readahead_length);
> -- 
> 2.35.3
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v4] readahead02.c: Fix check input fsize

[Wei Gao via ltp]

We run the test with a loop device it will fail with ENOSPC if we
pass -s bigger than the loop device, we should at least check if
the device is large enough in the test setup.The test should make
use of use tst_parse_filesize() so that we can pass sizes with
units e.g. -s 128M.

Signed-off-by: Wei Gao <wegao@suse.com>
Suggested-by: Cyril Hrubis <chrubis@suse.cz>
---
 include/tst_device.h                          |  4 +++-
 lib/tst_device.c                              |  2 +-
 .../kernel/syscalls/readahead/readahead02.c   | 21 +++++++++++++++----
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/include/tst_device.h b/include/tst_device.h
index 977427f1c..f03f17f7d 100644
--- a/include/tst_device.h
+++ b/include/tst_device.h
@@ -6,6 +6,8 @@
 #ifndef TST_DEVICE_H__
 #define TST_DEVICE_H__
 
+#define DEV_SIZE_MB 300u
+
 #include <unistd.h>
 #include <stdint.h>
 #include <sys/stat.h>
@@ -49,7 +51,7 @@ int tst_clear_device(const char *dev);
  * free loopdev). If path is non-NULL, it will be filled with free loopdev path.
  *
  */
-int tst_find_free_loopdev(const char *path, size_t path_len);
+int tst_find_free_loopdev(char *path, size_t path_len);
 
 /*
  * Attaches a file to a loop device.
diff --git a/lib/tst_device.c b/lib/tst_device.c
index 48d7e3ab6..b098fc80b 100644
--- a/lib/tst_device.c
+++ b/lib/tst_device.c
@@ -38,6 +38,7 @@
 #include "lapi/syscalls.h"
 #include "test.h"
 #include "safe_macros.h"
+#include "tst_device.h"
 
 #ifndef LOOP_CTL_GET_FREE
 # define LOOP_CTL_GET_FREE 0x4C82
@@ -46,7 +47,6 @@
 #define LOOP_CONTROL_FILE "/dev/loop-control"
 
 #define DEV_FILE "test_dev.img"
-#define DEV_SIZE_MB 300u
 #define UUID_STR_SZ 37
 #define UUID_FMT "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x"
 
diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 7acf4bb18..5989c7cbf 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -33,6 +33,7 @@
 #include "tst_test.h"
 #include "tst_timer.h"
 #include "lapi/syscalls.h"
+#include "tst_device.h"
 
 static char testfile[PATH_MAX] = "testfile";
 #define DROP_CACHES_FNAME "/proc/sys/vm/drop_caches"
@@ -366,11 +367,24 @@ static void setup_readahead_length(void)
 
 static void setup(void)
 {
-	if (opt_fsizestr) {
-		testfile_size = SAFE_STRTOL(opt_fsizestr, 1, INT_MAX);
-		tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
+	/*
+	 * Acutaly dev size will reduced after create filesystem,
+	 * so use dev_szie * 0.8 as dev real usage size, test case will
+	 * create two files within dev so we need div 2 get max file size
+	 */
+	size_t fsize_max = tst_device->size * 0.8 / 2 * 1024 * 1024;
+
+	/* At least two pagesize for test case */
+	pagesize = getpagesize();
+	size_t fsize_min = pagesize * 2;
+
+	if (tst_parse_filesize(opt_fsizestr, (long long *)&testfile_size, fsize_min, fsize_max)) {
+		tst_set_max_runtime(1 + DEFAULT_FILESIZE / (DEFAULT_FILESIZE/32));
+		tst_brk(TBROK, "invalid initial filesize '%s'", opt_fsizestr);
 	}
 
+	tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
+
 	if (access(PROC_IO_FNAME, F_OK))
 		tst_brk(TCONF, "Requires " PROC_IO_FNAME);
 
@@ -380,7 +394,6 @@ static void setup(void)
 	/* check if readahead is supported */
 	tst_syscall(__NR_readahead, 0, 0, 0);
 
-	pagesize = getpagesize();
 
 	setup_readahead_length();
 	tst_res(TINFO, "readahead length: %d", readahead_length);
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] readahead02.c: Fix check input fsize

[Cyril Hrubis]

Hi!
> We run the test with a loop device it will fail with ENOSPC if we
> pass -s bigger than the loop device, we should at least check if
> the device is large enough in the test setup.The test should make
> use of use tst_parse_filesize() so that we can pass sizes with
> units e.g. -s 128M.
> 
> Signed-off-by: Wei Gao <wegao@suse.com>
> Suggested-by: Petr Vorel <pvorel@suse.cz>
> Suggested-by: Richard Palethorpe <rpalethorpe@suse.com>
> Suggested-by: PCyril Hrubis <chrubis@suse.cz>
> ---
>  include/tst_device.h                          |  4 +++-
>  lib/tst_device.c                              |  2 +-
>  .../kernel/syscalls/readahead/readahead02.c   | 22 +++++++++++++++----
>  3 files changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/include/tst_device.h b/include/tst_device.h
> index 977427f1c..f03f17f7d 100644
> --- a/include/tst_device.h
> +++ b/include/tst_device.h
> @@ -6,6 +6,8 @@
>  #ifndef TST_DEVICE_H__
>  #define TST_DEVICE_H__
>  
> +#define DEV_SIZE_MB 300u
> +
>  #include <unistd.h>
>  #include <stdint.h>
>  #include <sys/stat.h>
> @@ -49,7 +51,7 @@ int tst_clear_device(const char *dev);
>   * free loopdev). If path is non-NULL, it will be filled with free loopdev path.
>   *
>   */
> -int tst_find_free_loopdev(const char *path, size_t path_len);
> +int tst_find_free_loopdev(char *path, size_t path_len);
>  
>  /*
>   * Attaches a file to a loop device.
> diff --git a/lib/tst_device.c b/lib/tst_device.c
> index 48d7e3ab6..b098fc80b 100644
> --- a/lib/tst_device.c
> +++ b/lib/tst_device.c
> @@ -38,6 +38,7 @@
>  #include "lapi/syscalls.h"
>  #include "test.h"
>  #include "safe_macros.h"
> +#include "tst_device.h"
>  
>  #ifndef LOOP_CTL_GET_FREE
>  # define LOOP_CTL_GET_FREE 0x4C82
> @@ -46,7 +47,6 @@
>  #define LOOP_CONTROL_FILE "/dev/loop-control"
>  
>  #define DEV_FILE "test_dev.img"
> -#define DEV_SIZE_MB 300u
>  #define UUID_STR_SZ 37
>  #define UUID_FMT "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x"
>  
> diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
> index 7acf4bb18..7cf6b5032 100644
> --- a/testcases/kernel/syscalls/readahead/readahead02.c
> +++ b/testcases/kernel/syscalls/readahead/readahead02.c
> @@ -33,6 +33,7 @@
>  #include "tst_test.h"
>  #include "tst_timer.h"
>  #include "lapi/syscalls.h"
> +#include "tst_device.h"
>  
>  static char testfile[PATH_MAX] = "testfile";
>  #define DROP_CACHES_FNAME "/proc/sys/vm/drop_caches"
> @@ -366,11 +367,25 @@ static void setup_readahead_length(void)
>  
>  static void setup(void)
>  {
> -	if (opt_fsizestr) {
> -		testfile_size = SAFE_STRTOL(opt_fsizestr, 1, INT_MAX);
> -		tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
> +	/*
> +	 * Acutaly dev size will reduced after create filesystem,
> +	 * so use dev_szie * 0.8 as dev real usage size, test case will
> +	 * create two files within dev so we need div 2 get max file size
> +	 */
> +	size_t dev_size = (tst_device->size ? tst_device->size : DEV_SIZE_MB) * 1024 * 1024;

Sigh, there is absolutelly no reason to use anything else than
tst_device->size. The tst_device->size _IS_ the actual device size
under all circumstances.

> +	size_t fsize_max = dev_size * 0.8 / 2;
> +
> +	/* At least two pagesize for test case */
> +	pagesize = getpagesize();
> +	size_t fsize_min = pagesize * 2;
> +
> +	if (tst_parse_filesize(opt_fsizestr, (long long *)&testfile_size, fsize_min, fsize_max)) {
> +		tst_set_max_runtime(1 + DEFAULT_FILESIZE / (DEFAULT_FILESIZE/32));
> +		tst_brk(TBROK, "invalid initial filesize '%s'", opt_fsizestr);
>  	}
>  
> +	tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
> +
>  	if (access(PROC_IO_FNAME, F_OK))
>  		tst_brk(TCONF, "Requires " PROC_IO_FNAME);
>  
> @@ -380,7 +395,6 @@ static void setup(void)
>  	/* check if readahead is supported */
>  	tst_syscall(__NR_readahead, 0, 0, 0);
>  
> -	pagesize = getpagesize();
>  
>  	setup_readahead_length();
>  	tst_res(TINFO, "readahead length: %d", readahead_length);
> -- 
> 2.35.3
> 

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4] readahead02.c: Fix check input fsize

[Cyril Hrubis]

Hi!
> diff --git a/include/tst_device.h b/include/tst_device.h
> index 977427f1c..f03f17f7d 100644
> --- a/include/tst_device.h
> +++ b/include/tst_device.h
> @@ -6,6 +6,8 @@
>  #ifndef TST_DEVICE_H__
>  #define TST_DEVICE_H__
>  
> +#define DEV_SIZE_MB 300u
> +
>  #include <unistd.h>
>  #include <stdint.h>
>  #include <sys/stat.h>
> @@ -49,7 +51,7 @@ int tst_clear_device(const char *dev);
>   * free loopdev). If path is non-NULL, it will be filled with free loopdev path.
>   *
>   */
> -int tst_find_free_loopdev(const char *path, size_t path_len);
> +int tst_find_free_loopdev(char *path, size_t path_len);
>  
>  /*
>   * Attaches a file to a loop device.
> diff --git a/lib/tst_device.c b/lib/tst_device.c
> index 48d7e3ab6..b098fc80b 100644
> --- a/lib/tst_device.c
> +++ b/lib/tst_device.c
> @@ -38,6 +38,7 @@
>  #include "lapi/syscalls.h"
>  #include "test.h"
>  #include "safe_macros.h"
> +#include "tst_device.h"
>  
>  #ifndef LOOP_CTL_GET_FREE
>  # define LOOP_CTL_GET_FREE 0x4C82
> @@ -46,7 +47,6 @@
>  #define LOOP_CONTROL_FILE "/dev/loop-control"
>  
>  #define DEV_FILE "test_dev.img"
> -#define DEV_SIZE_MB 300u
>  #define UUID_STR_SZ 37
>  #define UUID_FMT "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x"

These changes are useless now.

> diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
> index 7acf4bb18..5989c7cbf 100644
> --- a/testcases/kernel/syscalls/readahead/readahead02.c
> +++ b/testcases/kernel/syscalls/readahead/readahead02.c
> @@ -33,6 +33,7 @@
>  #include "tst_test.h"
>  #include "tst_timer.h"
>  #include "lapi/syscalls.h"
> +#include "tst_device.h"
>  
>  static char testfile[PATH_MAX] = "testfile";
>  #define DROP_CACHES_FNAME "/proc/sys/vm/drop_caches"
> @@ -366,11 +367,24 @@ static void setup_readahead_length(void)
>  
>  static void setup(void)
>  {
> -	if (opt_fsizestr) {
> -		testfile_size = SAFE_STRTOL(opt_fsizestr, 1, INT_MAX);
> -		tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
> +	/*
> +	 * Acutaly dev size will reduced after create filesystem,
> +	 * so use dev_szie * 0.8 as dev real usage size, test case will
> +	 * create two files within dev so we need div 2 get max file size
> +	 */
> +	size_t fsize_max = tst_device->size * 0.8 / 2 * 1024 * 1024;

I'm not entirely sure about the * 0.8 here. I suppose that we need some
space for metadata and some manipulation space but 20% is probably way
to much. What motivation is behind the exact number here?

> +	/* At least two pagesize for test case */
> +	pagesize = getpagesize();
> +	size_t fsize_min = pagesize * 2;
> +
> +	if (tst_parse_filesize(opt_fsizestr, (long long *)&testfile_size, fsize_min, fsize_max)) {
                                              ^
					      This is wrong, as long as
					      size of size_t and long
					      long is different it will
					      either corrupt memory or
					      crash.

The testfile_size has to be defined long long there is no way around it.

> +		tst_set_max_runtime(1 + DEFAULT_FILESIZE / (DEFAULT_FILESIZE/32));

Why do we even bother with setting runtime if we are doing
tst_brk(TBROK, ...) on the next line?

> +		tst_brk(TBROK, "invalid initial filesize '%s'", opt_fsizestr);
                                             ^
					I do not understand what
					'initial' means in this context.
>  	}
>  
> +	tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
> +
>  	if (access(PROC_IO_FNAME, F_OK))
>  		tst_brk(TCONF, "Requires " PROC_IO_FNAME);
>  
> @@ -380,7 +394,6 @@ static void setup(void)
>  	/* check if readahead is supported */
>  	tst_syscall(__NR_readahead, 0, 0, 0);
>  
> -	pagesize = getpagesize();
>  
>  	setup_readahead_length();
>  	tst_res(TINFO, "readahead length: %d", readahead_length);
> -- 
> 2.35.3
> 

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4] readahead02.c: Fix check input fsize

[Wei Gao via ltp]

On Mon, Jan 16, 2023 at 02:42:49PM +0100, Cyril Hrubis wrote:
> Hi!
> > diff --git a/include/tst_device.h b/include/tst_device.h
> > index 977427f1c..f03f17f7d 100644
> > --- a/include/tst_device.h
> > +++ b/include/tst_device.h
> > @@ -6,6 +6,8 @@
> >  #ifndef TST_DEVICE_H__
> >  #define TST_DEVICE_H__
> >  
> > +#define DEV_SIZE_MB 300u
> > +
> >  #include <unistd.h>
> >  #include <stdint.h>
> >  #include <sys/stat.h>
> > @@ -49,7 +51,7 @@ int tst_clear_device(const char *dev);
> >   * free loopdev). If path is non-NULL, it will be filled with free loopdev path.
> >   *
> >   */
> > -int tst_find_free_loopdev(const char *path, size_t path_len);
> > +int tst_find_free_loopdev(char *path, size_t path_len);
> >  
> >  /*
> >   * Attaches a file to a loop device.
> > diff --git a/lib/tst_device.c b/lib/tst_device.c
> > index 48d7e3ab6..b098fc80b 100644
> > --- a/lib/tst_device.c
> > +++ b/lib/tst_device.c
> > @@ -38,6 +38,7 @@
> >  #include "lapi/syscalls.h"
> >  #include "test.h"
> >  #include "safe_macros.h"
> > +#include "tst_device.h"
> >  
> >  #ifndef LOOP_CTL_GET_FREE
> >  # define LOOP_CTL_GET_FREE 0x4C82
> > @@ -46,7 +47,6 @@
> >  #define LOOP_CONTROL_FILE "/dev/loop-control"
> >  
> >  #define DEV_FILE "test_dev.img"
> > -#define DEV_SIZE_MB 300u
> >  #define UUID_STR_SZ 37
> >  #define UUID_FMT "%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x"
> 
> These changes are useless now.
ack
> 
> > diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
> > index 7acf4bb18..5989c7cbf 100644
> > --- a/testcases/kernel/syscalls/readahead/readahead02.c
> > +++ b/testcases/kernel/syscalls/readahead/readahead02.c
> > @@ -33,6 +33,7 @@
> >  #include "tst_test.h"
> >  #include "tst_timer.h"
> >  #include "lapi/syscalls.h"
> > +#include "tst_device.h"
> >  
> >  static char testfile[PATH_MAX] = "testfile";
> >  #define DROP_CACHES_FNAME "/proc/sys/vm/drop_caches"
> > @@ -366,11 +367,24 @@ static void setup_readahead_length(void)
> >  
> >  static void setup(void)
> >  {
> > -	if (opt_fsizestr) {
> > -		testfile_size = SAFE_STRTOL(opt_fsizestr, 1, INT_MAX);
> > -		tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
> > +	/*
> > +	 * Acutaly dev size will reduced after create filesystem,
> > +	 * so use dev_szie * 0.8 as dev real usage size, test case will
> > +	 * create two files within dev so we need div 2 get max file size
> > +	 */
> > +	size_t fsize_max = tst_device->size * 0.8 / 2 * 1024 * 1024;
> 
> I'm not entirely sure about the * 0.8 here. I suppose that we need some
> space for metadata and some manipulation space but 20% is probably way
> to much. What motivation is behind the exact number here?
> 

Acutaly dev size will reduced after create filesystem, for example
system set default dev size is 300M but acutal dev size is 280M

"df -h" example resulst in case dev size = 300M:
/dev/loop1      280M  272M     0 100% /tmp/LTP_reaNP2ElW/mntpoint

Around 7% space loss so we use dev_szie * 0.9 as dev real usage size
Test case will create two files within dev so we need div 2 to get
max file size

> > +	/* At least two pagesize for test case */
> > +	pagesize = getpagesize();
> > +	size_t fsize_min = pagesize * 2;
> > +
> > +	if (tst_parse_filesize(opt_fsizestr, (long long *)&testfile_size, fsize_min, fsize_max)) {
>                                               ^
> 					      This is wrong, as long as
> 					      size of size_t and long
> 					      long is different it will
> 					      either corrupt memory or
> 					      crash.
> 
> The testfile_size has to be defined long long there is no way around it.
ack
> 
> > +		tst_set_max_runtime(1 + DEFAULT_FILESIZE / (DEFAULT_FILESIZE/32));
> 
> Why do we even bother with setting runtime if we are doing
> tst_brk(TBROK, ...) on the next line?
ack
> 
> > +		tst_brk(TBROK, "invalid initial filesize '%s'", opt_fsizestr);
>                                              ^
> 					I do not understand what
> 					'initial' means in this context.
> >  	}
> >  
> > +	tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
> > +
> >  	if (access(PROC_IO_FNAME, F_OK))
> >  		tst_brk(TCONF, "Requires " PROC_IO_FNAME);
> >  
> > @@ -380,7 +394,6 @@ static void setup(void)
> >  	/* check if readahead is supported */
> >  	tst_syscall(__NR_readahead, 0, 0, 0);
> >  
> > -	pagesize = getpagesize();
> >  
> >  	setup_readahead_length();
> >  	tst_res(TINFO, "readahead length: %d", readahead_length);
> > -- 
> > 2.35.3
> > 
> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v5] readahead02.c: Fix check input fsize

[Wei Gao via ltp]

We run the test with a loop device it will fail with ENOSPC if we
pass -s bigger than the loop device, we should at least check if
the device is large enough in the test setup.The test should make
use of use tst_parse_filesize() so that we can pass sizes with
units e.g. -s 128M.

Signed-off-by: Wei Gao <wegao@suse.com>
Suggested-by: Cyril Hrubis <chrubis@suse.cz>
---
 .../kernel/syscalls/readahead/readahead02.c   | 64 ++++++++++++-------
 1 file changed, 40 insertions(+), 24 deletions(-)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 7acf4bb18..4770f55e7 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -40,10 +40,10 @@ static char testfile[PATH_MAX] = "testfile";
 #define PROC_IO_FNAME "/proc/self/io"
 #define DEFAULT_FILESIZE (64 * 1024 * 1024)
 
-static size_t testfile_size = DEFAULT_FILESIZE;
+static long long testfile_size = DEFAULT_FILESIZE;
 static char *opt_fsizestr;
 static int pagesize;
-static unsigned long cached_max;
+static long long cached_max;
 static int ovl_mounted;
 static int readahead_length  = 4096;
 static char sys_bdi_ra_path[PATH_MAX];
@@ -51,12 +51,12 @@ static int orig_bdi_limit;
 
 static const char mntpoint[] = OVL_BASE_MNTPOINT;
 
-static int libc_readahead(int fd, off_t offset, size_t len)
+static int libc_readahead(int fd, off_t offset, long long len)
 {
 	return readahead(fd, offset, len);
 }
 
-static int fadvise_willneed(int fd, off_t offset, size_t len)
+static int fadvise_willneed(int fd, off_t offset, long long len)
 {
 	/* Should have the same effect as readahead() syscall */
 	errno = posix_fadvise(fd, offset, len, POSIX_FADV_WILLNEED);
@@ -69,7 +69,7 @@ static struct tcase {
 	int use_overlay;
 	int use_fadvise;
 	/* Use either readahead() syscall or POSIX_FADV_WILLNEED */
-	int (*readahead)(int fd, off_t offset, size_t len);
+	int (*readahead)(int fd, off_t offset, long long len);
 } tcases[] = {
 	{ "readahead on file", 0, 0, libc_readahead },
 	{ "readahead on overlayfs file", 1, 0, libc_readahead },
@@ -121,11 +121,11 @@ static void create_testfile(int use_overlay)
 {
 	int fd;
 	char *tmp;
-	size_t i;
+	long long i;
 
 	sprintf(testfile, "%s/testfile",
 		use_overlay ? OVL_MNT : OVL_BASE_MNTPOINT);
-	tst_res(TINFO, "creating test file of size: %zu", testfile_size);
+	tst_res(TINFO, "creating test file of size: %lli", testfile_size);
 	tmp = SAFE_MALLOC(pagesize);
 
 	/* round to page size */
@@ -151,12 +151,12 @@ static void create_testfile(int use_overlay)
  * @cached: returns cached kB from /proc/meminfo
  */
 static int read_testfile(struct tcase *tc, int do_readahead,
-			 const char *fname, size_t fsize,
-			 unsigned long *read_bytes, long long *usec,
-			 unsigned long *cached)
+			 const char *fname, long long fsize,
+			 long long *read_bytes, long long *usec,
+			 long long *cached)
 {
 	int fd;
-	size_t i = 0;
+	long long i = 0;
 	long read_bytes_start;
 	unsigned char *p, tmp;
 	off_t offset = 0;
@@ -173,8 +173,8 @@ static int read_testfile(struct tcase *tc, int do_readahead,
 
 			i++;
 			offset += readahead_length;
-		} while ((size_t)offset < fsize);
-		tst_res(TINFO, "readahead calls made: %zu", i);
+		} while ((long long)offset < fsize);
+		tst_res(TINFO, "readahead calls made: %lli", i);
 		*cached = get_cached_size();
 
 		/* offset of file shouldn't change after readahead */
@@ -214,9 +214,9 @@ static int read_testfile(struct tcase *tc, int do_readahead,
 
 static void test_readahead(unsigned int n)
 {
-	unsigned long read_bytes, read_bytes_ra;
+	long long read_bytes, read_bytes_ra;
 	long long usec, usec_ra;
-	unsigned long cached_high, cached_low, cached, cached_ra;
+	long long cached_high, cached_low, cached, cached_ra;
 	int ret;
 	struct tcase *tc = &tcases[n];
 
@@ -286,8 +286,8 @@ static void test_readahead(unsigned int n)
 	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",
+		tst_res(TINFO, "read_testfile(0) read: %lli bytes", read_bytes);
+		tst_res(TINFO, "read_testfile(1) read: %lli bytes",
 			read_bytes_ra);
 		/* actual number of read bytes depends on total RAM */
 		if (read_bytes_ra < read_bytes)
@@ -299,9 +299,9 @@ static void test_readahead(unsigned int n)
 			" unable to determine read bytes during test");
 	}
 
-	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);
+	tst_res(TINFO, "cache can hold at least: %lli kB", cached_max);
+	tst_res(TINFO, "read_testfile(0) used cache: %lli kB", cached);
+	tst_res(TINFO, "read_testfile(1) used cache: %lli kB", cached_ra);
 
 	if (cached_max * 1024 >= testfile_size) {
 		/*
@@ -366,11 +366,28 @@ static void setup_readahead_length(void)
 
 static void setup(void)
 {
-	if (opt_fsizestr) {
-		testfile_size = SAFE_STRTOL(opt_fsizestr, 1, INT_MAX);
-		tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
+	/*
+	 * Acutaly dev size will reduced after create filesystem, for example
+	 * system set default dev size is 300M but acutal dev size is 280M
+	 * "df -h" example resulst in case dev size = 300M:
+	 * /dev/loop1      280M  272M     0 100% /tmp/LTP_reaNP2ElW/mntpoint
+	 * Around 7% space loss so we use dev_szie * 0.9 as dev real usage size
+	 * Test case will create two files within dev so we need div 2 to get
+	 * max file size
+	 */
+
+	long long fsize_max = tst_device->size * 0.9 / 2 * 1024 * 1024;
+
+	/* At least two pagesize for test case */
+	pagesize = getpagesize();
+	long long fsize_min = pagesize * 2;
+
+	if (tst_parse_filesize(opt_fsizestr, &testfile_size, fsize_min, fsize_max)) {
+		tst_brk(TBROK, "invalid input filesize '%s'", opt_fsizestr);
 	}
 
+	tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
+
 	if (access(PROC_IO_FNAME, F_OK))
 		tst_brk(TCONF, "Requires " PROC_IO_FNAME);
 
@@ -380,7 +397,6 @@ static void setup(void)
 	/* check if readahead is supported */
 	tst_syscall(__NR_readahead, 0, 0, 0);
 
-	pagesize = getpagesize();
 
 	setup_readahead_length();
 	tst_res(TINFO, "readahead length: %d", readahead_length);
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] readahead02.c fixes: use tst_parse_filesize() so that we can pass sizes with units e.g. -s 128M

[Richard Palethorpe]

Hello,

coolgw <coolgw1126@gmail.com> writes:

> Signed-off-by: WEI GAO <wegao@suse.com>
> ---
>  testcases/kernel/syscalls/readahead/readahead02.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
> index 3ed88c005..c282b4d68 100644
> --- a/testcases/kernel/syscalls/readahead/readahead02.c
> +++ b/testcases/kernel/syscalls/readahead/readahead02.c
> @@ -367,8 +367,8 @@ static void setup_readahead_length(void)
>  
>  static void setup(void)
>  {
> -	if (opt_fsizestr) {
> -		testfile_size = SAFE_STRTOL(opt_fsizestr, 1, INT_MAX);
> +        if (tst_parse_filesize(opt_fsizestr, &testfile_size, 1, INT_MAX)) {
> +		tst_brk(TBROK, "invalid initial filesize '%s'", opt_fsizestr);

So far correct; if parsing fails we call tst_brk which redirects to cleanup.

>  		tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));

However this will never be called because it is on the error path.

The time which we give the test to execute is proportional to the file
size (if the size is non-default!). If it were not then large file sizes
would cause timeouts (possibly at random) which would result in
regressions.

Also note that in the existing test we only use tst_set_max_runtime when
the value is non default. This is a mistake, but luckily the
test.max_runtime = 30 and the calculated runtime for the default size
would be 33. So you are not likely to cause a regression by changing
this.

Setting to changes requested in patchwork.

>  	}
>  
> -- 
> 2.35.3


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v1] readahead02.c fixes: use tst_parse_filesize() so that we can pass sizes with units e.g. -s 128M

[coolgw]

Signed-off-by: WEI GAO <wegao@suse.com>
---
 testcases/kernel/syscalls/readahead/readahead02.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 3ed88c005..c282b4d68 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -367,8 +367,8 @@ static void setup_readahead_length(void)
 
 static void setup(void)
 {
-	if (opt_fsizestr) {
-		testfile_size = SAFE_STRTOL(opt_fsizestr, 1, INT_MAX);
+        if (tst_parse_filesize(opt_fsizestr, &testfile_size, 1, INT_MAX)) {
+		tst_brk(TBROK, "invalid initial filesize '%s'", opt_fsizestr);
 		tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
 	}
 
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v1] readahead02.c fixes: use tst_parse_filesize() so that we can pass sizes with units e.g. -s 128M

[coolgw]

Signed-off-by: WEI GAO <wegao@suse.com>
---
 testcases/kernel/syscalls/readahead/readahead02.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 3ed88c005..c282b4d68 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -367,8 +367,8 @@ static void setup_readahead_length(void)
 
 static void setup(void)
 {
-	if (opt_fsizestr) {
-		testfile_size = SAFE_STRTOL(opt_fsizestr, 1, INT_MAX);
+        if (tst_parse_filesize(opt_fsizestr, &testfile_size, 1, INT_MAX)) {
+		tst_brk(TBROK, "invalid initial filesize '%s'", opt_fsizestr);
 		tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
 	}
 
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v1] readahead02.c fixes: use tst_parse_filesize() so that we can pass sizes with units e.g. -s 128M

[coolgw]

Signed-off-by: WEI GAO <wegao@suse.com>
---
 testcases/kernel/syscalls/readahead/readahead02.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/readahead/readahead02.c b/testcases/kernel/syscalls/readahead/readahead02.c
index 3ed88c005..c282b4d68 100644
--- a/testcases/kernel/syscalls/readahead/readahead02.c
+++ b/testcases/kernel/syscalls/readahead/readahead02.c
@@ -367,8 +367,8 @@ static void setup_readahead_length(void)
 
 static void setup(void)
 {
-	if (opt_fsizestr) {
-		testfile_size = SAFE_STRTOL(opt_fsizestr, 1, INT_MAX);
+        if (tst_parse_filesize(opt_fsizestr, &testfile_size, 1, INT_MAX)) {
+		tst_brk(TBROK, "invalid initial filesize '%s'", opt_fsizestr);
 		tst_set_max_runtime(1 + testfile_size / (DEFAULT_FILESIZE/32));
 	}
 
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp