[LTP] [PATCH 1/4] libswap: add known swap supported fs check

[LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Li Wang]

This introduce an enhancement to the library's is_swap_supported
function to check for filesystem compatibility before attempting
to create and enable a swap file.  A list of supported filesystems
is added (ext2, ext3, ext4, xfs, vfat, exfat, ntfs), and a check
against this list is performed to ensure that the swap operations
are only attempted on known compatible filesystems.

If the make_swapfile function fails, the error handling is now
more descriptive: it distinguishes between failures due to the
filesystem not supporting swap files and other types of failures.
Similarly, when attempting to enable the swap file with swapon,
the patch ensures that clearer error messages are provided in
cases where the operation is not supported by the filesystem.

Signed-off-by: Li Wang <liwang@redhat.com>
---
 libs/libltpswap/libswap.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
index 13610709e..623f2fb3c 100644
--- a/libs/libltpswap/libswap.c
+++ b/libs/libltpswap/libswap.c
@@ -12,6 +12,17 @@
 #include "libswap.h"
 #include "lapi/syscalls.h"
 
+static const char *const swap_supported_fs[] = {
+	"ext2",
+	"ext3",
+	"ext4",
+	"xfs",
+	"vfat",
+	"exfat",
+	"ntfs",
+	NULL
+};
+
 /*
  * Make a swap file
  */
@@ -40,23 +51,31 @@ int make_swapfile(const char *swapfile, int safe)
  */
 void is_swap_supported(const char *filename)
 {
+	int i, sw_support = 0;
 	int fibmap = tst_fibmap(filename);
 	long fs_type = tst_fs_type(filename);
 	const char *fstype = tst_fs_type_name(fs_type);
 
-	int ret = make_swapfile(filename, 1);
-	if (ret != 0) {
-		if (fibmap == 1)
-			tst_brk(TCONF, "mkswap on %s not supported", fstype);
-		else
-			tst_brk(TFAIL, "mkswap on %s failed", fstype);
+	for (i = 0; swap_supported_fs[i]; i++) {
+		if (!strcmp(fstype, swap_supported_fs[i])) {
+			sw_support = 1;
+			break;
+		}
 	}
 
+       int ret = make_swapfile(filename, 1);
+       if (ret != 0) {
+               if (fibmap == 1 && sw_support == 0)
+                       tst_brk(TCONF, "mkswap on %s not supported", fstype);
+               else
+                       tst_brk(TFAIL, "mkswap on %s failed", fstype);
+       }
+
 	TEST(tst_syscall(__NR_swapon, filename, 0));
 	if (TST_RET == -1) {
 		if (errno == EPERM)
 			tst_brk(TCONF, "Permission denied for swapon()");
-		else if (fibmap == 1 && errno == EINVAL)
+		else if (fibmap == 1 && errno == EINVAL && sw_support == 0)
 			tst_brk(TCONF, "Swapfile on %s not implemented", fstype);
 		else
 			tst_brk(TFAIL | TTERRNO, "swapon() on %s failed", fstype);
-- 
2.40.1


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

[LTP] [PATCH 2/4] swapon01: Test on all filesystems

[Li Wang]

From: Petr Vorel <pvorel@suse.cz>

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>
Signed-off-by: Li Wang <liwang@redhat.com>
---
 testcases/kernel/syscalls/swapon/swapon01.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/swapon/swapon01.c b/testcases/kernel/syscalls/swapon/swapon01.c
index e59fb20a1..e1fe50459 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,7 +18,8 @@
 #include "lapi/syscalls.h"
 #include "libswap.h"
 
-#define SWAP_FILE "swapfile01"
+#define MNTPOINT	"mntpoint"
+#define SWAP_FILE	MNTPOINT"/swapfile01"
 
 static void verify_swapon(void)
 {
@@ -36,8 +38,10 @@ static void setup(void)
 }
 
 static struct tst_test test = {
+	.mntpoint = MNTPOINT,
+	.mount_device = 1,
 	.needs_root = 1,
-	.needs_tmpdir = 1,
+	.all_filesystems = 1,
 	.test_all = verify_swapon,
 	.setup = setup
 };
-- 
2.40.1


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

[LTP] [PATCH 3/4] swapon01: Improving test with memory limits and swap reporting

[Li Wang]

This is target to create a more robust and controlled environment to test
the swapon system call. By introducing memory limits through cgroups and
filling memory with a known pattern, the test can better assess swapon
behavior when the system experiences memory pressure.

Additionally, the reporting of "SwapCached" memory before turning off the
swap file provides a clearer understanding of the swap system's state in
response to the test conditions.

Signed-off-by: Li Wang <liwang@redhat.com>
---
 testcases/kernel/syscalls/swapon/swapon01.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/testcases/kernel/syscalls/swapon/swapon01.c b/testcases/kernel/syscalls/swapon/swapon01.c
index e1fe50459..a74a5171e 100644
--- a/testcases/kernel/syscalls/swapon/swapon01.c
+++ b/testcases/kernel/syscalls/swapon/swapon01.c
@@ -20,11 +20,15 @@
 
 #define MNTPOINT	"mntpoint"
 #define SWAP_FILE	MNTPOINT"/swapfile01"
+#define TESTMEM		(1UL<<30)
 
 static void verify_swapon(void)
 {
 	TST_EXP_PASS(tst_syscall(__NR_swapon, SWAP_FILE, 0));
 
+	tst_pollute_memory(TESTMEM, 0x41);
+	tst_res(TINFO, "SwapCached: %ld Kb", SAFE_READ_MEMINFO("SwapCached:"));
+
 	if (TST_PASS && tst_syscall(__NR_swapoff, SWAP_FILE) != 0) {
 		tst_brk(TBROK | TERRNO,
 				"Failed to turn off swapfile, system reboot recommended");
@@ -35,6 +39,9 @@ static void setup(void)
 {
 	is_swap_supported(SWAP_FILE);
 	make_swapfile(SWAP_FILE, 0);
+
+	SAFE_CG_PRINTF(tst_cg, "cgroup.procs", "%d", getpid());
+	SAFE_CG_PRINTF(tst_cg, "memory.max", "%lu", TESTMEM);
 }
 
 static struct tst_test test = {
@@ -42,6 +49,7 @@ static struct tst_test test = {
 	.mount_device = 1,
 	.needs_root = 1,
 	.all_filesystems = 1,
+	.needs_cgroup_ctrls = (const char *const []){ "memory", NULL },
 	.test_all = verify_swapon,
 	.setup = setup
 };
-- 
2.40.1


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

[LTP] [PATCH 4/4] libswap: add Btrfs noCOW attribute setting for swap files

[Li Wang]

The patch aims to ensure swap files on Btrfs filesystems are created
with the appropriate FS_NOCOW_FL attribute, which is necessary to
disable CoW (Copy-on-Write) for swap files, perthe btrfs(5) manual page.
This change is gated behind a kernel version check to ensure compatibility
with the system's capabilities.

Signed-off-by: Li Wang <liwang@redhat.com>
---
 libs/libltpswap/libswap.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
index 623f2fb3c..0b1d9a227 100644
--- a/libs/libltpswap/libswap.c
+++ b/libs/libltpswap/libswap.c
@@ -4,6 +4,7 @@
  * Author: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
  */
 
+#include <linux/fs.h>
 #include <errno.h>
 
 #define TST_NO_DEFAULT_MAIN
@@ -23,11 +24,37 @@ static const char *const swap_supported_fs[] = {
 	NULL
 };
 
+static void set_nocow_attr(const char *filename)
+{
+	int fd;
+	int attrs;
+
+	fd = SAFE_OPEN(filename, O_RDONLY);
+
+	if (ioctl(fd, FS_IOC_GETFLAGS, &attrs) == -1) {
+		tst_res(TFAIL | TERRNO, "Error getting attributes");
+		close(fd);
+		return;
+	}
+
+	attrs |= FS_NOCOW_FL;
+
+	if (ioctl(fd, FS_IOC_SETFLAGS, &attrs) == -1)
+		tst_res(TFAIL | TERRNO, "Error setting FS_NOCOW_FL attribute");
+	else
+		tst_res(TINFO, "FS_NOCOW_FL attribute set on %s\n", filename);
+
+	close(fd);
+}
+
 /*
  * Make a swap file
  */
 int make_swapfile(const char *swapfile, int safe)
 {
+	long fs_type = tst_fs_type(swapfile);
+	const char *fstype = tst_fs_type_name(fs_type);
+
 	if (!tst_fs_has_free(".", sysconf(_SC_PAGESIZE) * 10, TST_BYTES))
 		tst_brk(TBROK, "Insufficient disk space to create swap file");
 
@@ -35,6 +62,14 @@ int make_swapfile(const char *swapfile, int safe)
 	if (tst_fill_file(swapfile, 0, sysconf(_SC_PAGESIZE), 10) != 0)
 		tst_brk(TBROK, "Failed to create swapfile");
 
+	/* Btrfs file need set 'nocow' attribute */
+	if (strcmp(fstype, "btrfs") == 0) {
+		if (tst_kvercmp(5, 0, 0) > 0)
+			set_nocow_attr(swapfile);
+		else
+			tst_brk(TCONF, "Swapfile on %s not implemented", fstype);
+	}
+
 	/* make the file swapfile */
 	const char *argv[2 + 1];
 	argv[0] = "mkswap";
-- 
2.40.1


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

Re: [LTP] [PATCH 4/4] libswap: add Btrfs noCOW attribute setting for swap files

[Petr Vorel]

Hi Li,

> The patch aims to ensure swap files on Btrfs filesystems are created
> with the appropriate FS_NOCOW_FL attribute, which is necessary to
> disable CoW (Copy-on-Write) for swap files, perthe btrfs(5) manual page.
> This change is gated behind a kernel version check to ensure compatibility
> with the system's capabilities.

> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
>  libs/libltpswap/libswap.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)

> diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> index 623f2fb3c..0b1d9a227 100644
> --- a/libs/libltpswap/libswap.c
> +++ b/libs/libltpswap/libswap.c
> @@ -4,6 +4,7 @@
>   * Author: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
>   */

> +#include <linux/fs.h>
>  #include <errno.h>

>  #define TST_NO_DEFAULT_MAIN
> @@ -23,11 +24,37 @@ static const char *const swap_supported_fs[] = {
>  	NULL
>  };

> +static void set_nocow_attr(const char *filename)
> +{
> +	int fd;
> +	int attrs;
> +
> +	fd = SAFE_OPEN(filename, O_RDONLY);
> +
> +	if (ioctl(fd, FS_IOC_GETFLAGS, &attrs) == -1) {
> +		tst_res(TFAIL | TERRNO, "Error getting attributes");
I guess we don't want to break all testing due ioctl failure, right?
Otherwise I would use SAFE_IOCTL().

> +		close(fd);
> +		return;
> +	}
> +
> +	attrs |= FS_NOCOW_FL;
> +
> +	if (ioctl(fd, FS_IOC_SETFLAGS, &attrs) == -1)
> +		tst_res(TFAIL | TERRNO, "Error setting FS_NOCOW_FL attribute");
And here as well.

> +	else
> +		tst_res(TINFO, "FS_NOCOW_FL attribute set on %s\n", filename);
This is redundant new line, please remove \n before merging.

nit: make check reports various of formatting issues (on master there are only
two). Would you dare to fix them? (some of them are added in the first commit).

make check-libswap
CHECK libs/libltpswap/libswap.c
libswap.c:75: WARNING: Missing a blank line after declarations
libswap.c:101: WARNING: please, no spaces at the start of a line
libswap.c:102: WARNING: Missing a blank line after declarations
libswap.c:102: WARNING: please, no spaces at the start of a line
libswap.c:102: WARNING: suspect code indent for conditional statements (7, 15)
libswap.c:103: ERROR: code indent should use tabs where possible
libswap.c:103: WARNING: please, no spaces at the start of a line
libswap.c:103: WARNING: suspect code indent for conditional statements (15, 23)
libswap.c:104: ERROR: code indent should use tabs where possible
libswap.c:104: WARNING: please, no spaces at the start of a line
libswap.c:105: ERROR: code indent should use tabs where possible
libswap.c:105: WARNING: please, no spaces at the start of a line
libswap.c:105: WARNING: suspect code indent for conditional statements (15, 23)
libswap.c:106: ERROR: code indent should use tabs where possible
libswap.c:106: WARNING: please, no spaces at the start of a line
libswap.c:107: WARNING: please, no spaces at the start of a line


> +
> +	close(fd);
> +}
> +
>  /*
>   * Make a swap file
>   */
>  int make_swapfile(const char *swapfile, int safe)
>  {
> +	long fs_type = tst_fs_type(swapfile);
> +	const char *fstype = tst_fs_type_name(fs_type);
> +
>  	if (!tst_fs_has_free(".", sysconf(_SC_PAGESIZE) * 10, TST_BYTES))
>  		tst_brk(TBROK, "Insufficient disk space to create swap file");

> @@ -35,6 +62,14 @@ int make_swapfile(const char *swapfile, int safe)
>  	if (tst_fill_file(swapfile, 0, sysconf(_SC_PAGESIZE), 10) != 0)
>  		tst_brk(TBROK, "Failed to create swapfile");

> +	/* Btrfs file need set 'nocow' attribute */
> +	if (strcmp(fstype, "btrfs") == 0) {

Maybe using just fs_type (long), i.e. avoid conversion to char * via
tst_fs_type_name()? Or am I missing something?

	long fs_type = tst_fs_type(swapfile);
	...
	if (fs_type == TST_BTRFS_MAGIC) {


Kind regards,
Petr

> +		if (tst_kvercmp(5, 0, 0) > 0)
> +			set_nocow_attr(swapfile);
> +		else
> +			tst_brk(TCONF, "Swapfile on %s not implemented", fstype);
> +	}
> +
>  	/* make the file swapfile */
>  	const char *argv[2 + 1];
>  	argv[0] = "mkswap";

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

Re: [LTP] [PATCH 4/4] libswap: add Btrfs noCOW attribute setting for swap files

[Petr Vorel]

Hi Li,

testing the patchset on SLES, at least swapon01 reports some results
on systems with TMPDIR on tmpfs or btrfs.

Therefore I agree it'd be good to use the same approach for all swapon* and
swapoff* tests.

I would be ok to get it to the release (generally patchset LGTM), but depends on
your and Cyril's time (no problem to postpone it).

Kind regards,
Petr

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

Re: [LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Petr Vorel]

Hi Li, Cyril,

> This introduce an enhancement to the library's is_swap_supported
> function to check for filesystem compatibility before attempting
> to create and enable a swap file.  A list of supported filesystems
> is added (ext2, ext3, ext4, xfs, vfat, exfat, ntfs), and a check
> against this list is performed to ensure that the swap operations
> are only attempted on known compatible filesystems.

> If the make_swapfile function fails, the error handling is now
> more descriptive: it distinguishes between failures due to the
> filesystem not supporting swap files and other types of failures.
> Similarly, when attempting to enable the swap file with swapon,
> the patch ensures that clearer error messages are provided in
> cases where the operation is not supported by the filesystem.

> Signed-off-by: Li Wang <liwang@redhat.com>
> ---
>  libs/libltpswap/libswap.c | 33 ++++++++++++++++++++++++++-------
>  1 file changed, 26 insertions(+), 7 deletions(-)

> diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> index 13610709e..623f2fb3c 100644
> --- a/libs/libltpswap/libswap.c
> +++ b/libs/libltpswap/libswap.c
> @@ -12,6 +12,17 @@
>  #include "libswap.h"
>  #include "lapi/syscalls.h"

> +static const char *const swap_supported_fs[] = {
> +	"ext2",
> +	"ext3",
> +	"ext4",
> +	"xfs",
> +	"vfat",
> +	"exfat",
> +	"ntfs",
> +	NULL
> +};
> +
>  /*
>   * Make a swap file
>   */
> @@ -40,23 +51,31 @@ int make_swapfile(const char *swapfile, int safe)
>   */
>  void is_swap_supported(const char *filename)
>  {
> +	int i, sw_support = 0;
>  	int fibmap = tst_fibmap(filename);
Just a note unrelated to this patchset. When testing on SLES kernel based on
5.3.18 we still get TCONF due missing FIBMAP ioctl support:

tst_test.c:1669: TINFO: === Testing on btrfs ===
tst_test.c:1118: TINFO: Formatting /dev/loop0 with btrfs opts='' extra opts=''
tst_test.c:1132: TINFO: Mounting /dev/loop0 to /tmp/LTP_swazaqF1L/mntpoint fstyp=btrfs flags=0
tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
libswap.c:45: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01

libswap.c:114: TCONF: Swapfile on btrfs not implemented

Am I wrong or could it be solved with FIEMAP (<linux/fiemap.h>)?
If yes, I wonder if we should fallback on btrfs when FIBMAP is missing

https://www.kernel.org/doc/Documentation/filesystems/fiemap.txt
https://unix.stackexchange.com/questions/623859/how-do-you-find-the-physical-offset-for-a-file-in-btrfs

Kind regards,
Petr

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

Re: [LTP] [PATCH 4/4] libswap: add Btrfs noCOW attribute setting for swap files

[Li Wang]

Hi Petr,

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

> Hi Li,
>
> > The patch aims to ensure swap files on Btrfs filesystems are created
> > with the appropriate FS_NOCOW_FL attribute, which is necessary to
> > disable CoW (Copy-on-Write) for swap files, perthe btrfs(5) manual page.
> > This change is gated behind a kernel version check to ensure
> compatibility
> > with the system's capabilities.
>
> > Signed-off-by: Li Wang <liwang@redhat.com>
> > ---
> >  libs/libltpswap/libswap.c | 35 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 35 insertions(+)
>
> > diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> > index 623f2fb3c..0b1d9a227 100644
> > --- a/libs/libltpswap/libswap.c
> > +++ b/libs/libltpswap/libswap.c
> > @@ -4,6 +4,7 @@
> >   * Author: Stanislav Kholmanskikh <stanislav.kholmanskikh@oracle.com>
> >   */
>
> > +#include <linux/fs.h>
> >  #include <errno.h>
>
> >  #define TST_NO_DEFAULT_MAIN
> > @@ -23,11 +24,37 @@ static const char *const swap_supported_fs[] = {
> >       NULL
> >  };
>
> > +static void set_nocow_attr(const char *filename)
> > +{
> > +     int fd;
> > +     int attrs;
> > +
> > +     fd = SAFE_OPEN(filename, O_RDONLY);
> > +
> > +     if (ioctl(fd, FS_IOC_GETFLAGS, &attrs) == -1) {
> > +             tst_res(TFAIL | TERRNO, "Error getting attributes");
> I guess we don't want to break all testing due ioctl failure, right?
> Otherwise I would use SAFE_IOCTL().
>

+1, thanks.


> > +             close(fd);
> > +             return;
> > +     }
> > +
> > +     attrs |= FS_NOCOW_FL;
> > +
> > +     if (ioctl(fd, FS_IOC_SETFLAGS, &attrs) == -1)
> > +             tst_res(TFAIL | TERRNO, "Error setting FS_NOCOW_FL
> attribute");
> And here as well.
>

+1


> > +     else
> > +             tst_res(TINFO, "FS_NOCOW_FL attribute set on %s\n",
> filename);
> This is redundant new line, please remove \n before merging.
>

+1, I would send a V2 for 4/4, because I found additional issue
that the "btrfs" should be added in swap_supported_fs[] as well.


> nit: make check reports various of formatting issues (on master there are
> only
> two). Would you dare to fix them? (some of them are added in the first
> commit).
>

+1


> make check-libswap
> CHECK libs/libltpswap/libswap.c
> libswap.c:75: WARNING: Missing a blank line after declarations
> libswap.c:101: WARNING: please, no spaces at the start of a line
> libswap.c:102: WARNING: Missing a blank line after declarations
> libswap.c:102: WARNING: please, no spaces at the start of a line
> libswap.c:102: WARNING: suspect code indent for conditional statements (7,
> 15)
> libswap.c:103: ERROR: code indent should use tabs where possible
> libswap.c:103: WARNING: please, no spaces at the start of a line
> libswap.c:103: WARNING: suspect code indent for conditional statements
> (15, 23)
> libswap.c:104: ERROR: code indent should use tabs where possible
> libswap.c:104: WARNING: please, no spaces at the start of a line
> libswap.c:105: ERROR: code indent should use tabs where possible
> libswap.c:105: WARNING: please, no spaces at the start of a line
> libswap.c:105: WARNING: suspect code indent for conditional statements
> (15, 23)
> libswap.c:106: ERROR: code indent should use tabs where possible
> libswap.c:106: WARNING: please, no spaces at the start of a line
> libswap.c:107: WARNING: please, no spaces at the start of a line
>
>
> > +
> > +     close(fd);
> > +}
> > +
> >  /*
> >   * Make a swap file
> >   */
> >  int make_swapfile(const char *swapfile, int safe)
> >  {
> > +     long fs_type = tst_fs_type(swapfile);
> > +     const char *fstype = tst_fs_type_name(fs_type);
> > +
> >       if (!tst_fs_has_free(".", sysconf(_SC_PAGESIZE) * 10, TST_BYTES))
> >               tst_brk(TBROK, "Insufficient disk space to create swap
> file");
>
> > @@ -35,6 +62,14 @@ int make_swapfile(const char *swapfile, int safe)
> >       if (tst_fill_file(swapfile, 0, sysconf(_SC_PAGESIZE), 10) != 0)
> >               tst_brk(TBROK, "Failed to create swapfile");
>
> > +     /* Btrfs file need set 'nocow' attribute */
> > +     if (strcmp(fstype, "btrfs") == 0) {
>
> Maybe using just fs_type (long), i.e. avoid conversion to char * via
> tst_fs_type_name()? Or am I missing something?
>
>         long fs_type = tst_fs_type(swapfile);
>         ...
>         if (fs_type == TST_BTRFS_MAGIC) {
>

Good point.


>
> Kind regards,
> Petr
>
> > +             if (tst_kvercmp(5, 0, 0) > 0)
> > +                     set_nocow_attr(swapfile);
> > +             else
> > +                     tst_brk(TCONF, "Swapfile on %s not implemented",
> fstype);
> > +     }
> > +
> >       /* make the file swapfile */
> >       const char *argv[2 + 1];
> >       argv[0] = "mkswap";
>
>

-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH 3/4] swapon01: Improving test with memory limits and swap reporting

[Petr Vorel]

Hi Li,

> This is target to create a more robust and controlled environment to test
> the swapon system call. By introducing memory limits through cgroups and
> filling memory with a known pattern, the test can better assess swapon
> behavior when the system experiences memory pressure.

> Additionally, the reporting of "SwapCached" memory before turning off the
> swap file provides a clearer understanding of the swap system's state in
> response to the test conditions.

LGTM, thanks!

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

Kind regards,
Petr

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

Re: [LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Li Wang]

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

> Hi Li, Cyril,
>
> > This introduce an enhancement to the library's is_swap_supported
> > function to check for filesystem compatibility before attempting
> > to create and enable a swap file.  A list of supported filesystems
> > is added (ext2, ext3, ext4, xfs, vfat, exfat, ntfs), and a check
> > against this list is performed to ensure that the swap operations
> > are only attempted on known compatible filesystems.
>
> > If the make_swapfile function fails, the error handling is now
> > more descriptive: it distinguishes between failures due to the
> > filesystem not supporting swap files and other types of failures.
> > Similarly, when attempting to enable the swap file with swapon,
> > the patch ensures that clearer error messages are provided in
> > cases where the operation is not supported by the filesystem.
>
> > Signed-off-by: Li Wang <liwang@redhat.com>
> > ---
> >  libs/libltpswap/libswap.c | 33 ++++++++++++++++++++++++++-------
> >  1 file changed, 26 insertions(+), 7 deletions(-)
>
> > diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> > index 13610709e..623f2fb3c 100644
> > --- a/libs/libltpswap/libswap.c
> > +++ b/libs/libltpswap/libswap.c
> > @@ -12,6 +12,17 @@
> >  #include "libswap.h"
> >  #include "lapi/syscalls.h"
>
> > +static const char *const swap_supported_fs[] = {
> > +     "ext2",
> > +     "ext3",
> > +     "ext4",
> > +     "xfs",
> > +     "vfat",
> > +     "exfat",
> > +     "ntfs",
> > +     NULL
> > +};
> > +
> >  /*
> >   * Make a swap file
> >   */
> > @@ -40,23 +51,31 @@ int make_swapfile(const char *swapfile, int safe)
> >   */
> >  void is_swap_supported(const char *filename)
> >  {
> > +     int i, sw_support = 0;
> >       int fibmap = tst_fibmap(filename);
> Just a note unrelated to this patchset. When testing on SLES kernel based
> on
> 5.3.18 we still get TCONF due missing FIBMAP ioctl support:
>
> tst_test.c:1669: TINFO: === Testing on btrfs ===
> tst_test.c:1118: TINFO: Formatting /dev/loop0 with btrfs opts='' extra
> opts=''
> tst_test.c:1132: TINFO: Mounting /dev/loop0 to /tmp/LTP_swazaqF1L/mntpoint
> fstyp=btrfs flags=0
> tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
> libswap.c:45: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01
>
> libswap.c:114: TCONF: Swapfile on btrfs not implemented
>

Interesting, can you try with the below command manually to see if swapfile
is supported correctly on the BTRFS?
"cut from man 5 btrfs"

          # truncate -s 0 swapfile
          # chattr +C swapfile
          # fallocate -l 2G swapfile
          # chmod 0600 swapfile
          # mkswap swapfile
          # swapon swapfile



>
> Am I wrong or could it be solved with FIEMAP (<linux/fiemap.h>)?
> If yes, I wonder if we should fallback on btrfs when FIBMAP is missing
>
> https://www.kernel.org/doc/Documentation/filesystems/fiemap.txt
>
> https://unix.stackexchange.com/questions/623859/how-do-you-find-the-physical-offset-for-a-file-in-btrfs
>
> Kind regards,
> Petr
>
>

-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Petr Vorel]

Hi Li,

> This introduce an enhancement to the library's is_swap_supported
> function to check for filesystem compatibility before attempting
> to create and enable a swap file.  A list of supported filesystems
> is added (ext2, ext3, ext4, xfs, vfat, exfat, ntfs), and a check
> against this list is performed to ensure that the swap operations
> are only attempted on known compatible filesystems.

> If the make_swapfile function fails, the error handling is now
> more descriptive: it distinguishes between failures due to the
> filesystem not supporting swap files and other types of failures.
> Similarly, when attempting to enable the swap file with swapon,
> the patch ensures that clearer error messages are provided in
> cases where the operation is not supported by the filesystem.

+1

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

formatting issues (tabs) are actually mostly from this commit, please fix them
before merge.

libswap.c:40: WARNING: Missing a blank line after declarations
libswap.c:66: WARNING: please, no spaces at the start of a line
libswap.c:67: WARNING: Missing a blank line after declarations
libswap.c:67: WARNING: please, no spaces at the start of a line
libswap.c:67: WARNING: suspect code indent for conditional statements (7, 15)
libswap.c:68: ERROR: code indent should use tabs where possible
libswap.c:68: WARNING: please, no spaces at the start of a line
libswap.c:68: WARNING: suspect code indent for conditional statements (15, 23)
libswap.c:69: ERROR: code indent should use tabs where possible
libswap.c:69: WARNING: please, no spaces at the start of a line
libswap.c:70: ERROR: code indent should use tabs where possible
libswap.c:70: WARNING: please, no spaces at the start of a line
libswap.c:70: WARNING: suspect code indent for conditional statements (15, 23)
libswap.c:71: ERROR: code indent should use tabs where possible
libswap.c:71: WARNING: please, no spaces at the start of a line
libswap.c:72: WARNING: please, no spaces at the start of a line

Kind regards,
Petr

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

Re: [LTP] [PATCH 4/4] libswap: add Btrfs noCOW attribute setting for swap files

[Li Wang]

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

> Hi Li,
>
> testing the patchset on SLES, at least swapon01 reports some results
> on systems with TMPDIR on tmpfs or btrfs.
>
> Therefore I agree it'd be good to use the same approach for all swapon* and
> swapoff* tests.
>
> I would be ok to get it to the release (generally patchset LGTM), but
> depends on
> your and Cyril's time (no problem to postpone it).
>

Thanks a lot. I can send out the V2 by the end of today.


-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Li Wang]

Hi Petr,

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

> Hi Li,
>
> > This introduce an enhancement to the library's is_swap_supported
> > function to check for filesystem compatibility before attempting
> > to create and enable a swap file.  A list of supported filesystems
> > is added (ext2, ext3, ext4, xfs, vfat, exfat, ntfs), and a check
> > against this list is performed to ensure that the swap operations
> > are only attempted on known compatible filesystems.
>
> > If the make_swapfile function fails, the error handling is now
> > more descriptive: it distinguishes between failures due to the
> > filesystem not supporting swap files and other types of failures.
> > Similarly, when attempting to enable the swap file with swapon,
> > the patch ensures that clearer error messages are provided in
> > cases where the operation is not supported by the filesystem.
>
> +1
>
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> formatting issues (tabs) are actually mostly from this commit, please fix
> them
> before merge.
>

Thanks very much, Petr.

The patchset V2 (based on your suggestions) is published in my git branch.
In case you want to do more tests today.

https://github.com/wangli5665/ltp/tree/libswap

And, I prefer to wait for Cryil's feedback before posting them in ML:)


-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Petr Vorel]

Hi Li, Cyril,

> Hi Li, Cyril,
...
> >  void is_swap_supported(const char *filename)
> >  {
> > +	int i, sw_support = 0;
> >  	int fibmap = tst_fibmap(filename);
> Just a note unrelated to this patchset. When testing on SLES kernel based on
> 5.3.18 we still get TCONF due missing FIBMAP ioctl support:

> tst_test.c:1669: TINFO: === Testing on btrfs ===
> tst_test.c:1118: TINFO: Formatting /dev/loop0 with btrfs opts='' extra opts=''
> tst_test.c:1132: TINFO: Mounting /dev/loop0 to /tmp/LTP_swazaqF1L/mntpoint fstyp=btrfs flags=0
> tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
> libswap.c:45: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01

> libswap.c:114: TCONF: Swapfile on btrfs not implemented

Hm, what makes me wonder is that btrfs does not support FIBMAP even on current
openSUSE Tumbleweed with 6.7.0 kernel:

# TMPDIR=/var/tmp/ LTP_SINGLE_FS_TYPE=btrfs ./swapon01
...
tst_test.c:1669: TINFO: === Testing on btrfs ===
tst_test.c:1117: TINFO: Formatting /dev/loop0 with btrfs opts='' extra opts=''
tst_test.c:1131: TINFO: Mounting /dev/loop0 to /var/tmp/LTP_swaMBctpq/mntpoint fstyp=btrfs flags=0
tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
libswap.c:45: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01

libswap.c:114: TCONF: Swapfile on btrfs not implemented

$ df -hT /var/tmp
Filesystem     Type   Size  Used Avail Use% Mounted on
/dev/vda2      btrfs   28G   19G  3.3G  86% /var

$ uname -r 
6.7.0-9.gaedda80-default

I thought the problem is that underlying fs is btrfs, but testing on Debian
on 6.6.x with TMPDIR on ext4 does not bring FIBMAP support:

# TMPDIR=/var/tmp LTP_SINGLE_FS_TYPE=btrfs /opt/ltp/testcases/bin/swapon01

tst_test.c:1669: TINFO: === Testing on btrfs ===
tst_test.c:1117: TINFO: Formatting /dev/loop0 with btrfs opts='' extra opts=''
tst_test.c:1131: TINFO: Mounting /dev/loop0 to /var/tmp/LTP_swaZf8FN6/mntpoint fstyp=btrfs flags=0
tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
libswap.c:45: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01

libswap.c:114: TCONF: Swapfile on btrfs not implemented

...

Kind regards,
Petr

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

Re: [LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Li Wang]

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

> Hi Li, Cyril,
>
> > Hi Li, Cyril,
> ...
> > >  void is_swap_supported(const char *filename)
> > >  {
> > > +   int i, sw_support = 0;
> > >     int fibmap = tst_fibmap(filename);
> > Just a note unrelated to this patchset. When testing on SLES kernel
> based on
> > 5.3.18 we still get TCONF due missing FIBMAP ioctl support:
>
> > tst_test.c:1669: TINFO: === Testing on btrfs ===
> > tst_test.c:1118: TINFO: Formatting /dev/loop0 with btrfs opts='' extra
> opts=''
> > tst_test.c:1132: TINFO: Mounting /dev/loop0 to
> /tmp/LTP_swazaqF1L/mntpoint fstyp=btrfs flags=0
> > tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
> > libswap.c:45: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01
>
> > libswap.c:114: TCONF: Swapfile on btrfs not implemented
>
> Hm, what makes me wonder is that btrfs does not support FIBMAP even on
> current
> openSUSE Tumbleweed with 6.7.0 kernel:
>


What we made use of FIBMAP is just to determine the
file's blocks are contiguous on the disk then we can assume
the swapfile support or not with that filesystem.

As you mentioned, maybe FIEMAP is an additional way
to check if a file's blocks are contiguous, I guess we might
need to improve the check in a simple function with both
FIEMAP and FIBMAP to guarantee we get the necessary info.

And yes, it can be achieved in a separate patch.





>
> # TMPDIR=/var/tmp/ LTP_SINGLE_FS_TYPE=btrfs ./swapon01
> ...
> tst_test.c:1669: TINFO: === Testing on btrfs ===
> tst_test.c:1117: TINFO: Formatting /dev/loop0 with btrfs opts='' extra
> opts=''
> tst_test.c:1131: TINFO: Mounting /dev/loop0 to
> /var/tmp/LTP_swaMBctpq/mntpoint fstyp=btrfs flags=0
> tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
> libswap.c:45: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01
>
> libswap.c:114: TCONF: Swapfile on btrfs not implemented
>
> $ df -hT /var/tmp
> Filesystem     Type   Size  Used Avail Use% Mounted on
> /dev/vda2      btrfs   28G   19G  3.3G  86% /var
>
> $ uname -r
> 6.7.0-9.gaedda80-default
>
> I thought the problem is that underlying fs is btrfs, but testing on Debian
> on 6.6.x with TMPDIR on ext4 does not bring FIBMAP support:
>
> # TMPDIR=/var/tmp LTP_SINGLE_FS_TYPE=btrfs /opt/ltp/testcases/bin/swapon01
>
> tst_test.c:1669: TINFO: === Testing on btrfs ===
> tst_test.c:1117: TINFO: Formatting /dev/loop0 with btrfs opts='' extra
> opts=''
> tst_test.c:1131: TINFO: Mounting /dev/loop0 to
> /var/tmp/LTP_swaZf8FN6/mntpoint fstyp=btrfs flags=0
> tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
> libswap.c:45: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01
>
> libswap.c:114: TCONF: Swapfile on btrfs not implemented
>
> ...
>
> Kind regards,
> Petr
>
>

-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Petr Vorel]

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

> > Hi Li, Cyril,

> > > This introduce an enhancement to the library's is_swap_supported
> > > function to check for filesystem compatibility before attempting
> > > to create and enable a swap file.  A list of supported filesystems
> > > is added (ext2, ext3, ext4, xfs, vfat, exfat, ntfs), and a check
> > > against this list is performed to ensure that the swap operations
> > > are only attempted on known compatible filesystems.

> > > If the make_swapfile function fails, the error handling is now
> > > more descriptive: it distinguishes between failures due to the
> > > filesystem not supporting swap files and other types of failures.
> > > Similarly, when attempting to enable the swap file with swapon,
> > > the patch ensures that clearer error messages are provided in
> > > cases where the operation is not supported by the filesystem.

> > > Signed-off-by: Li Wang <liwang@redhat.com>
> > > ---
> > >  libs/libltpswap/libswap.c | 33 ++++++++++++++++++++++++++-------
> > >  1 file changed, 26 insertions(+), 7 deletions(-)

> > > diff --git a/libs/libltpswap/libswap.c b/libs/libltpswap/libswap.c
> > > index 13610709e..623f2fb3c 100644
> > > --- a/libs/libltpswap/libswap.c
> > > +++ b/libs/libltpswap/libswap.c
> > > @@ -12,6 +12,17 @@
> > >  #include "libswap.h"
> > >  #include "lapi/syscalls.h"

> > > +static const char *const swap_supported_fs[] = {
> > > +     "ext2",
> > > +     "ext3",
> > > +     "ext4",
> > > +     "xfs",
> > > +     "vfat",
> > > +     "exfat",
> > > +     "ntfs",
> > > +     NULL
> > > +};
> > > +
> > >  /*
> > >   * Make a swap file
> > >   */
> > > @@ -40,23 +51,31 @@ int make_swapfile(const char *swapfile, int safe)
> > >   */
> > >  void is_swap_supported(const char *filename)
> > >  {
> > > +     int i, sw_support = 0;
> > >       int fibmap = tst_fibmap(filename);
> > Just a note unrelated to this patchset. When testing on SLES kernel based
> > on
> > 5.3.18 we still get TCONF due missing FIBMAP ioctl support:

> > tst_test.c:1669: TINFO: === Testing on btrfs ===
> > tst_test.c:1118: TINFO: Formatting /dev/loop0 with btrfs opts='' extra
> > opts=''
> > tst_test.c:1132: TINFO: Mounting /dev/loop0 to /tmp/LTP_swazaqF1L/mntpoint
> > fstyp=btrfs flags=0
> > tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
> > libswap.c:45: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01

> > libswap.c:114: TCONF: Swapfile on btrfs not implemented


> Interesting, can you try with the below command manually to see if swapfile
> is supported correctly on the BTRFS?
> "cut from man 5 btrfs"

>           # truncate -s 0 swapfile
>           # chattr +C swapfile
>           # fallocate -l 2G swapfile
>           # chmod 0600 swapfile
>           # mkswap swapfile
>           # swapon swapfile

It works (tested on openSUSE Tumbleweed with 6.6.11 which also reported FIBMAP
ioctl is NOT supported), on file created with:

dd if=/dev/zero of=swapfile bs=400M count=1

# swapon -s
Filename				Type		Size		Used		Priority
/dev/vda3                               partition	2098152		34148		-2
/root/swapfile                          file		2097148		0		-3

# df -hT .
Filesystem     Type   Size  Used Avail Use% Mounted on
/dev/vda2      btrfs   28G   21G  1.4G  95% /root

Kind regards,
Petr

> > Am I wrong or could it be solved with FIEMAP (<linux/fiemap.h>)?
> > If yes, I wonder if we should fallback on btrfs when FIBMAP is missing

> > https://www.kernel.org/doc/Documentation/filesystems/fiemap.txt

> > https://unix.stackexchange.com/questions/623859/how-do-you-find-the-physical-offset-for-a-file-in-btrfs

> > Kind regards,
> > Petr

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

Re: [LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Petr Vorel]

> Hi Petr,

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

> > Hi Li,

> > > This introduce an enhancement to the library's is_swap_supported
> > > function to check for filesystem compatibility before attempting
> > > to create and enable a swap file.  A list of supported filesystems
> > > is added (ext2, ext3, ext4, xfs, vfat, exfat, ntfs), and a check
> > > against this list is performed to ensure that the swap operations
> > > are only attempted on known compatible filesystems.

> > > If the make_swapfile function fails, the error handling is now
> > > more descriptive: it distinguishes between failures due to the
> > > filesystem not supporting swap files and other types of failures.
> > > Similarly, when attempting to enable the swap file with swapon,
> > > the patch ensures that clearer error messages are provided in
> > > cases where the operation is not supported by the filesystem.

> > +1

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

> > formatting issues (tabs) are actually mostly from this commit, please fix
> > them
> > before merge.


> Thanks very much, Petr.

> The patchset V2 (based on your suggestions) is published in my git branch.
> In case you want to do more tests today.

Thanks, I already scheduled tests, let you know later today.

> https://github.com/wangli5665/ltp/tree/libswap

> And, I prefer to wait for Cryil's feedback before posting them in ML:)

+1

Kind regards,
Petr

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

Re: [LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Li Wang]

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

> > Hi Petr,
>
> > On Mon, Jan 22, 2024 at 5:14 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> > > Hi Li,
>
> > > > This introduce an enhancement to the library's is_swap_supported
> > > > function to check for filesystem compatibility before attempting
> > > > to create and enable a swap file.  A list of supported filesystems
> > > > is added (ext2, ext3, ext4, xfs, vfat, exfat, ntfs), and a check
> > > > against this list is performed to ensure that the swap operations
> > > > are only attempted on known compatible filesystems.
>
> > > > If the make_swapfile function fails, the error handling is now
> > > > more descriptive: it distinguishes between failures due to the
> > > > filesystem not supporting swap files and other types of failures.
> > > > Similarly, when attempting to enable the swap file with swapon,
> > > > the patch ensures that clearer error messages are provided in
> > > > cases where the operation is not supported by the filesystem.
>
> > > +1
>
> > > Reviewed-by: Petr Vorel <pvorel@suse.cz>
>
> > > formatting issues (tabs) are actually mostly from this commit, please
> fix
> > > them
> > > before merge.
>
>
> > Thanks very much, Petr.
>
> > The patchset V2 (based on your suggestions) is published in my git
> branch.
> > In case you want to do more tests today.
>
> Thanks, I already scheduled tests, let you know later today.
>

Great, and FYI.

I just pushed a new commit to resolve the FIBMAP (unsupported on BTRFS)
problem on my ltp:libswap branch.

https://github.com/wangli5665/ltp/commit/699711bfe8c8dbc3597c46587345fa1197c054c9

Hope you can try on your side and give some feedback :).



> > https://github.com/wangli5665/ltp/tree/libswap
>
> > And, I prefer to wait for Cryil's feedback before posting them in ML:)
>
> +1
>
> Kind regards,
> Petr
>
>

-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Petr Vorel]

Hi Li,

> Great, and FYI.

> I just pushed a new commit to resolve the FIBMAP (unsupported on BTRFS)
> problem on my ltp:libswap branch.

> https://github.com/wangli5665/ltp/commit/699711bfe8c8dbc3597c46587345fa1197c054c9

> Hope you can try on your side and give some feedback :).

Unfortunately regardless of kernel version it fails:

# LTP_SINGLE_FS_TYPE=btrfs ./swapon01
...
tst_test.c:1669: TINFO: === Testing on btrfs ===
tst_test.c:1117: TINFO: Formatting /dev/loop0 with btrfs opts='' extra opts=''
tst_test.c:1131: TINFO: Mounting /dev/loop0 to /tmp/LTP_swa497AKp/mntpoint fstyp=btrfs flags=0
tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
libswap.c:93: TINFO: File 'mntpoint/swapfile01' is not contiguous
libswap.c:34: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01
libswap.c:169: TFAIL: swapon() on btrfs failed: EINVAL (22)

Also, tst_brk() in is_swap_supported() causes test to fail quickly.
Given similar Martin's fix to shell API I merged today, which use tst_res +
return instead of tst_brk [1] I suggest to use tst_res + change return function
from void to int and handle another return in the test. That will allow to test
other filesystems (which is useful to see, if particular filesystem code is
broken or or VFS or some common code reused by all filesystems). This is the
reason why I don't think "quit as early as possible" is a good approach when
testing on all filesystems.

Kind regards,
Petr

[1] https://github.com/linux-test-project/ltp/commit/5c73ad84f3e6db9a965df3ed4a846352136ed990

> > > https://github.com/wangli5665/ltp/tree/libswap

> > > And, I prefer to wait for Cryil's feedback before posting them in ML:)

> > +1

> > Kind regards,
> > Petr

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

Re: [LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Li Wang]

Hi Petr,

On Tue, Jan 23, 2024 at 4:23 AM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> > Great, and FYI.
>
> > I just pushed a new commit to resolve the FIBMAP (unsupported on BTRFS)
> > problem on my ltp:libswap branch.
>
> >
> https://github.com/wangli5665/ltp/commit/699711bfe8c8dbc3597c46587345fa1197c054c9
>
> > Hope you can try on your side and give some feedback :).
>
> Unfortunately regardless of kernel version it fails:
>
> # LTP_SINGLE_FS_TYPE=btrfs ./swapon01
> ...
> tst_test.c:1669: TINFO: === Testing on btrfs ===
> tst_test.c:1117: TINFO: Formatting /dev/loop0 with btrfs opts='' extra
> opts=''
> tst_test.c:1131: TINFO: Mounting /dev/loop0 to /tmp/LTP_swa497AKp/mntpoint
> fstyp=btrfs flags=0
> tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
> libswap.c:93: TINFO: File 'mntpoint/swapfile01' is not contiguous
> libswap.c:34: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01
> libswap.c:169: TFAIL: swapon() on btrfs failed: EINVAL (22)
>


Thanks for the quick feedback, very useful.

After looking over the code, I think it's because the tst_fill_file didn't
create a contiguous file at the beginning, so the solution is probably
to define a new function just used by creating a contiguous file for btrfs
in the libswap.c.

After a tiny amend, it works on a fedora38.

$ df -Th |grep btrfs
/dev/nvme0n1p3 btrfs     476G  112G  362G  24% /
/dev/nvme0n1p3 btrfs     476G  112G  362G  24% /home

$ sudo LTP_SINGLE_FS_TYPE=btrfs ./swapon01
tst_device.c:96: TINFO: Found free device 0 '/dev/loop0'
tst_test.c:1709: TINFO: LTP version: 20230929-292-g699711bfe
tst_test.c:1593: TINFO: Timeout per run is 0h 00m 30s
tst_supported_fs_types.c:161: TINFO: WARNING: testing only btrfs
tst_supported_fs_types.c:97: TINFO: Kernel supports btrfs
tst_supported_fs_types.c:62: TINFO: mkfs.btrfs does exist

tst_test.c:1669: TINFO: === Testing on btrfs ===
tst_test.c:1117: TINFO: Formatting /dev/loop0 with btrfs opts='' extra
opts=''
tst_test.c:1131: TINFO: Mounting /dev/loop0 to /tmp/LTP_swaMqVXj2/mntpoint
fstyp=btrfs flags=0
libswap.c:34: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01
tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
libswap.c:34: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01
swapon01.c:27: TPASS: tst_syscall(__NR_swapon, SWAP_FILE, 0) passed
swapon01.c:30: TINFO: SwapCached: 0 Kb

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





>
> Also, tst_brk() in is_swap_supported() causes test to fail quickly.
> Given similar Martin's fix to shell API I merged today, which use tst_res +
> return instead of tst_brk [1] I suggest to use tst_res + change return
> function
> from void to int and handle another return in the test. That will allow to
> test
> other filesystems (which is useful to see, if particular filesystem code is
> broken or or VFS or some common code reused by all filesystems). This is
> the
> reason why I don't think "quit as early as possible" is a good approach
> when
> testing on all filesystems.
>

Yes, good advice.


> Kind regards,
> Petr
>
> [1]
> https://github.com/linux-test-project/ltp/commit/5c73ad84f3e6db9a965df3ed4a846352136ed990
>
> > > > https://github.com/wangli5665/ltp/tree/libswap
>
> > > > And, I prefer to wait for Cryil's feedback before posting them in
> ML:)
>
> > > +1
>
> > > Kind regards,
> > > Petr
>
>

-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Li Wang]

On Tue, Jan 23, 2024 at 1:55 PM Li Wang <liwang@redhat.com> wrote:

> Hi Petr,
>
> On Tue, Jan 23, 2024 at 4:23 AM Petr Vorel <pvorel@suse.cz> wrote:
>
>> Hi Li,
>>
>> > Great, and FYI.
>>
>> > I just pushed a new commit to resolve the FIBMAP (unsupported on BTRFS)
>> > problem on my ltp:libswap branch.
>>
>> >
>> https://github.com/wangli5665/ltp/commit/699711bfe8c8dbc3597c46587345fa1197c054c9
>>
>> > Hope you can try on your side and give some feedback :).
>>
>> Unfortunately regardless of kernel version it fails:
>>
>> # LTP_SINGLE_FS_TYPE=btrfs ./swapon01
>> ...
>> tst_test.c:1669: TINFO: === Testing on btrfs ===
>> tst_test.c:1117: TINFO: Formatting /dev/loop0 with btrfs opts='' extra
>> opts=''
>> tst_test.c:1131: TINFO: Mounting /dev/loop0 to
>> /tmp/LTP_swa497AKp/mntpoint fstyp=btrfs flags=0
>> tst_ioctl.c:21: TINFO: FIBMAP ioctl is NOT supported: EINVAL (22)
>> libswap.c:93: TINFO: File 'mntpoint/swapfile01' is not contiguous
>> libswap.c:34: TINFO: FS_NOCOW_FL attribute set on mntpoint/swapfile01
>> libswap.c:169: TFAIL: swapon() on btrfs failed: EINVAL (22)
>>
>
>
> Thanks for the quick feedback, very useful.
>
> After looking over the code, I think it's because the tst_fill_file didn't
> create a contiguous file at the beginning, so the solution is probably
> to define a new function just used by creating a contiguous file for btrfs
> in the libswap.c.
>
> After a tiny amend, it works on a fedora38.
>

V3 is here: https://github.com/wangli5665/ltp/commits/libswap-v3

It get passed on my fedora38(BTRFS), rhel9(XFS) platforms, I would
post out in ML, just n case people can review the latest version.

(it made quite a lot of changes vs the v1, so I don't want people to see
the
buggy version to fall into confusion :)


-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Petr Vorel]

Hi Li,

> V3 is here: https://github.com/wangli5665/ltp/commits/libswap-v3

I've just scheduled new runs for both Tumbleweed and all SLES versions
(various kernel versions).

> It get passed on my fedora38(BTRFS), rhel9(XFS) platforms, I would
> post out in ML, just n case people can review the latest version.

> (it made quite a lot of changes vs the v1, so I don't want people to see
> the
> buggy version to fall into confusion :)

+1

Thanks!

Kind regards,
Petr

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

Re: [LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Li Wang]

On Tue, Jan 23, 2024 at 3:48 PM Petr Vorel <pvorel@suse.cz> wrote:

> Hi Li,
>
> > V3 is here: https://github.com/wangli5665/ltp/commits/libswap-v3
>
> I've just scheduled new runs for both Tumbleweed and all SLES versions
> (various kernel versions).
>

Thanks Petr, and sorry for posting to ML so later.
I did some polishing work and fixed a few tiny issues based on above V3.

I'm not sure if the libswap-v3 branch will test well (with tiny issues).
So I pushed the latest to my main branch:
 https://github.com/wangli5665/ltp
And, I deployed it with a general test (against RHEL) for the coming
release work.
Will share my test result with you and Cryil later.

-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Petr Vorel]

> On Tue, Jan 23, 2024 at 3:48 PM Petr Vorel <pvorel@suse.cz> wrote:

> > Hi Li,

> > > V3 is here: https://github.com/wangli5665/ltp/commits/libswap-v3

> > I've just scheduled new runs for both Tumbleweed and all SLES versions
> > (various kernel versions).


> Thanks Petr, and sorry for posting to ML so later.
> I did some polishing work and fixed a few tiny issues based on above V3.

> I'm not sure if the libswap-v3 branch will test well (with tiny issues).
> So I pushed the latest to my main branch:
>  https://github.com/wangli5665/ltp
> And, I deployed it with a general test (against RHEL) for the coming
> release work.

Thanks Li!
As I wrote to 4th patch [1] there will be v4 needed, but fix should be easy.

Kind regards,
Petr

[1] https://lore.kernel.org/ltp/20240123121156.GA175806@pevik/

> Will share my test result with you and Cryil later.

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

Re: [LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Li Wang]

On Tue, Jan 23, 2024 at 8:31 PM Petr Vorel <pvorel@suse.cz> wrote:

> > On Tue, Jan 23, 2024 at 3:48 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> > > Hi Li,
>
> > > > V3 is here: https://github.com/wangli5665/ltp/commits/libswap-v3
>
> > > I've just scheduled new runs for both Tumbleweed and all SLES versions
> > > (various kernel versions).
>
>
> > Thanks Petr, and sorry for posting to ML so later.
> > I did some polishing work and fixed a few tiny issues based on above V3.
>
> > I'm not sure if the libswap-v3 branch will test well (with tiny issues).
> > So I pushed the latest to my main branch:
> >  https://github.com/wangli5665/ltp
> > And, I deployed it with a general test (against RHEL) for the coming
> > release work.
>
> Thanks Li!
> As I wrote to 4th patch [1] there will be v4 needed, but fix should be
> easy.
>

The fix is needed, but it does not belong to my libswap patch set.
Without applying the five patches, swapoff still fails, right?
(it originally caused by the tst_fill_file() but not other thing)

It probably needs to be fixed in a separate patch in following up.


-- 
Regards,
Li Wang

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

Re: [LTP] [PATCH 1/4] libswap: add known swap supported fs check

[Petr Vorel]

> On Tue, Jan 23, 2024 at 8:31 PM Petr Vorel <pvorel@suse.cz> wrote:

> > > On Tue, Jan 23, 2024 at 3:48 PM Petr Vorel <pvorel@suse.cz> wrote:

> > > > Hi Li,

> > > > > V3 is here: https://github.com/wangli5665/ltp/commits/libswap-v3

> > > > I've just scheduled new runs for both Tumbleweed and all SLES versions
> > > > (various kernel versions).


> > > Thanks Petr, and sorry for posting to ML so later.
> > > I did some polishing work and fixed a few tiny issues based on above V3.

> > > I'm not sure if the libswap-v3 branch will test well (with tiny issues).
> > > So I pushed the latest to my main branch:
> > >  https://github.com/wangli5665/ltp
> > > And, I deployed it with a general test (against RHEL) for the coming
> > > release work.

> > Thanks Li!
> > As I wrote to 4th patch [1] there will be v4 needed, but fix should be
> > easy.


> The fix is needed, but it does not belong to my libswap patch set.
> Without applying the five patches, swapoff still fails, right?
No, that was introduced by 4th patch which added support for btrfs
(before it TCONF - swap not implemented).

Kind regards,
Petr

> (it originally caused by the tst_fill_file() but not other thing)

> It probably needs to be fixed in a separate patch in following up.

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