Re: [LTP] [PATCH 1/2] swapon01: Test on all filesystems

[Petr Vorel <pvorel@suse.cz>]

> On Fri, Jan 19, 2024 at 7:42 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> > Hi!
> > > Test on all filesystems to increase coverage.  btrfs and tmpfs
> > > currently does not support swap file, but keep it in case this get
> > > changed in the future.

> > > Testing on all filesystems usually requests root, but we don't require
> > > it with .needs_root = 1. But using swapon() requires it as well, thus
> > > keep it defined.

> > > Also detect the support on the same file as which is being tested.

> > For the previous iteration of this patch Li wasn't sure if this adds
> > enough value since there isn't ton of filesystem specific code involved
> > unless we actually start writing to the swapfile.

I wonder how to force. We could setup

sysctl vm.swappiness=100

But how to consume enough RAM to be actually written?

> Yes, if testing with all_filesystems, only make_swapfile process makes
> sense IMO.

> The reset swapon() operation does the same thing to the kernel.

> > And as the patch is I would agree with that. What may make sense I think
> > is to require certain filesystem to support swap, i.e. fail the test in
> > the case that we haven't managed to create and enable the swapfile where
> > it's supposed to work. That would guard about accidental breakages, as
> > it is the test would not catch these because it woult TCONF in the
> > setup.


> +1

> This sounds reasonable, looks like we need a whitelist to guarantee
> those filesystems that support swapfile, otherwise it's easy to miss
> some false negatives with report TCONF by is_swap_supported().

OK, even without writing it would make sense to test on all filesystems.

Replacing is_swap_supported() with a static list sounds good to me.
We might endup to check kernel release for certain filesystem
(if it got swap support later). Going to send a patch.

Kind regards,
Petr


> > > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > > ---
> > > IMHO it's not a big slowdown thus I keep btrfs and tmpfs.
> > > They might get support one day.

> > >  testcases/kernel/syscalls/swapon/swapon01.c | 17 +++++++++++------
> > >  1 file changed, 11 insertions(+), 6 deletions(-)

> > > diff --git a/testcases/kernel/syscalls/swapon/swapon01.c
> > b/testcases/kernel/syscalls/swapon/swapon01.c
> > > index b5c3f359c..6b7f96cf7 100644
> > > --- a/testcases/kernel/syscalls/swapon/swapon01.c
> > > +++ b/testcases/kernel/syscalls/swapon/swapon01.c
> > > @@ -9,6 +9,9 @@
> > >   * Checks that swapon() succeds with swapfile.
> > >   */

> > > +#define MNTPOINT     "mntpoint"
> > > +#define SWAP_FILE    MNTPOINT"/swapfile01"
> > > +
> > >  #include <unistd.h>
> > >  #include <errno.h>
> > >  #include <stdlib.h>
> > > @@ -18,14 +21,14 @@

> > >  static void verify_swapon(void)
> > >  {
> > > -     TEST(tst_syscall(__NR_swapon, "./swapfile01", 0));
> > > +     TEST(tst_syscall(__NR_swapon, SWAP_FILE, 0));

> > >       if (TST_RET == -1) {
> > >               tst_res(TFAIL | TTERRNO, "Failed to turn on swapfile");
> > >       } else {
> > >               tst_res(TPASS, "Succeeded to turn on swapfile");
> > >               /*we need to turn this swap file off for -i option */
> > > -             if (tst_syscall(__NR_swapoff, "./swapfile01") != 0) {
> > > +             if (tst_syscall(__NR_swapoff, SWAP_FILE) != 0) {
> > >                       tst_brk(TBROK | TERRNO, "Failed to turn off
> > swapfile,"
> > >                               " system reboot after execution of LTP "
> > >                               "test suite is recommended.");
> > > @@ -35,13 +38,15 @@ static void verify_swapon(void)

> > >  static void setup(void)
> > >  {
> > > -     is_swap_supported("./tstswap");
> > > -     make_swapfile("swapfile01", 0);
> > > +     is_swap_supported(SWAP_FILE);
> > > +     make_swapfile(SWAP_FILE, 0);
> > >  }

> > >  static struct tst_test test = {
> > > -     .needs_root = 1,
> > > -     .needs_tmpdir = 1,
> > > +     .mntpoint = MNTPOINT,
> > > +     .mount_device = 1,
> > > +     .needs_root = 1, /* for swapon() */
> > > +     .all_filesystems = 1,
> > >       .test_all = verify_swapon,
> > >       .setup = setup
> > >  };
> > > --
> > > 2.42.0


> > --
> > Cyril Hrubis
> > chrubis@suse.cz

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

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