[LTP] [PATCH] fanotify: Fix broken tests due to fanotify_events_supported_by_kernel()

[LTP] [PATCH] fanotify: Fix broken tests due to fanotify_events_supported_by_kernel()

[Jan Kara]

When LTP test is run with CWD in btrfs subvolume, tests like fanotify16
fail with:

fanotify.h:169: TBROK: fanotify_mark (3, FAN_MARK_ADD, ..., AT_FDCWD, ".") failed: EXDEV (18)

This is because fanotify_events_supported_by_kernel() try to place a
mark onto CWD and that is forbidden for btrfs subvolumes. Change
fanotify_events_supported_by_kernel() to use "/" instead of "." which
has higher chances of working for btrfs.

Also update the error message to provide a bit more info.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 testcases/kernel/syscalls/fanotify/fanotify.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
index 32b510cdc178..f2fe0c05b449 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify.h
+++ b/testcases/kernel/syscalls/fanotify/fanotify.h
@@ -161,12 +161,13 @@ static inline int fanotify_events_supported_by_kernel(uint64_t mask,
 
 	fd = SAFE_FANOTIFY_INIT(init_flags, O_RDONLY);
 
-	if (fanotify_mark(fd, FAN_MARK_ADD | mark_flags, mask, AT_FDCWD, ".") < 0) {
+	if (fanotify_mark(fd, FAN_MARK_ADD | mark_flags, mask, AT_FDCWD, "/") < 0) {
 		if (errno == EINVAL) {
 			rval = -1;
 		} else {
 			tst_brk(TBROK | TERRNO,
-				"fanotify_mark (%d, FAN_MARK_ADD, ..., AT_FDCWD, \".\") failed", fd);
+				"fanotify_mark (%d, FAN_MARK_ADD | %x, %llx, AT_FDCWD, \".\") failed",
+				fd, mark_flags, (unsigned long long)mask);
 		}
 	}
 
-- 
2.35.3


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

Re: [LTP] [PATCH] fanotify: Fix broken tests due to fanotify_events_supported_by_kernel()

[Petr Vorel]

Hi Jan,

> When LTP test is run with CWD in btrfs subvolume, tests like fanotify16
> fail with:

> fanotify.h:169: TBROK: fanotify_mark (3, FAN_MARK_ADD, ..., AT_FDCWD, ".") failed: EXDEV (18)

I'll have a deeper look tomorrow, but actually with this patch it fails

LTP_SINGLE_FS_TYPE=btrfs ./fanotify16
...
fanotify.h:168: TBROK: fanotify_mark (3, FAN_MARK_ADD | 0, 10000000, AT_FDCWD, ".") failed: EXDEV (18)

And it works without it. What am I missing?

Tested on 6.7.0-rc1-2.g86e46c2-default

> This is because fanotify_events_supported_by_kernel() try to place a
> mark onto CWD and that is forbidden for btrfs subvolumes. Change
> fanotify_events_supported_by_kernel() to use "/" instead of "." which
> has higher chances of working for btrfs.

> Also update the error message to provide a bit more info.

> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  testcases/kernel/syscalls/fanotify/fanotify.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index 32b510cdc178..f2fe0c05b449 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -161,12 +161,13 @@ static inline int fanotify_events_supported_by_kernel(uint64_t mask,

>  	fd = SAFE_FANOTIFY_INIT(init_flags, O_RDONLY);

> -	if (fanotify_mark(fd, FAN_MARK_ADD | mark_flags, mask, AT_FDCWD, ".") < 0) {
> +	if (fanotify_mark(fd, FAN_MARK_ADD | mark_flags, mask, AT_FDCWD, "/") < 0) {
>  		if (errno == EINVAL) {
>  			rval = -1;
>  		} else {
>  			tst_brk(TBROK | TERRNO,
> -				"fanotify_mark (%d, FAN_MARK_ADD, ..., AT_FDCWD, \".\") failed", fd);
> +				"fanotify_mark (%d, FAN_MARK_ADD | %x, %llx, AT_FDCWD, \".\") failed",
				"fanotify_mark (%d, FAN_MARK_ADD | %x, %llx, AT_FDCWD, \"/\") failed",

(You changed directory . to /, thus error message should be updated as well)

Kind regards,
Petr

> +				fd, mark_flags, (unsigned long long)mask);
>  		}
>  	}

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

Re: [LTP] [PATCH] fanotify: Fix broken tests due to fanotify_events_supported_by_kernel()

[Amir Goldstein]

On Mon, Nov 27, 2023 at 5:40 PM Jan Kara <jack@suse.cz> wrote:
>
> When LTP test is run with CWD in btrfs subvolume, tests like fanotify16
> fail with:
>
> fanotify.h:169: TBROK: fanotify_mark (3, FAN_MARK_ADD, ..., AT_FDCWD, ".") failed: EXDEV (18)
>
> This is because fanotify_events_supported_by_kernel() try to place a
> mark onto CWD and that is forbidden for btrfs subvolumes. Change
> fanotify_events_supported_by_kernel() to use "/" instead of "." which
> has higher chances of working for btrfs.
>

FWIW, "." in setup() is usually a tempdir (under LTP temp root)
So I'm not sure that "/" is a better choice than ".".
At least the LTP temp dir is configurable.
and no reason that "/" is not a btrfs subvol inside a container...

FYI, in this branch:
https://github.com/amir73il/ltp/commits/fanotify_fsid

I have already implemented fanotify_flags_supported_on_fs()
which can be used to test support for an event/mark on a specific path.

I did not make the change in fanotify16.c to use
fanotify_flags_supported_on_fs() instead of
fanotify_{mark,events}_supported_by_kernel() but it would be trivial to do.

Thanks,
Amir.



> Also update the error message to provide a bit more info.
>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  testcases/kernel/syscalls/fanotify/fanotify.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h
> index 32b510cdc178..f2fe0c05b449 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify.h
> +++ b/testcases/kernel/syscalls/fanotify/fanotify.h
> @@ -161,12 +161,13 @@ static inline int fanotify_events_supported_by_kernel(uint64_t mask,
>
>         fd = SAFE_FANOTIFY_INIT(init_flags, O_RDONLY);
>
> -       if (fanotify_mark(fd, FAN_MARK_ADD | mark_flags, mask, AT_FDCWD, ".") < 0) {
> +       if (fanotify_mark(fd, FAN_MARK_ADD | mark_flags, mask, AT_FDCWD, "/") < 0) {
>                 if (errno == EINVAL) {
>                         rval = -1;
>                 } else {
>                         tst_brk(TBROK | TERRNO,
> -                               "fanotify_mark (%d, FAN_MARK_ADD, ..., AT_FDCWD, \".\") failed", fd);
> +                               "fanotify_mark (%d, FAN_MARK_ADD | %x, %llx, AT_FDCWD, \".\") failed",
> +                               fd, mark_flags, (unsigned long long)mask);
>                 }
>         }
>
> --
> 2.35.3
>

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

Re: [LTP] [PATCH] fanotify: Fix broken tests due to fanotify_events_supported_by_kernel()

[Jan Kara]

On Mon 27-11-23 19:40:14, Amir Goldstein wrote:
> On Mon, Nov 27, 2023 at 5:40 PM Jan Kara <jack@suse.cz> wrote:
> >
> > When LTP test is run with CWD in btrfs subvolume, tests like fanotify16
> > fail with:
> >
> > fanotify.h:169: TBROK: fanotify_mark (3, FAN_MARK_ADD, ..., AT_FDCWD, ".") failed: EXDEV (18)
> >
> > This is because fanotify_events_supported_by_kernel() try to place a
> > mark onto CWD and that is forbidden for btrfs subvolumes. Change
> > fanotify_events_supported_by_kernel() to use "/" instead of "." which
> > has higher chances of working for btrfs.
> >
> 
> FWIW, "." in setup() is usually a tempdir (under LTP temp root)
> So I'm not sure that "/" is a better choice than ".".
> At least the LTP temp dir is configurable.
> and no reason that "/" is not a btrfs subvol inside a container...

Yeah, that's a good point. I was thinking it need not be so simple but
wanted something to start a discussion :)

> FYI, in this branch:
> https://github.com/amir73il/ltp/commits/fanotify_fsid
> 
> I have already implemented fanotify_flags_supported_on_fs()
> which can be used to test support for an event/mark on a specific path.
> 
> I did not make the change in fanotify16.c to use
> fanotify_flags_supported_on_fs() instead of
> fanotify_{mark,events}_supported_by_kernel() but it would be trivial to do.

OK, this is probably a more robust idea so that we test all the features
against a path we are then actually going to use for testing. I'll pick the
commit "fanotify: Generalize helper fanotify_flags_supported_on_fs()" from
your branch and rework the fix based on that tomorrow. Thanks!

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

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

Re: [LTP] [PATCH] fanotify: Fix broken tests due to fanotify_events_supported_by_kernel()

[Petr Vorel]

Hi Jan, Amir,

> On Mon 27-11-23 19:40:14, Amir Goldstein wrote:
> > On Mon, Nov 27, 2023 at 5:40 PM Jan Kara <jack@suse.cz> wrote:

> > > When LTP test is run with CWD in btrfs subvolume, tests like fanotify16
> > > fail with:

> > > fanotify.h:169: TBROK: fanotify_mark (3, FAN_MARK_ADD, ..., AT_FDCWD, ".") failed: EXDEV (18)

> > > This is because fanotify_events_supported_by_kernel() try to place a
> > > mark onto CWD and that is forbidden for btrfs subvolumes. Change
> > > fanotify_events_supported_by_kernel() to use "/" instead of "." which
> > > has higher chances of working for btrfs.


> > FWIW, "." in setup() is usually a tempdir (under LTP temp root)
> > So I'm not sure that "/" is a better choice than ".".
> > At least the LTP temp dir is configurable.
> > and no reason that "/" is not a btrfs subvol inside a container...

> Yeah, that's a good point. I was thinking it need not be so simple but
> wanted something to start a discussion :)

The variable is TMPDIR (all variables are printed with -h, e.g. ./fanotify01 -h).
I was expecting this would be a problem, but instead the test was broken on
Tumbleweed with the default TMPDIR value /tmp.

> > FYI, in this branch:
> > https://github.com/amir73il/ltp/commits/fanotify_fsid

FYI the first commit breaks at least vfat on fanotify13
# LTP_SINGLE_FS_TYPE=vfat ./fanotify13
...
tst_test.c:1650: TINFO: === Testing on vfat ===
tst_test.c:1105: TINFO: Formatting /dev/loop1 with vfat opts='' extra opts=''
tst_test.c:1119: TINFO: Mounting /dev/loop1 to /tmp/LTP_fanM9wLom/mntpoint fstyp=vfat flags=0
fanotify13.c:152: TINFO: Test #0.1: FAN_REPORT_FID with mark flag: FAN_MARK_INODE
fanotify13.c:157: TCONF: overlayfs not supported on vfat
fanotify13.c:152: TINFO: Test #1.1: FAN_REPORT_FID with mark flag: FAN_MARK_INODE
fanotify13.c:157: TCONF: overlayfs not supported on vfat
fanotify13.c:152: TINFO: Test #2.1: FAN_REPORT_FID with mark flag: FAN_MARK_MOUNT
fanotify13.c:157: TCONF: overlayfs not supported on vfat
fanotify13.c:152: TINFO: Test #3.1: FAN_REPORT_FID with mark flag: FAN_MARK_MOUNT
fanotify13.c:157: TCONF: overlayfs not supported on vfat
fanotify13.c:152: TINFO: Test #4.1: FAN_REPORT_FID with mark flag: FAN_MARK_FILESYSTEM
fanotify13.c:157: TCONF: overlayfs not supported on vfat
fanotify13.c:152: TINFO: Test #5.1: FAN_REPORT_FID with mark flag: FAN_MARK_FILESYSTEM
fanotify13.c:157: TCONF: overlayfs not supported on vfat
tst_device.c:408: TINFO: umount('mntpoint') failed with EBUSY, try  1...
tst_device.c:412: TINFO: Likely gvfsd-trash is probing newly mounted fs, kill it to speed up tests.
tst_device.c:408: TINFO: umount('mntpoint') failed with EBUSY, try  2...
tst_device.c:408: TINFO: umount('mntpoint') failed with EBUSY, try  3...
...
tst_device.c:408: TINFO: umount('mntpoint') failed with EBUSY, try 50...
tst_device.c:419: TWARN: Failed to umount('mntpoint') after 50 retries
tst_test.c:1650: TINFO: === Testing on exfat ===
tst_test.c:1105: TINFO: Formatting /dev/loop1 with exfat opts='' extra opts=''
tst_test.c:1119: TINFO: Mounting /dev/loop1 to /tmp/LTP_fanM9wLom/mntpoint fstyp=exfat flags=0
tst_test.c:1119: TINFO: Trying FUSE...
FUSE exfat 1.4.0 (libfuse2)
fuse: mount failed: Device or resource busy
tst_test.c:1119: TBROK: mount.exfat failed with 256

HINT: You _MAY_ be missing kernel fixes:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c285a2f01d69
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bc2473c90fca


> > I have already implemented fanotify_flags_supported_on_fs()
> > which can be used to test support for an event/mark on a specific path.

> > I did not make the change in fanotify16.c to use
> > fanotify_flags_supported_on_fs() instead of
> > fanotify_{mark,events}_supported_by_kernel() but it would be trivial to do.

> OK, this is probably a more robust idea so that we test all the features
> against a path we are then actually going to use for testing. I'll pick the
> commit "fanotify: Generalize helper fanotify_flags_supported_on_fs()" from
> your branch and rework the fix based on that tomorrow. Thanks!

Great!

Thanks a lot to you both!

Kind regards,
Petr

> 								Honza

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

Re: [LTP] [PATCH] fanotify: Fix broken tests due to fanotify_events_supported_by_kernel()

[Amir Goldstein]

On Tue, Nov 28, 2023 at 12:07 AM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Jan, Amir,
>
> > On Mon 27-11-23 19:40:14, Amir Goldstein wrote:
> > > On Mon, Nov 27, 2023 at 5:40 PM Jan Kara <jack@suse.cz> wrote:
>
> > > > When LTP test is run with CWD in btrfs subvolume, tests like fanotify16
> > > > fail with:
>
> > > > fanotify.h:169: TBROK: fanotify_mark (3, FAN_MARK_ADD, ..., AT_FDCWD, ".") failed: EXDEV (18)
>
> > > > This is because fanotify_events_supported_by_kernel() try to place a
> > > > mark onto CWD and that is forbidden for btrfs subvolumes. Change
> > > > fanotify_events_supported_by_kernel() to use "/" instead of "." which
> > > > has higher chances of working for btrfs.
>
>
> > > FWIW, "." in setup() is usually a tempdir (under LTP temp root)
> > > So I'm not sure that "/" is a better choice than ".".
> > > At least the LTP temp dir is configurable.
> > > and no reason that "/" is not a btrfs subvol inside a container...
>
> > Yeah, that's a good point. I was thinking it need not be so simple but
> > wanted something to start a discussion :)
>
> The variable is TMPDIR (all variables are printed with -h, e.g. ./fanotify01 -h).
> I was expecting this would be a problem, but instead the test was broken on
> Tumbleweed with the default TMPDIR value /tmp.
>
> > > FYI, in this branch:
> > > https://github.com/amir73il/ltp/commits/fanotify_fsid
>
> FYI the first commit breaks at least vfat on fanotify13
> # LTP_SINGLE_FS_TYPE=vfat ./fanotify13
> ...
> tst_test.c:1650: TINFO: === Testing on vfat ===
> tst_test.c:1105: TINFO: Formatting /dev/loop1 with vfat opts='' extra opts=''
> tst_test.c:1119: TINFO: Mounting /dev/loop1 to /tmp/LTP_fanM9wLom/mntpoint fstyp=vfat flags=0
> fanotify13.c:152: TINFO: Test #0.1: FAN_REPORT_FID with mark flag: FAN_MARK_INODE
> fanotify13.c:157: TCONF: overlayfs not supported on vfat
> fanotify13.c:152: TINFO: Test #1.1: FAN_REPORT_FID with mark flag: FAN_MARK_INODE
> fanotify13.c:157: TCONF: overlayfs not supported on vfat
> fanotify13.c:152: TINFO: Test #2.1: FAN_REPORT_FID with mark flag: FAN_MARK_MOUNT
> fanotify13.c:157: TCONF: overlayfs not supported on vfat
> fanotify13.c:152: TINFO: Test #3.1: FAN_REPORT_FID with mark flag: FAN_MARK_MOUNT
> fanotify13.c:157: TCONF: overlayfs not supported on vfat
> fanotify13.c:152: TINFO: Test #4.1: FAN_REPORT_FID with mark flag: FAN_MARK_FILESYSTEM
> fanotify13.c:157: TCONF: overlayfs not supported on vfat
> fanotify13.c:152: TINFO: Test #5.1: FAN_REPORT_FID with mark flag: FAN_MARK_FILESYSTEM
> fanotify13.c:157: TCONF: overlayfs not supported on vfat
> tst_device.c:408: TINFO: umount('mntpoint') failed with EBUSY, try  1...
> tst_device.c:412: TINFO: Likely gvfsd-trash is probing newly mounted fs, kill it to speed up tests.
> tst_device.c:408: TINFO: umount('mntpoint') failed with EBUSY, try  2...
> tst_device.c:408: TINFO: umount('mntpoint') failed with EBUSY, try  3...
> ...
> tst_device.c:408: TINFO: umount('mntpoint') failed with EBUSY, try 50...
> tst_device.c:419: TWARN: Failed to umount('mntpoint') after 50 retries
> tst_test.c:1650: TINFO: === Testing on exfat ===
> tst_test.c:1105: TINFO: Formatting /dev/loop1 with exfat opts='' extra opts=''
> tst_test.c:1119: TINFO: Mounting /dev/loop1 to /tmp/LTP_fanM9wLom/mntpoint fstyp=exfat flags=0
> tst_test.c:1119: TINFO: Trying FUSE...
> FUSE exfat 1.4.0 (libfuse2)
> fuse: mount failed: Device or resource busy
> tst_test.c:1119: TBROK: mount.exfat failed with 256
>

