* [LTP] [PATCH 1/1] libswap: Change TWARN message to TINFO
@ 2024-04-17 9:02 Petr Vorel
2024-04-17 9:03 ` Andrea Cervesato via ltp
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Petr Vorel @ 2024-04-17 9:02 UTC (permalink / raw)
To: ltp
This is hit on systems with 64kb page size (e.g. on aarch64 with
CONFIG_ARM64_64K_PAGES=y or on ppc64le with CONFIG_PAGE_SIZE_64KB=y).
Using TINFO causes test not "failing" with non-zero exit code.
Fixes: f987ffff5 ("libswap: add two methods to create swapfile")
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
libs/libltpswap/libswap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
index 313a15f24..6d4184ef9 100644
--- a/libs/libltpswap/libswap.c
+++ b/libs/libltpswap/libswap.c
@@ -160,7 +160,7 @@ int make_swapfile_(const char *file, const int lineno,
/* To guarantee at least one page can be swapped out */
if (blk_size * blocks < pg_size) {
- tst_res(TWARN, "Swapfile size is less than the system page size. "
+ tst_res(TINFO, "Swapfile size is less than the system page size. "
"Using page size (%lu bytes) instead of block size (%lu bytes).",
(unsigned long)pg_size, blk_size);
blk_size = pg_size;
--
2.43.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [LTP] [PATCH 1/1] libswap: Change TWARN message to TINFO
2024-04-17 9:02 [LTP] [PATCH 1/1] libswap: Change TWARN message to TINFO Petr Vorel
@ 2024-04-17 9:03 ` Andrea Cervesato via ltp
2024-04-17 9:10 ` Petr Vorel
2024-04-17 10:02 ` Cyril Hrubis
2 siblings, 0 replies; 11+ messages in thread
From: Andrea Cervesato via ltp @ 2024-04-17 9:03 UTC (permalink / raw)
To: ltp
Hi Petr,
On 4/17/24 11:02, Petr Vorel wrote:
> This is hit on systems with 64kb page size (e.g. on aarch64 with
> CONFIG_ARM64_64K_PAGES=y or on ppc64le with CONFIG_PAGE_SIZE_64KB=y).
> Using TINFO causes test not "failing" with non-zero exit code.
>
> Fixes: f987ffff5 ("libswap: add two methods to create swapfile")
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> libs/libltpswap/libswap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> index 313a15f24..6d4184ef9 100644
> --- a/libs/libltpswap/libswap.c
> +++ b/libs/libltpswap/libswap.c
> @@ -160,7 +160,7 @@ int make_swapfile_(const char *file, const int lineno,
>
> /* To guarantee at least one page can be swapped out */
> if (blk_size * blocks < pg_size) {
> - tst_res(TWARN, "Swapfile size is less than the system page size. "
> + tst_res(TINFO, "Swapfile size is less than the system page size. "
> "Using page size (%lu bytes) instead of block size (%lu bytes).",
> (unsigned long)pg_size, blk_size);
> blk_size = pg_size;
Are we sure about this? TWARN wasn't a bad choice, considering the
meaning of the message.
Andrea
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [LTP] [PATCH 1/1] libswap: Change TWARN message to TINFO
2024-04-17 9:02 [LTP] [PATCH 1/1] libswap: Change TWARN message to TINFO Petr Vorel
2024-04-17 9:03 ` Andrea Cervesato via ltp
@ 2024-04-17 9:10 ` Petr Vorel
2024-04-17 10:02 ` Cyril Hrubis
2 siblings, 0 replies; 11+ messages in thread
From: Petr Vorel @ 2024-04-17 9:10 UTC (permalink / raw)
To: ltp
Hi,
> This is hit on systems with 64kb page size (e.g. on aarch64 with
> CONFIG_ARM64_64K_PAGES=y or on ppc64le with CONFIG_PAGE_SIZE_64KB=y).
> Using TINFO causes test not "failing" with non-zero exit code.
Also, sometimes we use:
tst_res(TINFO, "WARNING: ...",
e.g. TWARN which would still keep the test exiting 0 (I know, i bit strange).
Maybe we would need some specific TWARN_NOFAIL, which would print as TWARN
(=> more visible) but test exit code would be 0.
Kind regards,
Petr
> Fixes: f987ffff5 ("libswap: add two methods to create swapfile")
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> libs/libltpswap/libswap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> index 313a15f24..6d4184ef9 100644
> --- a/libs/libltpswap/libswap.c
> +++ b/libs/libltpswap/libswap.c
> @@ -160,7 +160,7 @@ int make_swapfile_(const char *file, const int lineno,
> /* To guarantee at least one page can be swapped out */
> if (blk_size * blocks < pg_size) {
> - tst_res(TWARN, "Swapfile size is less than the system page size. "
> + tst_res(TINFO, "Swapfile size is less than the system page size. "
> "Using page size (%lu bytes) instead of block size (%lu bytes).",
> (unsigned long)pg_size, blk_size);
> blk_size = pg_size;
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [LTP] [PATCH 1/1] libswap: Change TWARN message to TINFO
2024-04-17 9:02 [LTP] [PATCH 1/1] libswap: Change TWARN message to TINFO Petr Vorel
2024-04-17 9:03 ` Andrea Cervesato via ltp
2024-04-17 9:10 ` Petr Vorel
@ 2024-04-17 10:02 ` Cyril Hrubis
2024-04-17 10:06 ` Petr Vorel
2 siblings, 1 reply; 11+ messages in thread
From: Cyril Hrubis @ 2024-04-17 10:02 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp
Hi!
> This is hit on systems with 64kb page size (e.g. on aarch64 with
> CONFIG_ARM64_64K_PAGES=y or on ppc64le with CONFIG_PAGE_SIZE_64KB=y).
> Using TINFO causes test not "failing" with non-zero exit code.
>
> Fixes: f987ffff5 ("libswap: add two methods to create swapfile")
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> libs/libltpswap/libswap.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> index 313a15f24..6d4184ef9 100644
> --- a/libs/libltpswap/libswap.c
> +++ b/libs/libltpswap/libswap.c
> @@ -160,7 +160,7 @@ int make_swapfile_(const char *file, const int lineno,
>
> /* To guarantee at least one page can be swapped out */
> if (blk_size * blocks < pg_size) {
> - tst_res(TWARN, "Swapfile size is less than the system page size. "
> + tst_res(TINFO, "Swapfile size is less than the system page size. "
> "Using page size (%lu bytes) instead of block size (%lu bytes).",
> (unsigned long)pg_size, blk_size);
This looks like we are working around a test bug, which test is
triggering this condition?
> blk_size = pg_size;
> --
> 2.43.0
>
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [LTP] [PATCH 1/1] libswap: Change TWARN message to TINFO
2024-04-17 10:02 ` Cyril Hrubis
@ 2024-04-17 10:06 ` Petr Vorel
2024-04-17 10:31 ` Cyril Hrubis
0 siblings, 1 reply; 11+ messages in thread
From: Petr Vorel @ 2024-04-17 10:06 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp
> Hi!
> > This is hit on systems with 64kb page size (e.g. on aarch64 with
> > CONFIG_ARM64_64K_PAGES=y or on ppc64le with CONFIG_PAGE_SIZE_64KB=y).
> > Using TINFO causes test not "failing" with non-zero exit code.
> > Fixes: f987ffff5 ("libswap: add two methods to create swapfile")
> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> > libs/libltpswap/libswap.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> > index 313a15f24..6d4184ef9 100644
> > --- a/libs/libltpswap/libswap.c
> > +++ b/libs/libltpswap/libswap.c
> > @@ -160,7 +160,7 @@ int make_swapfile_(const char *file, const int lineno,
> > /* To guarantee at least one page can be swapped out */
> > if (blk_size * blocks < pg_size) {
> > - tst_res(TWARN, "Swapfile size is less than the system page size. "
> > + tst_res(TINFO, "Swapfile size is less than the system page size. "
> > "Using page size (%lu bytes) instead of block size (%lu bytes).",
> > (unsigned long)pg_size, blk_size);
> This looks like we are working around a test bug, which test is
> triggering this condition?
All swap tests: swapoff0[12], swapon0[1-3]
https://lore.kernel.org/ltp/CAEemH2ev62JxH7-DA5Sc2LjMKrquYqt927ATHZefNPAOiXb5qA@mail.gmail.com/
Kind regards,
Petr
> > blk_size = pg_size;
> > --
> > 2.43.0
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [LTP] [PATCH 1/1] libswap: Change TWARN message to TINFO
2024-04-17 10:06 ` Petr Vorel
@ 2024-04-17 10:31 ` Cyril Hrubis
2024-04-17 12:24 ` Petr Vorel
0 siblings, 1 reply; 11+ messages in thread
From: Cyril Hrubis @ 2024-04-17 10:31 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp
Hi!
Looking at the patches we get the warning from the is_swap_supported()
because we create only 10 blocks worth of swapfile there, right?
So the easiest fix is to make sure we create big enough swapfile there
as well, what about?
diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
index 313a15f24..36b5af483 100644
--- a/libs/libltpswap/libswap.c
+++ b/libs/libltpswap/libswap.c
@@ -192,7 +192,7 @@ int make_swapfile_(const char *file, const int lineno,
bool is_swap_supported(const char *filename)
{
int i, sw_support = 0;
- int ret = SAFE_MAKE_SWAPFILE_BLKS(filename, 10);
+ int ret = SAFE_MAKE_SWAPFILE_SIZE(filename, 1);
int fi_contiguous = file_is_contiguous(filename);
long fs_type = tst_fs_type(filename);
const char *fstype = tst_fs_type_name(fs_type);
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [LTP] [PATCH 1/1] libswap: Change TWARN message to TINFO
2024-04-17 10:31 ` Cyril Hrubis
@ 2024-04-17 12:24 ` Petr Vorel
2024-04-17 13:37 ` Petr Vorel
0 siblings, 1 reply; 11+ messages in thread
From: Petr Vorel @ 2024-04-17 12:24 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp
> Hi!
> Looking at the patches we get the warning from the is_swap_supported()
> because we create only 10 blocks worth of swapfile there, right?
> So the easiest fix is to make sure we create big enough swapfile there
> as well, what about?
> diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> index 313a15f24..36b5af483 100644
> --- a/libs/libltpswap/libswap.c
> +++ b/libs/libltpswap/libswap.c
> @@ -192,7 +192,7 @@ int make_swapfile_(const char *file, const int lineno,
> bool is_swap_supported(const char *filename)
> {
> int i, sw_support = 0;
> - int ret = SAFE_MAKE_SWAPFILE_BLKS(filename, 10);
> + int ret = SAFE_MAKE_SWAPFILE_SIZE(filename, 1);
Let me first check where it's needed, because SAFE_MAKE_SWAPFILE_BLKS() is
called on more places.
Also, first is needed to move __FILE__, __LINE__ in macros (from function),
sending patch...
Kind regards,
Petr
> int fi_contiguous = file_is_contiguous(filename);
> long fs_type = tst_fs_type(filename);
> const char *fstype = tst_fs_type_name(fs_type);
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [LTP] [PATCH 1/1] libswap: Change TWARN message to TINFO
2024-04-17 12:24 ` Petr Vorel
@ 2024-04-17 13:37 ` Petr Vorel
2024-04-17 13:42 ` Cyril Hrubis
0 siblings, 1 reply; 11+ messages in thread
From: Petr Vorel @ 2024-04-17 13:37 UTC (permalink / raw)
To: Cyril Hrubis, ltp, Li Wang
> > Hi!
> > Looking at the patches we get the warning from the is_swap_supported()
> > because we create only 10 blocks worth of swapfile there, right?
> > So the easiest fix is to make sure we create big enough swapfile there
> > as well, what about?
> > diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> > index 313a15f24..36b5af483 100644
> > --- a/libs/libltpswap/libswap.c
> > +++ b/libs/libltpswap/libswap.c
> > @@ -192,7 +192,7 @@ int make_swapfile_(const char *file, const int lineno,
> > bool is_swap_supported(const char *filename)
> > {
> > int i, sw_support = 0;
> > - int ret = SAFE_MAKE_SWAPFILE_BLKS(filename, 10);
> > + int ret = SAFE_MAKE_SWAPFILE_SIZE(filename, 1);
> Let me first check where it's needed, because SAFE_MAKE_SWAPFILE_BLKS() is
> called on more places.
Yes, not only it's directly used from the library, which affects all swapo{ff,n}
tests, but MAKE_SWAPFILE_BLKS() is also used directly by some of the tests.
Changing it to {SAFE_,}MAKE_SWAPFILE_SIZE() on all places would mean we don't need
{SAFE_,}MAKE_SWAPFILE_BLKS() at all. Therefore I think this check should be
suppressed to TINFO. Other option is to use high enough value (more blocks)
(not sure if using really small swap file makes sense anyway).
> Also, first is needed to move __FILE__, __LINE__ in macros (from function),
> sending patch...
https://lore.kernel.org/ltp/20240417123113.731780-1-pvorel@suse.cz/
https://lore.kernel.org/ltp/20240417123113.731780-1-pvorel@suse.cz/
Kind regards,
Petr
> Kind regards,
> Petr
> > int fi_contiguous = file_is_contiguous(filename);
> > long fs_type = tst_fs_type(filename);
> > const char *fstype = tst_fs_type_name(fs_type);
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [LTP] [PATCH 1/1] libswap: Change TWARN message to TINFO
2024-04-17 13:37 ` Petr Vorel
@ 2024-04-17 13:42 ` Cyril Hrubis
2024-04-17 18:38 ` Petr Vorel
0 siblings, 1 reply; 11+ messages in thread
From: Cyril Hrubis @ 2024-04-17 13:42 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp
Hi!
> Changing it to {SAFE_,}MAKE_SWAPFILE_SIZE() on all places would mean we don't need
> {SAFE_,}MAKE_SWAPFILE_BLKS() at all. Therefore I think this check should be
> suppressed to TINFO. Other option is to use high enough value (more blocks)
> (not sure if using really small swap file makes sense anyway).
I do not think that we should use too small swap file even for the case
where we try to detect if swapfiles are supported. Currently it seems
that kernel accepts swapfiles smaller than page size, but it may also
start rejecting them with EINVAL in the future.
So I would keep the warning and rather tried to fix all the parts where
we attempt to create a swapfile smaller than page size.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [LTP] [PATCH 1/1] libswap: Change TWARN message to TINFO
2024-04-17 13:42 ` Cyril Hrubis
@ 2024-04-17 18:38 ` Petr Vorel
2024-04-18 5:30 ` Li Wang
0 siblings, 1 reply; 11+ messages in thread
From: Petr Vorel @ 2024-04-17 18:38 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: ltp
> Hi!
> > Changing it to {SAFE_,}MAKE_SWAPFILE_SIZE() on all places would mean we don't need
> > {SAFE_,}MAKE_SWAPFILE_BLKS() at all. Therefore I think this check should be
> > suppressed to TINFO. Other option is to use high enough value (more blocks)
> > (not sure if using really small swap file makes sense anyway).
> I do not think that we should use too small swap file even for the case
> where we try to detect if swapfiles are supported. Currently it seems
> that kernel accepts swapfiles smaller than page size, but it may also
> start rejecting them with EINVAL in the future.
> So I would keep the warning and rather tried to fix all the parts where
> we attempt to create a swapfile smaller than page size.
+1, setting "changes requested", I'll send v2 with higher number of blocks to
fix this.
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [LTP] [PATCH 1/1] libswap: Change TWARN message to TINFO
2024-04-17 18:38 ` Petr Vorel
@ 2024-04-18 5:30 ` Li Wang
0 siblings, 0 replies; 11+ messages in thread
From: Li Wang @ 2024-04-18 5:30 UTC (permalink / raw)
To: Petr Vorel; +Cc: ltp
On Thu, Apr 18, 2024 at 2:38 AM Petr Vorel <pvorel@suse.cz> wrote:
> > Hi!
> > > Changing it to {SAFE_,}MAKE_SWAPFILE_SIZE() on all places would mean
> we don't need
> > > {SAFE_,}MAKE_SWAPFILE_BLKS() at all. Therefore I think this check
> should be
> > > suppressed to TINFO. Other option is to use high enough value (more
> blocks)
> > > (not sure if using really small swap file makes sense anyway).
>
> > I do not think that we should use too small swap file even for the case
> > where we try to detect if swapfiles are supported. Currently it seems
> > that kernel accepts swapfiles smaller than page size, but it may also
> > start rejecting them with EINVAL in the future.
>
> > So I would keep the warning and rather tried to fix all the parts where
> > we attempt to create a swapfile smaller than page size.
>
> +1, setting "changes requested", I'll send v2 with higher number of blocks
> to
> fix this.
>
Agree.
--
Regards,
Li Wang
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-04-18 5:30 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-17 9:02 [LTP] [PATCH 1/1] libswap: Change TWARN message to TINFO Petr Vorel
2024-04-17 9:03 ` Andrea Cervesato via ltp
2024-04-17 9:10 ` Petr Vorel
2024-04-17 10:02 ` Cyril Hrubis
2024-04-17 10:06 ` Petr Vorel
2024-04-17 10:31 ` Cyril Hrubis
2024-04-17 12:24 ` Petr Vorel
2024-04-17 13:37 ` Petr Vorel
2024-04-17 13:42 ` Cyril Hrubis
2024-04-17 18:38 ` Petr Vorel
2024-04-18 5:30 ` Li Wang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.