[LTP] [PATCH v1] Reduce needed memory by the dio_read test

[LTP] [PATCH v1] Reduce needed memory by the dio_read test

[Andrea Cervesato]

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
---
 runtest/ltp-aiodio.part4 | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/runtest/ltp-aiodio.part4 b/runtest/ltp-aiodio.part4
index fc1e1d479..8c10f67fb 100644
--- a/runtest/ltp-aiodio.part4
+++ b/runtest/ltp-aiodio.part4
@@ -62,7 +62,7 @@ DIT002 dio_truncate
 #Running read_checkzero
 #gread_checkzero
 #Running dio_read
-DOR000 dio_read -n 1 -i 100
-DOR001 dio_read -n 10 -i 30
-DOR002 dio_read -n 20 -i 15
-DOR003 dio_read -n 100 -i 4
+DOR000 dio_read -n 1 -i 100 -r 512k -w 512k -s 5M
+DOR001 dio_read -n 10 -i 30 -r 512k -w 512k -s 5M
+DOR002 dio_read -n 20 -i 15 -r 512k -w 512k -s 5M
+DOR003 dio_read -n 100 -i 4 -r 512k -w 512k -s 5M
-- 
2.34.1


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

Re: [LTP] [PATCH v1] Reduce needed memory by the dio_read test

[Martin Doucha]

Hi,
Acked-by: Martin Doucha <mdoucha@suse.cz>

On 20. 01. 22 16:04, Andrea Cervesato wrote:
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
> ---
>  runtest/ltp-aiodio.part4 | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/runtest/ltp-aiodio.part4 b/runtest/ltp-aiodio.part4
> index fc1e1d479..8c10f67fb 100644
> --- a/runtest/ltp-aiodio.part4
> +++ b/runtest/ltp-aiodio.part4
> @@ -62,7 +62,7 @@ DIT002 dio_truncate
>  #Running read_checkzero
>  #gread_checkzero
>  #Running dio_read
> -DOR000 dio_read -n 1 -i 100
> -DOR001 dio_read -n 10 -i 30
> -DOR002 dio_read -n 20 -i 15
> -DOR003 dio_read -n 100 -i 4
> +DOR000 dio_read -n 1 -i 100 -r 512k -w 512k -s 5M
> +DOR001 dio_read -n 10 -i 30 -r 512k -w 512k -s 5M
> +DOR002 dio_read -n 20 -i 15 -r 512k -w 512k -s 5M
> +DOR003 dio_read -n 100 -i 4 -r 512k -w 512k -s 5M


-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic

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

Re: [LTP] [PATCH v1] Reduce needed memory by the dio_read test

[Cyril Hrubis]

Hi!
> Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
> ---
>  runtest/ltp-aiodio.part4 | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/runtest/ltp-aiodio.part4 b/runtest/ltp-aiodio.part4
> index fc1e1d479..8c10f67fb 100644
> --- a/runtest/ltp-aiodio.part4
> +++ b/runtest/ltp-aiodio.part4
> @@ -62,7 +62,7 @@ DIT002 dio_truncate
>  #Running read_checkzero
>  #gread_checkzero
>  #Running dio_read
> -DOR000 dio_read -n 1 -i 100
> -DOR001 dio_read -n 10 -i 30
> -DOR002 dio_read -n 20 -i 15
> -DOR003 dio_read -n 100 -i 4
> +DOR000 dio_read -n 1 -i 100 -r 512k -w 512k -s 5M
> +DOR001 dio_read -n 10 -i 30 -r 512k -w 512k -s 5M
> +DOR002 dio_read -n 20 -i 15 -r 512k -w 512k -s 5M
> +DOR003 dio_read -n 100 -i 4 -r 512k -w 512k -s 5M

Limiting the child buffer to 512k makes sense, at least in the cases
where n > 1 as by default the child allocates 32MB buffer, with 100
children that makes up to ~3GB. Also this really should have been in the
patch description, which should really describe why the changes have
been done.

