[LTP] [PATCH] swapon01: swapon01: prevent OOM happening in swap process
Li Wang
liwang@redhat.com
Tue Mar 19 09:18:35 CET 2024
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.
>
> 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