[LTP] [PATCH v2 0/2] add method to create swapfile by MB size

[LTP] [PATCH v2 0/2] add method to create swapfile by MB size

[Li Wang]

V1 --> V2
	* squash 2/3 to 1/3 to make the build completely
	* make use of Wei's suggest to simplify the process
	* refine some code incedent

Li Wang (2):
  libswap: add two methods to create swapfile
  swapon01: create 128MB swapfile

 include/libswap.h                             | 16 ++++++++++++++--
 libs/libltpswap/libswap.c                     | 14 +++++++++++---
 testcases/kernel/syscalls/swapoff/swapoff01.c |  2 +-
 testcases/kernel/syscalls/swapoff/swapoff02.c |  2 +-
 testcases/kernel/syscalls/swapon/swapon01.c   |  2 +-
 testcases/kernel/syscalls/swapon/swapon02.c   |  4 ++--
 testcases/kernel/syscalls/swapon/swapon03.c   |  4 ++--
 7 files changed, 32 insertions(+), 12 deletions(-)

-- 
2.40.1


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

[LTP] [PATCH v2 1/2] libswap: add two methods to create swapfile

[Li Wang]

This introduces new functionality to the libswap library by adding
two methods for creating a swapfile: SWAPFILE_BY_SIZE and SWAPFILE_BY_BLOCKS.
The make_swapfile function is updated to accept an additional enum
swapfile_method parameter to specify the creation method.

Two macros, MAKE_SWAPFILE_SIZE and MAKE_SWAPFILE_BLKS, are defined
to simplify the interface for creating swapfiles by size and by
blocks respectively.

Signed-off-by: Li Wang <liwang@redhat.com>
Signed-off-by: Wei Gao <wegao@suse.com>
Reviewed-by: Wei Gao <wegao@suse.com>
---
 include/libswap.h                             | 16 ++++++++++++++--
 libs/libltpswap/libswap.c                     | 14 +++++++++++---
 testcases/kernel/syscalls/swapoff/swapoff01.c |  2 +-
 testcases/kernel/syscalls/swapoff/swapoff02.c |  2 +-
 testcases/kernel/syscalls/swapon/swapon01.c   |  2 +-
 testcases/kernel/syscalls/swapon/swapon02.c   |  4 ++--
 testcases/kernel/syscalls/swapon/swapon03.c   |  4 ++--
 7 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/include/libswap.h b/include/libswap.h
index 8c75e20ae..85ba88ed6 100644
--- a/include/libswap.h
+++ b/include/libswap.h
@@ -11,10 +11,22 @@
 #ifndef __LIBSWAP_H__
 #define __LIBSWAP_H__
 
+enum swapfile_method {
+    SWAPFILE_BY_SIZE,
+    SWAPFILE_BY_BLKS
+};
+
 /*
- * Make a swap file
+ * Create a swapfile of a specified size or number of blocks.
  */
-int make_swapfile(const char *swapfile, int blocks, int safe);
+int make_swapfile(const char *swapfile, unsigned int num,
+			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_BLKS)
 
 /*
  * Check swapon/swapoff support status of filesystems or files
diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
index a26ea25e4..0e2476ec2 100644
--- a/libs/libltpswap/libswap.c
+++ b/libs/libltpswap/libswap.c
@@ -133,18 +133,26 @@ out:
 	return contiguous;
 }
 
-int make_swapfile(const char *swapfile, int blocks, int safe)
+int make_swapfile(const char *swapfile, unsigned int num, 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 = 0;
 
 	if (statvfs(".", &fs_info) == -1)
 		return -1;
 
 	blk_size = fs_info.f_bsize;
 
+	if (method == SWAPFILE_BY_SIZE)
+		blocks = num * 1024 * 1024 / blk_size;
+	else if (method == SWAPFILE_BY_BLKS)
+		blocks = num;
+	else
+		tst_brk(TBROK, "Invalid method, please see include/libswap.h");
+
 	/* To guarantee at least one page can be swapped out */
 	if (blk_size * blocks < pg_size)
 		bs = pg_size;
