[v4,3/7] syscalls/swapon03: use tst_max_swapfiles() and GET_USED_SWAPFILES() api

Signed-off-by: Yang Xu <xuyang2018.jy@fujitsu.com>
---
 testcases/kernel/syscalls/swapon/swapon03.c | 40 ++++-----------------
 1 file changed, 6 insertions(+), 34 deletions(-)

Hi Xu,

I see swapon03 failures on aarch64 and ppc64le on SLES and openSUSE after this commit.

Here is timeout after 31s:

# ./swapon03
...
tst_test.c:1701: TINFO: === Testing on ext2 ===
tst_test.c:1117: TINFO: Formatting /dev/loop0 with ext2 opts='' extra opts=''
mke2fs 1.47.0 (5-Feb-2023)
tst_test.c:1131: TINFO: Mounting /dev/loop0 to /tmp/LTP_swaVqwa6f/mntpoint fstyp=ext2 flags=0
tst_ioctl.c:26: TINFO: FIBMAP ioctl is supported
tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
Test timeouted, sending SIGKILL!
tst_test.c:1679: TINFO: If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1
tst_test.c:1681: TBROK: Test killed! (timeout?)

Summary:
passed   0
failed   0
broken   1
skipped  0
warnings 0
### TEST swapon03 COMPLETE >>> 2.

I tried to run with .max_runtime = 60, but even then it fails after 1m 30s:
...
tst_supported_fs_types.c:97: TINFO: Kernel supports tmpfs
tst_supported_fs_types.c:49: TINFO: mkfs is not needed for tmpfs
tst_test.c:1701: TINFO: === Testing on ext2 ===
tst_test.c:1117: TINFO: Formatting /dev/loop0 with ext2 opts='' extra opts=''
mke2fs 1.47.0 (5-Feb-2023)
tst_test.c:1131: TINFO: Mounting /dev/loop0 to /tmp/LTP_swaQsjhAp/mntpoint fstyp=ext2 flags=0
tst_ioctl.c:26: TINFO: FIBMAP ioctl is supported
tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
Test timeouted, sending SIGKILL!
tst_test.c:1679: TINFO: If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1
tst_test.c:1681: TBROK: Test killed! (timeout?)

Summary:
passed   0
failed   0
broken   1
skipped  0
warnings 0

I'm trying with LTP_TIMEOUT_MUL=3.

BTW there is still broken swapoff01 on master on ppc64le which I reported [1]:
libswap.c:153: TBROK: Failed to create swapfile
(obviously no change in this patchset)

But I'll ping Li separately.

Kind regards,
Petr

[1] https://lore.kernel.org/ltp/20240131190122.GB30901@pevik/

> Hi Xu,
> 
> I see swapon03 failures on aarch64 and ppc64le on SLES and openSUSE after this commit.
> 
> Here is timeout after 31s:
> 
> # ./swapon03
> ...
> tst_test.c:1701: TINFO: === Testing on ext2 ===
> tst_test.c:1117: TINFO: Formatting /dev/loop0 with ext2 opts='' extra opts=''
> mke2fs 1.47.0 (5-Feb-2023)
> tst_test.c:1131: TINFO: Mounting /dev/loop0 to /tmp/LTP_swaVqwa6f/mntpoint fstyp=ext2 flags=0
> tst_ioctl.c:26: TINFO: FIBMAP ioctl is supported
> tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
> tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
> tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
> tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
> Test timeouted, sending SIGKILL!
> tst_test.c:1679: TINFO: If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1
> tst_test.c:1681: TBROK: Test killed! (timeout?)
> 
> Summary:
> passed   0
> failed   0
> broken   1
> skipped  0
> warnings 0
> ### TEST swapon03 COMPLETE >>> 2.
> 
> I tried to run with .max_runtime = 60, but even then it fails after 1m 30s:
> ...
> tst_supported_fs_types.c:97: TINFO: Kernel supports tmpfs
> tst_supported_fs_types.c:49: TINFO: mkfs is not needed for tmpfs
> tst_test.c:1701: TINFO: === Testing on ext2 ===
> tst_test.c:1117: TINFO: Formatting /dev/loop0 with ext2 opts='' extra opts=''
> mke2fs 1.47.0 (5-Feb-2023)
> tst_test.c:1131: TINFO: Mounting /dev/loop0 to /tmp/LTP_swaQsjhAp/mntpoint fstyp=ext2 flags=0
> tst_ioctl.c:26: TINFO: FIBMAP ioctl is supported
> tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
> tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
> tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
> tst_kconfig.c:87: TINFO: Parsing kernel config '/proc/config.gz'
> Test timeouted, sending SIGKILL!
> tst_test.c:1679: TINFO: If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1
> tst_test.c:1681: TBROK: Test killed! (timeout?)
> 
> Summary:
> passed   0
> failed   0
> broken   1
> skipped  0
> warnings 0
> 
> I'm trying with LTP_TIMEOUT_MUL=3.
> 
> BTW there is still broken swapoff01 on master on ppc64le which I reported [1]:
> libswap.c:153: TBROK: Failed to create swapfile
> (obviously no change in this patchset)
> 
> But I'll ping Li separately.
> 
> Kind regards,
> Petr
> 
> [1] https://lore.kernel.org/ltp/20240131190122.GB30901@pevik/