But I'm not sure about the -s 5M part, the file size was always 128MB
before and reducing it to 5M sounds like too much. On my machine that
reduces the testing time from about one minute to a few seconds, that sounds
like change that is too radical without further explanation. If we want
to run the test with smaller file size I would go for 64M, that reduces
the runtime to half without reducing the changes for triggering a bug
too much.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v1] Reduce needed memory by the dio_read test

[Petr Vorel]

Hi all,

> Hi!
> > Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.de>
> > ---
> >  runtest/ltp-aiodio.part4 | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)

> > diff --git a/runtest/ltp-aiodio.part4 b/runtest/ltp-aiodio.part4
> > index fc1e1d479..8c10f67fb 100644
> > --- a/runtest/ltp-aiodio.part4
> > +++ b/runtest/ltp-aiodio.part4
> > @@ -62,7 +62,7 @@ DIT002 dio_truncate
> >  #Running read_checkzero
> >  #gread_checkzero
> >  #Running dio_read
> > -DOR000 dio_read -n 1 -i 100
> > -DOR001 dio_read -n 10 -i 30
> > -DOR002 dio_read -n 20 -i 15
> > -DOR003 dio_read -n 100 -i 4
> > +DOR000 dio_read -n 1 -i 100 -r 512k -w 512k -s 5M
> > +DOR001 dio_read -n 10 -i 30 -r 512k -w 512k -s 5M
> > +DOR002 dio_read -n 20 -i 15 -r 512k -w 512k -s 5M
> > +DOR003 dio_read -n 100 -i 4 -r 512k -w 512k -s 5M

> Limiting the child buffer to 512k makes sense, at least in the cases
> where n > 1 as by default the child allocates 32MB buffer, with 100
> children that makes up to ~3GB. Also this really should have been in the
> patch description, which should really describe why the changes have
> been done.

> But I'm not sure about the -s 5M part, the file size was always 128MB
> before and reducing it to 5M sounds like too much. On my machine that
> reduces the testing time from about one minute to a few seconds, that sounds
> like change that is too radical without further explanation. If we want
> to run the test with smaller file size I would go for 64M, that reduces
> the runtime to half without reducing the changes for triggering a bug
> too much.
Good catch.

I'm also going to verify ADSP075 dio_sparse  -w 518192k -s 518192k -n 1000
from ltp-aiodio.part2. Cyril's suggestion is to use '-n 1' to emulate
only single children in previous implementation.

Kind regards,
Petr

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

Re: [LTP] [PATCH v1] Reduce needed memory by the dio_read test

[Andrea Cervesato via ltp]

[-- Attachment #1.1: Type: text/plain, Size: 1817 bytes --]

Hi!

By changing the file size to 32M, test requires 2 minutes to complete on 
my machine using ~51MB memory allocation for 100 threads. We can go for 
it. I'll also write a little explanation to the patch.

Andrea

On 1/20/22 16:31, Cyril Hrubis wrote:
> Hi!
>> Signed-off-by: Andrea Cervesato<andrea.cervesato@suse.de>
>> ---
>>   runtest/ltp-aiodio.part4 | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/runtest/ltp-aiodio.part4 b/runtest/ltp-aiodio.part4
>> index fc1e1d479..8c10f67fb 100644
>> --- a/runtest/ltp-aiodio.part4
>> +++ b/runtest/ltp-aiodio.part4
>> @@ -62,7 +62,7 @@ DIT002 dio_truncate
>>   #Running read_checkzero
>>   #gread_checkzero
>>   #Running dio_read
>> -DOR000 dio_read -n 1 -i 100
>> -DOR001 dio_read -n 10 -i 30
>> -DOR002 dio_read -n 20 -i 15
>> -DOR003 dio_read -n 100 -i 4
>> +DOR000 dio_read -n 1 -i 100 -r 512k -w 512k -s 5M
>> +DOR001 dio_read -n 10 -i 30 -r 512k -w 512k -s 5M
>> +DOR002 dio_read -n 20 -i 15 -r 512k -w 512k -s 5M
>> +DOR003 dio_read -n 100 -i 4 -r 512k -w 512k -s 5M
> Limiting the child buffer to 512k makes sense, at least in the cases
> where n > 1 as by default the child allocates 32MB buffer, with 100
> children that makes up to ~3GB. Also this really should have been in the
> patch description, which should really describe why the changes have
> been done.
>
> But I'm not sure about the -s 5M part, the file size was always 128MB
> before and reducing it to 5M sounds like too much. On my machine that
> reduces the testing time from about one minute to a few seconds, that sounds
> like change that is too radical without further explanation. If we want
> to run the test with smaller file size I would go for 64M, that reduces
> the runtime to half without reducing the changes for triggering a bug
> too much.
>

[-- Attachment #1.2: Type: text/html, Size: 2395 bytes --]

[-- Attachment #2: Type: text/plain, Size: 60 bytes --]


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

Re: [LTP] [PATCH v1] Reduce needed memory by the dio_read test

[Petr Vorel]

Hi all,

...
> > But I'm not sure about the -s 5M part, the file size was always 128MB
> > before and reducing it to 5M sounds like too much. On my machine that
> > reduces the testing time from about one minute to a few seconds, that sounds
> > like change that is too radical without further explanation. If we want
> > to run the test with smaller file size I would go for 64M, that reduces
> > the runtime to half without reducing the changes for triggering a bug
> > too much.
> Good catch.

> I'm also going to verify ADSP075 dio_sparse  -w 518192k -s 518192k -n 1000
> from ltp-aiodio.part2. Cyril's suggestion is to use '-n 1' to emulate
> only single children in previous implementation.
No, this is wrong. Also the issue can be just a timeout.
ADSP074 (uses -n 512) runs framework ~3.5 min, ADSP075 (uses -n 1000) gets killed
by LTP timeout. Do we want to increase tst_test.timeout for test?

Am I correct that we cannot increase based on particular input (for particular
getopt value) as we cannot later update results.timeout during run?

Kind regards,
Petr

> Kind regards,
> Petr

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

Re: [LTP] [PATCH v1] Reduce needed memory by the dio_read test

[Cyril Hrubis]

Hi!
> No, this is wrong. Also the issue can be just a timeout.
> ADSP074 (uses -n 512) runs framework ~3.5 min, ADSP075 (uses -n 1000) gets killed
> by LTP timeout. Do we want to increase tst_test.timeout for test?
> 
> Am I correct that we cannot increase based on particular input (for particular
> getopt value) as we cannot later update results.timeout during run?

There actually is tst_set_timeout() however in this case I wouldn't
bother because the test runtime largerly depends on the actuall speed of
the disk it is writing to and other parameters of the test such as block
size and the actual formula would be quite complex.

Also I do plan to fix this properly by V2 of the runtime patchset once
the release is finished, so I would go for minimal fix just before the
release which would be setting tst_test.timeout to sufficiently large
value.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v1] Reduce needed memory by the dio_read test

[Cyril Hrubis]

Hi!
> Also I do plan to fix this properly by V2 of the runtime patchset once
> the release is finished, so I would go for minimal fix just before the
> release which would be setting tst_test.timeout to sufficiently large
> value.

And I guess that we should do that for all the rewritten aio tests.

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH v1] Reduce needed memory by the dio_read test

[Petr Vorel]

Hi Cyril,
> Hi!
> > Also I do plan to fix this properly by V2 of the runtime patchset once
> > the release is finished, so I would go for minimal fix just before the
> > release which would be setting tst_test.timeout to sufficiently large
> > value.

> And I guess that we should do that for all the rewritten aio tests.
Sure, agree. Would be .timeout = 1800 enough (30 min)?
I can send a patch based on my last patch "aiodio: Skip tests on tmpfs" [1]

Kind regards,
Petr

[1] https://lore.kernel.org/ltp/YeqIB+Y4rO2XK57i@pevik/T/#t

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