I believe this breakage is a regression due to overlayfs bug in v6.7-rc1
that was fixed by commit
37f32f526438 ("ovl: fix memory leak in ovl_parse_param()") in v6.7-rc2.

Thanks,
Amir.

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

Re: [LTP] [PATCH] fanotify: Fix broken tests due to fanotify_events_supported_by_kernel()

[Petr Vorel]

Hi Amir,

...
> > > > FYI, in this branch:
> > > > https://github.com/amir73il/ltp/commits/fanotify_fsid

> > FYI the first commit breaks at least vfat on fanotify13
> > # LTP_SINGLE_FS_TYPE=vfat ./fanotify13
> > ...
> > tst_test.c:1650: TINFO: === Testing on vfat ===
> > tst_test.c:1105: TINFO: Formatting /dev/loop1 with vfat opts='' extra opts=''
> > tst_test.c:1119: TINFO: Mounting /dev/loop1 to /tmp/LTP_fanM9wLom/mntpoint fstyp=vfat flags=0
> > fanotify13.c:152: TINFO: Test #0.1: FAN_REPORT_FID with mark flag: FAN_MARK_INODE
> > fanotify13.c:157: TCONF: overlayfs not supported on vfat
> > fanotify13.c:152: TINFO: Test #1.1: FAN_REPORT_FID with mark flag: FAN_MARK_INODE
> > fanotify13.c:157: TCONF: overlayfs not supported on vfat
> > fanotify13.c:152: TINFO: Test #2.1: FAN_REPORT_FID with mark flag: FAN_MARK_MOUNT
> > fanotify13.c:157: TCONF: overlayfs not supported on vfat
> > fanotify13.c:152: TINFO: Test #3.1: FAN_REPORT_FID with mark flag: FAN_MARK_MOUNT
> > fanotify13.c:157: TCONF: overlayfs not supported on vfat
> > fanotify13.c:152: TINFO: Test #4.1: FAN_REPORT_FID with mark flag: FAN_MARK_FILESYSTEM
> > fanotify13.c:157: TCONF: overlayfs not supported on vfat
> > fanotify13.c:152: TINFO: Test #5.1: FAN_REPORT_FID with mark flag: FAN_MARK_FILESYSTEM
> > fanotify13.c:157: TCONF: overlayfs not supported on vfat
> > tst_device.c:408: TINFO: umount('mntpoint') failed with EBUSY, try  1...
> > tst_device.c:412: TINFO: Likely gvfsd-trash is probing newly mounted fs, kill it to speed up tests.
> > tst_device.c:408: TINFO: umount('mntpoint') failed with EBUSY, try  2...
> > tst_device.c:408: TINFO: umount('mntpoint') failed with EBUSY, try  3...
> > ...
> > tst_device.c:408: TINFO: umount('mntpoint') failed with EBUSY, try 50...
> > tst_device.c:419: TWARN: Failed to umount('mntpoint') after 50 retries
> > tst_test.c:1650: TINFO: === Testing on exfat ===
> > tst_test.c:1105: TINFO: Formatting /dev/loop1 with exfat opts='' extra opts=''
> > tst_test.c:1119: TINFO: Mounting /dev/loop1 to /tmp/LTP_fanM9wLom/mntpoint fstyp=exfat flags=0
> > tst_test.c:1119: TINFO: Trying FUSE...
> > FUSE exfat 1.4.0 (libfuse2)
> > fuse: mount failed: Device or resource busy
> > tst_test.c:1119: TBROK: mount.exfat failed with 256


> I believe this breakage is a regression due to overlayfs bug in v6.7-rc1
> that was fixed by commit
> 37f32f526438 ("ovl: fix memory leak in ovl_parse_param()") in v6.7-rc2.

Indeed, it works on openSUSE 6.7.0-rc3-3.ge7296f9-default.

Thanks for info.

Kind regards,
Petr

> Thanks,
> Amir.

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