To be honest, I don't know why this commit leads case hang.
So can you add some debug info in swapon03 between tst_max_swapfiles and 
tst_count_swaps
api(I can't reproduce it becase I don't have situation)
ps: I guess the following way maybe lead hang in tst_count_swaps
"
+	while ((c = fgetc(fp)) != EOF) {
+		if (c == '\n')
+			used++;
+	}
"

Maybe I should use the old way[1]

[1]https://patchwork.ozlabs.org/project/ltp/patch/20231222050006.148845-2-xuyang2018.jy@fujitsu.com/

Best Regards
Yang Xu

Hi Yang Xu,

LGTM, few comments below.

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

> @@ -19,7 +19,6 @@

>  #include "tst_test.h"
>  #include "lapi/syscalls.h"
> -#include "swaponoff.h"
+1

>  #include "libswap.h"

>  #define MNTPOINT	"mntpoint"
> @@ -104,47 +103,20 @@ static void verify_swapon(void)

Here is comment:
/*
 * Create 33 and activate 30 swapfiles.
 */
This would deserve to update, because we use tst_max_swapfiles(), rirght?
Something like: "Create max number of swap files + 1 and activate max number
swap files -2."
>  static int setup_swap(void)
>  {
>  	pid_t pid;
> -	int j, fd;
>  	int status;
> +	int j, max_swapfiles, used_swapfiles;
>  	int res = 0;
>  	char filename[FILENAME_MAX];
> -	char buf[BUFSIZ + 1];
> -
> -	/* Find out how many swapfiles (1 line per entry) already exist */
> -	swapfiles = 0;

>  	if (seteuid(0) < 0)
>  		tst_brk(TFAIL | TERRNO, "Failed to call seteuid");
nit: while at it, could you please use here SAFE_SETEUID(0) ?

> -	/* This includes the first (header) line */
> -	if ((fd = open("/proc/swaps", O_RDONLY)) == -1) {
> -		tst_brk(TFAIL | TERRNO,
> -			"Failed to find out existing number of swap files");
> -	}
> -	do {
> -		char *p = buf;
> -		res = read(fd, buf, BUFSIZ);
> -		if (res < 0) {
> -			tst_brk(TFAIL | TERRNO,
> -				 "Failed to find out existing number of swap files");
> -		}
> -		buf[res] = '\0';
> -		while ((p = strchr(p, '\n'))) {
> -			p++;
> -			swapfiles++;
> -		}
> -	} while (BUFSIZ <= res);
> -	close(fd);
> -	if (swapfiles)
> -		swapfiles--;	/* don't count the /proc/swaps header */
Explicitly mention "don't count the /proc/swaps header" would not hurt in the
previous commit where you use -1.
> -
> -	if (swapfiles < 0)
> -		tst_brk(TFAIL, "Failed to find existing number of swapfiles");
> -
>  	/* Determine how many more files are to be created */
> -	swapfiles = MAX_SWAPFILES - swapfiles;
> -	if (swapfiles > MAX_SWAPFILES)
> -		swapfiles = MAX_SWAPFILES;
> +	max_swapfiles = tst_max_swapfiles();
> +	used_swapfiles = tst_count_swaps();
> +	swapfiles = max_swapfiles - used_swapfiles;
Instead of used_swapfiles directly call tst_count_swaps() can be used.
> +	if (swapfiles > max_swapfiles)
> +		swapfiles = max_swapfiles;

Please add here blank line for readability.
>  	pid = SAFE_FORK();
>  	if (pid == 0) {
>  		/*create and turn on remaining swapfiles */

Kind regards,
Petr