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

[Petr Vorel]

Hi Li,


> On Mon, Jan 22, 2024 at 2:27 PM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi Li, Cyril,

> > > Hi Petr, Cyril,

> > > On Fri, Jan 19, 2024 at 10:43 PM Petr Vorel <pvorel@suse.cz> wrote:

> > > > Test on all filesystems to increase coverage.
> > > > Skip filesystems which does not support swap (currently bcachefs, btrfs
> > > > and tmpfs).

> > > > Tested on 5.10, 6.6 and 6.7.

> > > > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > > > ---
> > > > Hi Li, Cyril,

> > > > feel free to postpone this after the release.
> > > > If we are for adding this, I'll retest on Monday older filesystems.

> > > > man swapon(8) mentions:

> > > >    Btrfs
> > > >        Swap files on Btrfs are supported since Linux 5.0 on files with
> > > > nocow
> > > >        attribute. See the btrfs(5) manual page for more details.

> > > > => Can we pass "nocow" just to btrfs? Or should it be added to single
> > > > test, which uses just btrfs with nocow?

> > > >    NFS
> > > >        Swap over NFS may not work.

> > > > => I guess this is not important for us, as NFS is not part of
> > > > filesystems in .all_filesystems, but maybe we should still add it to
> > the
> > > > whitelist?

> > > > Kind regards,
> > > > Petr

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

> > > > diff --git a/testcases/kernel/syscalls/swapon/swapon01.c
> > > > b/testcases/kernel/syscalls/swapon/swapon01.c
> > > > index e59fb20a1..ef4ebfdcf 100644
> > > > --- a/testcases/kernel/syscalls/swapon/swapon01.c
> > > > +++ b/testcases/kernel/syscalls/swapon/swapon01.c
> > > > @@ -8,6 +8,7 @@
> > > >   * [Description]
> > > >   *
> > > >   * Checks that swapon() succeds with swapfile.
> > > > + * Testing on all filesystems which support swap file.
> > > >   */

> > > >  #include <unistd.h>
> > > > @@ -17,13 +18,14 @@
> > > >  #include "lapi/syscalls.h"
> > > >  #include "libswap.h"

> > > > -#define SWAP_FILE "swapfile01"
> > > > +#define MNTPOINT       "mntpoint"
> > > > +#define SWAP_FILE      MNTPOINT"/swapfile01"

> > > >  static void verify_swapon(void)
> > > >  {
> > > >         TST_EXP_PASS(tst_syscall(__NR_swapon, SWAP_FILE, 0));

> > > > -       if (TST_PASS && tst_syscall(__NR_swapoff, SWAP_FILE) != 0) {
> > > > +       if (tst_syscall(__NR_swapoff, SWAP_FILE) != 0) {
> > > >                 tst_brk(TBROK | TERRNO,
> > > >                                 "Failed to turn off swapfile, system
> > > > reboot recommended");
> > > >         }
> > > > @@ -31,13 +33,20 @@ static void verify_swapon(void)

> > > >  static void setup(void)
> > > >  {
> > > > -       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
> > > > +       .setup = setup,
> > > > +       .skip_filesystems = (const char *[]) {
> > > > +               "bcachefs",
> > > > +               "btrfs",
> > > > +               "tmpfs",
> > > > +               NULL
> > > > +       },



> > > IMHO, I hate to add the skip_filesystem in this test, because if we do,
> > > then we have to add to all of the related swap tests that, it add more
> > > tedious work to people.

> > First I thought it'd be ok to have just single test which uses
> > all_filesystems
> > (better one than none), but sure, having all of them would be better.

> > > So, if we decide to postpone this patch to release, we can just refactor
> > > the is_swap_supported() function to make things easier. I will send an
> > RFC
> > > patch to show something in my mind, what we need to do later is just
> > > to finalize the swap_supported_fs[] struct list.

> > Sure, this is not critical, it can wait if you have better solution. Thanks
> > for spending time on it.


> Yes, but it is ready there 1/4 :).
>   https://lists.linux.it/pipermail/ltp/2024-January/036804.html

> The rest work is just to finalize the swap_supported_fs list.
> And, I agree that all of these can be postponed. Because
> so far it won't be a critical/urgent-fix for a new release.

OTOH if you prepared this, I'm ok to review and test.
I already run various tests on this.

Kind regards,
Petr