[LTP] [PATCH] swapon01: swapon01: prevent OOM happening in swap process
Wei Gao
wegao@suse.com
Tue Mar 19 10:20:19 CET 2024
On Tue, Mar 19, 2024 at 04:18:35PM +0800, Li Wang wrote:
> On Tue, Mar 19, 2024 at 3:29 PM Wei Gao <wegao@suse.com> wrote:
>
> > On Tue, Mar 19, 2024 at 01:51:10PM +0800, Li Wang wrote:
> > > On Tue, Mar 19, 2024 at 1:43 PM Li Wang <liwang@redhat.com> wrote:
> > >
> > > >
> > > >
> > > > On Tue, Mar 19, 2024 at 1:04 PM Petr Vorel <pvorel@suse.cz> wrote:
> > > >
> > > >> > On Mon, Mar 18, 2024 at 8:40 PM Wei Gao <wegao@suse.com> wrote:
> > > >>
> > > >>
> > > >>
> > > >>
> > > >> > > > That's because the available swapfile on your SUT is too small,
> > > >> > > > you can adjust it (then retest it) by yourself to find a proper
> > > >> size.
> > > >>
> > > >> > > > This is fine as long as the swapfile size is less than 300MB,
> > > >> > > > otherwise we need to set .dev_min_size like what we did
> > > >> > > > for swapoff01.c.
> > > >>
> > > >> > > > And, on the other side, we can't guarantee the system SwapCached
> > > >> > > > happened every time, it depends on the system's configuration.
> > > >>
> > > >>
> > > >> > > 100M is good enough for current system, could you help check
> > following
> > > >> > > patch?
> > > >>
> > > >>
> > > >> > Can we rewrite the make_swapfile() API to support passing MB size
> > for
> > > >> > making the swapfile?
> > > >>
> > > >> I guess it would be desirable (but keep also possible to pass that 10
> > > >> blocks,
> > > >> therefore maybe use flag to distinguish between MB and blocks?).
> > > >>
> > > >
> > > > That's fine but a bit complex for users to distinguish flags.
> > > >
> > > > Or, what about making the function use the flag as static, and
> > > > export two additional functions with MB and blocks?
> > > >
> > > > enum swapfile_method {
> > > > SWAPFILE_BY_SIZE,
> > > > SWAPFILE_BY_BLOCKS
> > > > };
> > > >
> > > > static int make_swapfile(const char *swapfile, unsigned int para, int
> > > > safe, enum swapfile_method method) {
> > > > // The main logic to achieve the swapfile-making process
> > > > // ...
> > > > }
> > > >
> > > > int make_swapfile_size(const char *swapfile, unsigned int size, int
> > safe) {
> > > > return make_swapfile(swapfile, size, safe, SWAPFILE_BY_SIZE);
> > > > }
> > > >
> > > > int make_swapfile_blks(const char *swapfile, unsigned int blocks, int
> > > > safe) {
> > > > return make_swapfile(swapfile, blocks, safe, SWAPFILE_BY_BLOCKS);
> > > > }
> > > >
> > >
> > > Even simpler to define two macros with one make_swapfile():
> > >
> > > #define MAKE_SWAPFILE_SIZE(swapfile, size, safe) \
> > > make_swapfile(swapfile, size, safe, SWAPFILE_BY_SIZE)
> > >
> > > #define MAKE_SWAPFILE_BLKS(swapfile, blocks, safe) \
> > > make_swapfile(swapfile, blocks, safe, SWAPFILE_BY_BLOCKS)
> > >
> > >
> > Make a quick below patch:
> >
>
> Thanks for your work Wei! (but seems duplicated with my patchset[1])
> [1] https://lists.linux.it/pipermail/ltp/2024-March/037627.html
>
> Sorry, I didn't mean to take the credit, I just tested it and found that
> the changes are quite simple, so just sent it out...
>
> Would you mind helping out with comments and reviews there?
> And I can help to add your SBT tag later.
>
No problem, i have checked your patch and looks good to me, no more comments.
I suppose we can merge this soon, thanks for your patch :)
>
>
> >
> > diff --git a/include/libswap.h b/include/libswap.h
> > index 8c75e20ae..c15930615 100644
> > --- a/include/libswap.h
> > +++ b/include/libswap.h
> > @@ -14,7 +14,18 @@
> > /*
> > * Make a swap file
> > */
> > -int make_swapfile(const char *swapfile, int blocks, int safe);
> > +enum swapfile_method {
> > + SWAPFILE_BY_SIZE,
> > + SWAPFILE_BY_BLOCKS
> > +};
> > +
> > +int make_swapfile(const char *swapfile, unsigned int para, int safe, enum
> > swapfile_method method);
> > +
> > +#define MAKE_SWAPFILE_SIZE(swapfile, size, safe) \
> > + make_swapfile(swapfile, size, safe, SWAPFILE_BY_SIZE)
> > +
> > +#define MAKE_SWAPFILE_BLKS(swapfile, blocks, safe) \
> > + make_swapfile(swapfile, blocks, safe, SWAPFILE_BY_BLOCKS)
> >
> > /*
> > * Check swapon/swapoff support status of filesystems or files
> > diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> > index a26ea25e4..88376be05 100644
> > --- a/libs/libltpswap/libswap.c
> > +++ b/libs/libltpswap/libswap.c
> > @@ -133,18 +133,24 @@ out:
> > return contiguous;
> > }
> >
> > -int make_swapfile(const char *swapfile, int blocks, int safe)
> > +int make_swapfile(const char *swapfile, unsigned int para, int safe, enum
> > swapfile_method method)
> > {
> > struct statvfs fs_info;
> > unsigned long blk_size, bs;
> > size_t pg_size = sysconf(_SC_PAGESIZE);
> > char mnt_path[100];
> > + unsigned int blocks;
> >
> > if (statvfs(".", &fs_info) == -1)
> > return -1;
> >
> > blk_size = fs_info.f_bsize;
> >
> > + if (method == SWAPFILE_BY_SIZE)
> > + blocks = para * 1024 * 1024 / blk_size;
> > + else
> > + blocks = para;
> > +
> > /* To guarantee at least one page can be swapped out */
> > if (blk_size * blocks < pg_size)
> > bs = pg_size;
> > @@ -181,7 +187,7 @@ int make_swapfile(const char *swapfile, int blocks,
> > int safe)
> > bool is_swap_supported(const char *filename)
> > {
> > int i, sw_support = 0;
> > - int ret = make_swapfile(filename, 10, 1);
> > + int ret = make_swapfile(filename, 10, 1, SWAPFILE_BY_BLOCKS);
> > int fi_contiguous = file_is_contiguous(filename);
> > long fs_type = tst_fs_type(filename);
> > const char *fstype = tst_fs_type_name(fs_type);
> > diff --git a/testcases/kernel/syscalls/swapon/swapon01.c
> > b/testcases/kernel/syscalls/swapon/swapon01.c
> > index d406e4bd9..823813714 100644
> > --- a/testcases/kernel/syscalls/swapon/swapon01.c
> > +++ b/testcases/kernel/syscalls/swapon/swapon01.c
> > @@ -38,7 +38,7 @@ static void verify_swapon(void)
> > static void setup(void)
> > {
> > is_swap_supported(SWAP_FILE);
> > - make_swapfile(SWAP_FILE, 10, 0);
> > + MAKE_SWAPFILE_SIZE(SWAP_FILE, 100, 0);
> >
> > SAFE_CG_PRINTF(tst_cg, "cgroup.procs", "%d", getpid());
> > SAFE_CG_PRINTF(tst_cg, "memory.max", "%lu", TESTMEM);
> > diff --git a/testcases/kernel/syscalls/swapon/swapon02.c
> > b/testcases/kernel/syscalls/swapon/swapon02.c
> > index 7e876d26a..f76bb28cf 100644
> > --- a/testcases/kernel/syscalls/swapon/swapon02.c
> > +++ b/testcases/kernel/syscalls/swapon/swapon02.c
> > @@ -50,8 +50,8 @@ static void setup(void)
> > is_swap_supported(TEST_FILE);
> >
> > SAFE_TOUCH(NOTSWAP_FILE, 0777, NULL);
> > - make_swapfile(SWAP_FILE, 10, 0);
> > - make_swapfile(USED_FILE, 10, 0);
> > + MAKE_SWAPFILE_BLKS(SWAP_FILE, 10, 0);
> > + MAKE_SWAPFILE_BLKS(USED_FILE, 10, 0);
> >
> > if (tst_syscall(__NR_swapon, USED_FILE, 0))
> > tst_res(TWARN | TERRNO, "swapon(alreadyused) failed");
> > diff --git a/testcases/kernel/syscalls/swapon/swapon03.c
> > b/testcases/kernel/syscalls/swapon/swapon03.c
> > index 6f47fc01f..aaaedfa11 100644
> > --- a/testcases/kernel/syscalls/swapon/swapon03.c
> > +++ b/testcases/kernel/syscalls/swapon/swapon03.c
> > @@ -49,7 +49,7 @@ static int setup_swap(void)
> >
> > /* Create the swapfile */
> > snprintf(filename, sizeof(filename), "%s%02d",
> > TEST_FILE, j + 2);
> > - make_swapfile(filename, 10, 0);
> > + MAKE_SWAPFILE_BLKS(filename, 10, 0);
> >
> > /* turn on the swap file */
> > TST_EXP_PASS_SILENT(swapon(filename, 0));
> > @@ -62,7 +62,7 @@ static int setup_swap(void)
> > tst_brk(TFAIL, "Failed to setup swap files");
> >
> > tst_res(TINFO, "Successfully created %d swap files", swapfiles);
> > - make_swapfile(TEST_FILE, 10, 0);
> > + MAKE_SWAPFILE_BLKS(TEST_FILE, 10, 0);
> >
> > return 0;
> > }
> >
> > >
> > > --
> > > Regards,
> > > Li Wang
> >
> >
>
> --
> Regards,
> Li Wang
More information about the ltp
mailing list