[LTP] [PATCH v2 0/2] Fix dio_append test

[LTP] [PATCH v2 0/2] Fix dio_append test

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

This patch-set is meant to fix dio_append issues and to increase number of
operations which are performed by it.

Andrea Cervesato (2):
  Fix dio_append/aiodio_append tests
  Increase default appends operations in dio_append

 testcases/kernel/io/ltp-aiodio/common.h     | 12 ++++++++++--
 testcases/kernel/io/ltp-aiodio/dio_append.c |  7 +++++--
 2 files changed, 15 insertions(+), 4 deletions(-)

-- 
2.35.3


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

[LTP] [PATCH v2 1/2] Fix dio_append/aiodio_append tests

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

Ensure that dio_append and aiodio_append will end all children if
parent asked for it. The way we have to do it, is to ensure that
*run_child variable is checked before opening the file to read.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 testcases/kernel/io/ltp-aiodio/common.h | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/io/ltp-aiodio/common.h b/testcases/kernel/io/ltp-aiodio/common.h
index 200bbe18e..9a2d27166 100644
--- a/testcases/kernel/io/ltp-aiodio/common.h
+++ b/testcases/kernel/io/ltp-aiodio/common.h
@@ -62,8 +62,12 @@ static inline void io_read(const char *filename, int filesize, volatile int *run
 	int i;
 	int r;
 
-	while ((fd = open(filename, O_RDONLY, 0666)) < 0)
+	while ((fd = open(filename, O_RDONLY, 0666)) < 0) {
+		if (!*run_child)
+			return;
+
 		usleep(100);
+	}
 
 	tst_res(TINFO, "child %i reading file", getpid());
 
@@ -102,8 +106,12 @@ static inline void io_read_eof(const char *filename, volatile int *run_child)
 	int fd;
 	int r;
 
-	while ((fd = open(filename, O_RDONLY, 0666)) < 0)
+	while ((fd = open(filename, O_RDONLY, 0666)) < 0) {
+		if (!*run_child)
+			return;
+
 		usleep(100);
+	}
 
 	tst_res(TINFO, "child %i reading file", getpid());
 
-- 
2.35.3


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

[LTP] [PATCH v2 2/2] Increase default appends operations in dio_append

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

This test is currently ending quite fast on regular systems, so we
increase the number of operations in order to have bigger chances to
find bugs.

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 testcases/kernel/io/ltp-aiodio/dio_append.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/io/ltp-aiodio/dio_append.c b/testcases/kernel/io/ltp-aiodio/dio_append.c
index 057ae73d9..0ecb76e2f 100644
--- a/testcases/kernel/io/ltp-aiodio/dio_append.c
+++ b/testcases/kernel/io/ltp-aiodio/dio_append.c
@@ -33,7 +33,7 @@ static void setup(void)
 {
 	numchildren = 16;
 	writesize = 64 * 1024;
-	appends = 1000;
+	appends = 10000;
 
 	if (tst_parse_int(str_numchildren, &numchildren, 1, INT_MAX))
 		tst_brk(TBROK, "Invalid number of children '%s'", str_numchildren);
@@ -44,6 +44,9 @@ static void setup(void)
 	if (tst_parse_int(str_appends, &appends, 1, INT_MAX))
 		tst_brk(TBROK, "Invalid number of appends '%s'", str_appends);
 
+	if (!tst_fs_has_free(".", appends, writesize))
+		tst_brk(TCONF, "Not enough space to run the test");
+
 	run_child = SAFE_MMAP(NULL, sizeof(int), PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0);
 }
 
@@ -97,7 +100,7 @@ static struct tst_test test = {
 	.options = (struct tst_option[]) {
 		{"n:", &str_numchildren, "Number of processes (default 16)"},
 		{"w:", &str_writesize, "Write size for each append (default 64K)"},
-		{"c:", &str_appends, "Number of appends (default 1000)"},
+		{"c:", &str_appends, "Number of appends (default 10000)"},
 		{}
 	},
 	.skip_filesystems = (const char *[]) {
-- 
2.35.3


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

Re: [LTP] [PATCH v2 1/2] Fix dio_append/aiodio_append tests

[Petr Vorel]

> From: Andrea Cervesato <andrea.cervesato@suse.com>

> Ensure that dio_append and aiodio_append will end all children if
> parent asked for it. The way we have to do it, is to ensure that
> *run_child variable is checked before opening the file to read.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 0/2] Fix dio_append test

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 2/2] Increase default appends operations in dio_append

[Petr Vorel]

Hi Andrea,

> From: Andrea Cervesato <andrea.cervesato@suse.com>

> This test is currently ending quite fast on regular systems, so we
> increase the number of operations in order to have bigger chances to
> find bugs.

> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
> ---
>  testcases/kernel/io/ltp-aiodio/dio_append.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)