@@ -175,13 +183,13 @@ int make_swapfile(const char *swapfile, int blocks, int safe)
 	argv[2] = NULL;
 
 	return tst_cmd(argv, "/dev/null", "/dev/null", safe ?
-				   TST_CMD_PASS_RETVAL | TST_CMD_TCONF_ON_MISSING : 0);
+			TST_CMD_PASS_RETVAL | TST_CMD_TCONF_ON_MISSING : 0);
 }
 
 bool is_swap_supported(const char *filename)
 {
 	int i, sw_support = 0;
-	int ret = make_swapfile(filename, 10, 1);
+	int ret = MAKE_SWAPFILE_BLKS(filename, 10, 1);
 	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/swapoff/swapoff01.c b/testcases/kernel/syscalls/swapoff/swapoff01.c
index 2a0b683c1..d0d7c3c1f 100644
--- a/testcases/kernel/syscalls/swapoff/swapoff01.c
+++ b/testcases/kernel/syscalls/swapoff/swapoff01.c
@@ -44,7 +44,7 @@ static void setup(void)
 {
 	is_swap_supported(TEST_FILE);
 
-	if (make_swapfile(SWAP_FILE, 65536, 1))
+	if (MAKE_SWAPFILE_BLKS(SWAP_FILE, 65536, 1))
 		tst_brk(TBROK, "Failed to create file for swap");
 }
 
diff --git a/testcases/kernel/syscalls/swapoff/swapoff02.c b/testcases/kernel/syscalls/swapoff/swapoff02.c
index 52906848f..b57290386 100644
--- a/testcases/kernel/syscalls/swapoff/swapoff02.c
+++ b/testcases/kernel/syscalls/swapoff/swapoff02.c
@@ -88,7 +88,7 @@ static void setup(void)
 
 	is_swap_supported(TEST_FILE);
 
-	if (make_swapfile(SWAP_FILE, 10, 1))
+	if (MAKE_SWAPFILE_BLKS(SWAP_FILE, 10, 1))
 		tst_brk(TBROK, "Failed to create file for swap");
 }
 
diff --git a/testcases/kernel/syscalls/swapon/swapon01.c b/testcases/kernel/syscalls/swapon/swapon01.c
index d406e4bd9..2e399db61 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_BLKS(SWAP_FILE, 10, 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;
 }
-- 
2.40.1


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

Re: [LTP] [PATCH v2 1/2] libswap: add two methods to create swapfile

[Petr Vorel]

Hi Li,

Generally LGTM.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

...
>  /*
> - * Make a swap file
> + * Create a swapfile of a specified size or number of blocks.
>   */
> -int make_swapfile(const char *swapfile, int blocks, int safe);
> +int make_swapfile(const char *swapfile, unsigned int num,
> +			int safe, enum swapfile_method method);
I wonder if it would help to add const char *file, const int lineno here.

> +
> +#define MAKE_SWAPFILE_SIZE(swapfile, size, safe) \
> +    make_swapfile(swapfile, size, safe, SWAPFILE_BY_SIZE)
nit: I like the name but one have to search which units (kB vs. MB vs. GB) are used.

> +
> +#define MAKE_SWAPFILE_BLKS(swapfile, blocks, safe) \
> +    make_swapfile(swapfile, blocks, safe, SWAPFILE_BY_BLKS)

And we could also have SAFE_ variants.

Therefore maybe rename make_swapfile() to make_swapfile_()
(approach in LTP for functions to be wrapped) and define macros:

int make_swapfile_(const char *file, const int lineno,
	const char *swapfile, unsigned int num,
	int safe, enum swapfile_method method);

#define MAKE_SWAPFILE_SIZE(swapfile, size, safe) \
    make_swapfile_(__FILE__, __LINE__, swapfile, size, 0, SWAPFILE_BY_SIZE)

#define MAKE_SWAPFILE_BLKS(swapfile, blocks, safe) \
    make_swapfile_(__FILE__, __LINE__, swapfile, blocks, 0, SWAPFILE_BY_BLKS)

