* [LTP] [PATCH v1] readahead02.c fixes: use tst_parse_filesize() so that we can pass sizes with units e.g. -s 128M
@ 2023-01-09 3:27 coolgw
2023-01-10 10:01 ` Richard Palethorpe
0 siblings, 1 reply; 6+ messages in thread
From: coolgw @ 2023-01-09 3:27 UTC (permalink / raw)
To: wegao, ltp
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [LTP] [PATCH v1] readahead02.c fixes: use tst_parse_filesize() so that we can pass sizes with units e.g. -s 128M
2023-01-09 3:27 [LTP] [PATCH v1] readahead02.c fixes: use tst_parse_filesize() so that we can pass sizes with units e.g. -s 128M coolgw
@ 2023-01-10 10:01 ` Richard Palethorpe
0 siblings, 0 replies; 6+ messages in thread
From: Richard Palethorpe @ 2023-01-10 10:01 UTC (permalink / raw)
To: coolgw; +Cc: ltp
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [LTP] [PATCH v1] readahead02.c fixes: use tst_parse_filesize() so that we can pass sizes with units e.g. -s 128M
@ 2023-01-09 3:28 coolgw
0 siblings, 0 replies; 6+ messages in thread
From: coolgw @ 2023-01-09 3:28 UTC (permalink / raw)
To: wegao, ltp
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [LTP] [PATCH v1] readahead02.c fixes: use tst_parse_filesize() so that we can pass sizes with units e.g. -s 128M
@ 2023-01-09 5:06 coolgw
0 siblings, 0 replies; 6+ messages in thread
From: coolgw @ 2023-01-09 5:06 UTC (permalink / raw)
To: wegao, ltp
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [LTP] [PATCH v1] readahead02.c fixes: use tst_parse_filesize() so that we can pass sizes with units e.g. -s 128M
@ 2023-01-09 5:12 coolgw
2023-01-10 9:16 ` Petr Vorel
0 siblings, 1 reply; 6+ messages in thread
From: coolgw @ 2023-01-09 5:12 UTC (permalink / raw)
To: ltp
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [LTP] [PATCH v1] readahead02.c fixes: use tst_parse_filesize() so that we can pass sizes with units e.g. -s 128M
2023-01-09 5:12 coolgw
@ 2023-01-10 9:16 ` Petr Vorel
0 siblings, 0 replies; 6+ messages in thread
From: Petr Vorel @ 2023-01-10 9:16 UTC (permalink / raw)
To: coolgw; +Cc: ltp
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-10 10:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-09 3:27 [LTP] [PATCH v1] readahead02.c fixes: use tst_parse_filesize() so that we can pass sizes with units e.g. -s 128M coolgw
2023-01-10 10:01 ` Richard Palethorpe
-- strict thread matches above, loose matches on Subject: below --
2023-01-09 3:28 coolgw
2023-01-09 5:06 coolgw
2023-01-09 5:12 coolgw
2023-01-10 9:16 ` Petr Vorel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox