[LTP] [PATCH 1/1] cachestat01: Reduce required space on 64kb page size

[LTP] [PATCH 1/1] cachestat01: Reduce required space on 64kb page size

[Petr Vorel]

ppc64le (6.10.1 on Tumbleweed) fails due 64kb page size requiring more
space:

cachestat01.c:39: TINFO: Number of pages: 4096
cachestat01.c:56: TPASS: cachestat(fd, cs_range, cs, 0) passed
cachestat01.c:59: TPASS: cs->nr_cache + cs->nr_evicted == num_pages (4096)
cachestat01.c:38: TINFO: Disable file synchronization
cachestat01.c:39: TINFO: Number of pages: 8192
cachestat01.c:46: TBROK: write(3,0x1000ddb0aa0,65536) failed: ENOSPC (28)

Fixes: 93b28ee69d ("Add cachestat01 test")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
First I thought we should use tst_fs_has_free() in each
test_cached_pages() run, but test uses .all_filesystems, thus it's
always 300 MB. Should we, just in case, use .dev_min_size = 300,
to make sure it runs correctly even we change the DEV_SIZE_MB value?

Or, should we really calculate required space with tst_fs_has_free(),
based on page size num_pages and * 1.1 (reserved space for metadata)?

Kind regards,
Petr

 testcases/kernel/syscalls/cachestat/cachestat01.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/cachestat/cachestat01.c b/testcases/kernel/syscalls/cachestat/cachestat01.c
index f7f6275cbd..edb6335d9e 100644
--- a/testcases/kernel/syscalls/cachestat/cachestat01.c
+++ b/testcases/kernel/syscalls/cachestat/cachestat01.c
@@ -67,7 +67,7 @@ static void test_cached_pages(const unsigned int use_sync, const int num_pages)
 
 static void run(unsigned int use_sync)
 {
-	for (int i = 0; i < 15; i++)
+	for (int i = 0; i < page_size > 4096 ? 13 : 15; i++)
 		test_cached_pages(use_sync, 1 << i);
 }
 
-- 
2.45.2


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

Re: [LTP] [PATCH 1/1] cachestat01: Reduce required space on 64kb page size

[Petr Vorel]

...
> First I thought we should use tst_fs_has_free() in each
> test_cached_pages() run, but test uses .all_filesystems, thus it's
> always 300 MB. Should we, just in case, use .dev_min_size = 300,
> to make sure it runs correctly even we change the DEV_SIZE_MB value?

Or for people who use custom block device passed via $LTP_DEV.

Kind regards,
Petr

> Or, should we really calculate required space with tst_fs_has_free(),
> based on page size num_pages and * 1.1 (reserved space for metadata)?

> Kind regards,
> Petr

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

Re: [LTP] [PATCH 1/1] cachestat01: Reduce required space on 64kb page size

[Cyril Hrubis]

Hi!
> cachestat01.c:39: TINFO: Number of pages: 4096
> cachestat01.c:56: TPASS: cachestat(fd, cs_range, cs, 0) passed
> cachestat01.c:59: TPASS: cs->nr_cache + cs->nr_evicted == num_pages (4096)
> cachestat01.c:38: TINFO: Disable file synchronization
> cachestat01.c:39: TINFO: Number of pages: 8192
> cachestat01.c:46: TBROK: write(3,0x1000ddb0aa0,65536) failed: ENOSPC (28)
> 
> Fixes: 93b28ee69d ("Add cachestat01 test")
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> First I thought we should use tst_fs_has_free() in each
> test_cached_pages() run, but test uses .all_filesystems, thus it's
> always 300 MB. Should we, just in case, use .dev_min_size = 300,
> to make sure it runs correctly even we change the DEV_SIZE_MB value?
> 
> Or, should we really calculate required space with tst_fs_has_free(),
> based on page size num_pages and * 1.1 (reserved space for metadata)?

Hmm, I guess that we should calculate how many pages are going to fit
into the filesystem and cap it on a sane number as well, so that the
test runs for a reasonable time if user passed a real and big block
device, so something as:

num_pages = MIN(tst_device->size/(2*page_size), 15);

>  testcases/kernel/syscalls/cachestat/cachestat01.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/testcases/kernel/syscalls/cachestat/cachestat01.c b/testcases/kernel/syscalls/cachestat/cachestat01.c
> index f7f6275cbd..edb6335d9e 100644
> --- a/testcases/kernel/syscalls/cachestat/cachestat01.c
> +++ b/testcases/kernel/syscalls/cachestat/cachestat01.c
> @@ -67,7 +67,7 @@ static void test_cached_pages(const unsigned int use_sync, const int num_pages)
>  
>  static void run(unsigned int use_sync)
>  {
> -	for (int i = 0; i < 15; i++)
> +	for (int i = 0; i < page_size > 4096 ? 13 : 15; i++)

Uff, this is starting to look like perl...

>  		test_cached_pages(use_sync, 1 << i);
>  }
>  
> -- 
> 2.45.2
> 

-- 
Cyril Hrubis
chrubis@suse.cz

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

Re: [LTP] [PATCH 1/1] cachestat01: Reduce required space on 64kb page size

[Petr Vorel]

Hi Cyril,

...
> > First I thought we should use tst_fs_has_free() in each
> > test_cached_pages() run, but test uses .all_filesystems, thus it's
> > always 300 MB. Should we, just in case, use .dev_min_size = 300,
> > to make sure it runs correctly even we change the DEV_SIZE_MB value?

> > Or, should we really calculate required space with tst_fs_has_free(),
> > based on page size num_pages and * 1.1 (reserved space for metadata)?

> Hmm, I guess that we should calculate how many pages are going to fit
> into the filesystem and cap it on a sane number as well, so that the
> test runs for a reasonable time if user passed a real and big block
> device, so something as:

> num_pages = MIN(tst_device->size/(2*page_size), 15);

Thanks! Sounds reasonable, I'll retest.

> >  testcases/kernel/syscalls/cachestat/cachestat01.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)

> > diff --git a/testcases/kernel/syscalls/cachestat/cachestat01.c b/testcases/kernel/syscalls/cachestat/cachestat01.c
> > index f7f6275cbd..edb6335d9e 100644
> > --- a/testcases/kernel/syscalls/cachestat/cachestat01.c
> > +++ b/testcases/kernel/syscalls/cachestat/cachestat01.c
> > @@ -67,7 +67,7 @@ static void test_cached_pages(const unsigned int use_sync, const int num_pages)

> >  static void run(unsigned int use_sync)
> >  {
> > -	for (int i = 0; i < 15; i++)
> > +	for (int i = 0; i < page_size > 4096 ? 13 : 15; i++)

> Uff, this is starting to look like perl...

OK, using variable makes things readable.

Kind regards,
Petr

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