#define SAFE_MAKE_SWAPFILE_SIZE(swapfile, size, safe) \
    make_swapfile_(__FILE__, __LINE__, swapfile, size, 1, SWAPFILE_BY_SIZE)

#define SAFE_MAKE_SWAPFILE_BLKS(swapfile, blocks, safe) \
    make_swapfile_(__FILE__, __LINE__, swapfile, blocks, 1, SWAPFILE_BY_BLKS)


>  /*
>   * Check swapon/swapoff support status of filesystems or files
> diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> index a26ea25e4..0e2476ec2 100644
> --- a/libs/libltpswap/libswap.c
> +++ b/libs/libltpswap/libswap.c
> @@ -133,18 +133,26 @@ out:
>  	return contiguous;
>  }

> -int make_swapfile(const char *swapfile, int blocks, int safe)
> +int make_swapfile(const char *swapfile, unsigned int num, 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 = 0;

>  	if (statvfs(".", &fs_info) == -1)
>  		return -1;

>  	blk_size = fs_info.f_bsize;

> +	if (method == SWAPFILE_BY_SIZE)
> +		blocks = num * 1024 * 1024 / blk_size;
> +	else if (method == SWAPFILE_BY_BLKS)
> +		blocks = num;
> +	else
> +		tst_brk(TBROK, "Invalid method, please see include/libswap.h");

nit: I would print the method.

Using const char *file, const int lineno and tst_brk_() would help
later to point out which file actually contains wrong method.

...

Kind regards,
Petr

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

Re: [LTP] [PATCH v2 1/2] libswap: add two methods to create swapfile

[Li Wang]

On Wed, Mar 20, 2024 at 3:31 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> Generally LGTM.
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> ...
> >  /*
> > - * Make a swap file
> > + * Create a swapfile of a specified size or number of blocks.
> >   */
> > -int make_swapfile(const char *swapfile, int blocks, int safe);
> > +int make_swapfile(const char *swapfile, unsigned int num,
> > +                     int safe, enum swapfile_method method);
> I wonder if it would help to add const char *file, const int lineno here.
>
> > +
> > +#define MAKE_SWAPFILE_SIZE(swapfile, size, safe) \
> > +    make_swapfile(swapfile, size, safe, SWAPFILE_BY_SIZE)
> nit: I like the name but one have to search which units (kB vs. MB vs. GB)
> are used.
>

Maybe we can add code comments like:

+/**
+ * Macro to create swapfile size in megabytes (MB)
+ */
 #define MAKE_SWAPFILE_SIZE(swapfile, size, safe) \
    ...

+/**
+ * Macro to create swapfile size in block numbers
+ */
 #define MAKE_SWAPFILE_BLKS(swapfile, blocks, safe) \
      ...


> > +
> > +#define MAKE_SWAPFILE_BLKS(swapfile, blocks, safe) \
> > +    make_swapfile(swapfile, blocks, safe, SWAPFILE_BY_BLKS)
>
> And we could also have SAFE_ variants.
>
> Therefore maybe rename make_swapfile() to make_swapfile_()
> (approach in LTP for functions to be wrapped) and define macros:
>
> int make_swapfile_(const char *file, const int lineno,
>         const char *swapfile, unsigned int num,
>         int safe, enum swapfile_method method);
>
> #define MAKE_SWAPFILE_SIZE(swapfile, size, safe) \
>     make_swapfile_(__FILE__, __LINE__, swapfile, size, 0, SWAPFILE_BY_SIZE)
>
> #define MAKE_SWAPFILE_BLKS(swapfile, blocks, safe) \
>     make_swapfile_(__FILE__, __LINE__, swapfile, blocks, 0,
> SWAPFILE_BY_BLKS)
>
> #define SAFE_MAKE_SWAPFILE_SIZE(swapfile, size, safe) \
>     make_swapfile_(__FILE__, __LINE__, swapfile, size, 1, SWAPFILE_BY_SIZE)
>
> #define SAFE_MAKE_SWAPFILE_BLKS(swapfile, blocks, safe) \
>     make_swapfile_(__FILE__, __LINE__, swapfile, blocks, 1,
> SWAPFILE_BY_BLKS)
>