> diff --git a/testcases/kernel/io/ltp-aiodio/dio_append.c b/testcases/kernel/io/ltp-aiodio/dio_append.c
> index 057ae73d9..0ecb76e2f 100644
> --- a/testcases/kernel/io/ltp-aiodio/dio_append.c
> +++ b/testcases/kernel/io/ltp-aiodio/dio_append.c
> @@ -33,7 +33,7 @@ static void setup(void)
>  {
>  	numchildren = 16;
>  	writesize = 64 * 1024;
> -	appends = 1000;
> +	appends = 10000;

>  	if (tst_parse_int(str_numchildren, &numchildren, 1, INT_MAX))
>  		tst_brk(TBROK, "Invalid number of children '%s'", str_numchildren);
> @@ -44,6 +44,9 @@ static void setup(void)
>  	if (tst_parse_int(str_appends, &appends, 1, INT_MAX))
>  		tst_brk(TBROK, "Invalid number of appends '%s'", str_appends);

> +	if (!tst_fs_has_free(".", appends, writesize))
> +		tst_brk(TCONF, "Not enough space to run the test");

I'm ok to keep this in single commit with increasing the number of operations,
but it should have been at least mentioned in the commit message.

And, if I understand correctly, this was meant by Cyril in v1
https://lore.kernel.org/ltp/ZbjpATp6cK9AkvBm@yuki/

Kind regards,
Petr

> +
>  	run_child = SAFE_MMAP(NULL, sizeof(int), PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0);
>  }

> @@ -97,7 +100,7 @@ static struct tst_test test = {
>  	.options = (struct tst_option[]) {
>  		{"n:", &str_numchildren, "Number of processes (default 16)"},
>  		{"w:", &str_writesize, "Write size for each append (default 64K)"},
> -		{"c:", &str_appends, "Number of appends (default 1000)"},
> +		{"c:", &str_appends, "Number of appends (default 10000)"},
>  		{}
>  	},
>  	.skip_filesystems = (const char *[]) {

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

Re: [LTP] [PATCH v2 2/2] Increase default appends operations in dio_append

[Petr Vorel]

Hi Andrea, Cyril,

merged. Thanks!

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 2/2] Increase default appends operations in dio_append

[Cyril Hrubis]

Hi!
> I'm ok to keep this in single commit with increasing the number of operations,
> but it should have been at least mentioned in the commit message.

The commit changelog should mention the addition of the function to
check the available space, but otherwise it's ok.

> And, if I understand correctly, this was meant by Cyril in v1
> https://lore.kernel.org/ltp/ZbjpATp6cK9AkvBm@yuki/

I mostly object to having two unrelated chnages in a single commit,
functional fix shouldn't be cobbled togegher with a change that adjusts
default parameters.

Also the check for a free space should be ideally added into all I/O
tests eventually.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v2 2/2] Increase default appends operations in dio_append

[Petr Vorel]

> Hi!
> > I'm ok to keep this in single commit with increasing the number of operations,
> > but it should have been at least mentioned in the commit message.

> The commit changelog should mention the addition of the function to
> check the available space, but otherwise it's ok.

> > And, if I understand correctly, this was meant by Cyril in v1
> > https://lore.kernel.org/ltp/ZbjpATp6cK9AkvBm@yuki/

> I mostly object to having two unrelated chnages in a single commit,
> functional fix shouldn't be cobbled togegher with a change that adjusts
> default parameters.

I figured that out, that's why I merged before you wrote. I'm sorry for the
noise.

> Also the check for a free space should be ideally added into all I/O
> tests eventually.

+1

Kind regards,
Petr

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