Good suggestions.


>
> >  /*
> >   * Check swapon/swapoff support status of filesystems or files
> > diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> > index a26ea25e4..0e2476ec2 100644
> > --- a/libs/libltpswap/libswap.c
> > +++ b/libs/libltpswap/libswap.c
> > @@ -133,18 +133,26 @@ out:
> >       return contiguous;
> >  }
>
> > -int make_swapfile(const char *swapfile, int blocks, int safe)
> > +int make_swapfile(const char *swapfile, unsigned int num, 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 = 0;
>
> >       if (statvfs(".", &fs_info) == -1)
> >               return -1;
>
> >       blk_size = fs_info.f_bsize;
>
> > +     if (method == SWAPFILE_BY_SIZE)
> > +             blocks = num * 1024 * 1024 / blk_size;
> > +     else if (method == SWAPFILE_BY_BLKS)
> > +             blocks = num;
> > +     else
> > +             tst_brk(TBROK, "Invalid method, please see
> include/libswap.h");
>
> nit: I would print the method.
>

+1


> Using const char *file, const int lineno and tst_brk_() would help
> later to point out which file actually contains wrong method.
>

Yes, that should be better. Thanks!


-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH v2 1/2] libswap: add two methods to create swapfile

[Petr Vorel]

...
> > > -int make_swapfile(const char *swapfile, int blocks, int safe);
> > > +int make_swapfile(const char *swapfile, unsigned int num,
> > > +                     int safe, enum swapfile_method method);
> > I wonder if it would help to add const char *file, const int lineno here.

> > > +
> > > +#define MAKE_SWAPFILE_SIZE(swapfile, size, safe) \
> > > +    make_swapfile(swapfile, size, safe, SWAPFILE_BY_SIZE)
> > nit: I like the name but one have to search which units (kB vs. MB vs. GB)
> > are used.


> Maybe we can add code comments like:

> +/**
> + * Macro to create swapfile size in megabytes (MB)
> + */
>  #define MAKE_SWAPFILE_SIZE(swapfile, size, safe) \
>     ...

> +/**
> + * Macro to create swapfile size in block numbers
> + */
>  #define MAKE_SWAPFILE_BLKS(swapfile, blocks, safe) \
>       ...

Sure, that's enough.

Kind regards,
Petr

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

[LTP] [PATCH v2 2/2] swapon01: create 128MB swapfile

[Li Wang]

The modification in the setup function changes the swap file
creation approach. Instead of allocating swap space based on
a number of blocks (which potentially could be too small for
the polluted memory swapout, thus causing an OOM), it now uses
a fixed size of 128 units for the swap file creation.

This is likely a conservative figure to prevent the OOM killer
from being triggered due to excessive memory allocation for the
swap file, helping ensure the test runs successfully on systems
with no swap space.

Signed-off-by: Li Wang <liwang@redhat.com>
Reviewed-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/syscalls/swapon/swapon01.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/swapon/swapon01.c b/testcases/kernel/syscalls/swapon/swapon01.c
index 2e399db61..17fde00bc 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_BLKS(SWAP_FILE, 10, 0);
+	MAKE_SWAPFILE_SIZE(SWAP_FILE, 128, 0);
 
 	SAFE_CG_PRINTF(tst_cg, "cgroup.procs", "%d", getpid());
 	SAFE_CG_PRINTF(tst_cg, "memory.max", "%lu", TESTMEM);
-- 
2.40.1


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

Re: [LTP] [PATCH v2 2/2] swapon01: create 128MB swapfile

[Petr Vorel]

> The modification in the setup function changes the swap file
> creation approach. Instead of allocating swap space based on
> a number of blocks (which potentially could be too small for
> the polluted memory swapout, thus causing an OOM), it now uses
> a fixed size of 128 units for the swap file creation.

Great, thank you.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

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