All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH v2 1/2] fanotify21: fix test failure when running iterations
@ 2026-05-28 16:29 Amir Goldstein
  2026-05-28 16:29 ` [LTP] [PATCH v2 2/2] fanotify13: " Amir Goldstein
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Amir Goldstein @ 2026-05-28 16:29 UTC (permalink / raw)
  To: Petr Vorel; +Cc: Jan Kara, ltp, AnonymeMeow

Peter reported that fanotify21 -i2 fails.
Fix this by always remounting ro/rw before every test.
Use a bind mount, where remount works regardless of base fs.

Reported-by: Petr Vorel <pvorel@suse.cz>
Link: https://lore.kernel.org/linux-fsdevel/20260527072312.GA231966@pevik/
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

changes since v1:
- Address LTP AI Reviewer comments

 .../kernel/syscalls/fanotify/fanotify21.c      | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify21.c b/testcases/kernel/syscalls/fanotify/fanotify21.c
index 340fb0018..4d2115469 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify21.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify21.c
@@ -123,6 +123,9 @@ static void do_setup(void)
 	int pidfd;
 	int init_flags = FAN_REPORT_PIDFD;
 
+	/* Bind mount so remount ro/rw always work */
+	SAFE_MOUNT(MOUNT_PATH, MOUNT_PATH, "none", MS_BIND, NULL);
+
 	if (tst_variant) {
 		fanotify_fd = -1;
 		fd_error_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_FD_ERROR, ".");
@@ -171,15 +174,9 @@ static void do_test(unsigned int num)
 		return;
 	}
 
-	if (tc->remount_ro) {
-		/* SAFE_MOUNT fails to remount FUSE */
-		if (mount(tst_device->dev, MOUNT_PATH, tst_device->fs_type,
-			  MS_REMOUNT|MS_RDONLY, NULL) != 0) {
-			tst_brk(TFAIL,
-				"filesystem %s failed to remount readonly",
-				tst_device->fs_type);
-		}
-	}
+	/* remount ro/rw the bind mount */
+	SAFE_MOUNT("none", MOUNT_PATH, "none", MS_BIND | MS_REMOUNT |
+		   (tc->remount_ro ? MS_RDONLY : 0), NULL);
 
 	/*
 	 * Generate the event in either self or a child process. Event
@@ -355,6 +352,9 @@ static void do_cleanup(void)
 
 	if (self_pidfd_fdinfo)
 		free(self_pidfd_fdinfo);
+
+	/* Unmount the bind mount */
+	SAFE_UMOUNT(MOUNT_PATH);
 }
 
 static struct tst_test test = {
-- 
2.54.0


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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [LTP] [PATCH v2 2/2] fanotify13: fix test failure when running iterations
  2026-05-28 16:29 [LTP] [PATCH v2 1/2] fanotify21: fix test failure when running iterations Amir Goldstein
@ 2026-05-28 16:29 ` Amir Goldstein
  2026-05-29 16:30   ` Petr Vorel
                     ` (2 more replies)
  2026-05-28 18:43 ` [LTP] fanotify21: " linuxtestproject.agent
  2026-05-29 16:25 ` [LTP] [PATCH v2 1/2] " Petr Vorel
  2 siblings, 3 replies; 15+ messages in thread
From: Amir Goldstein @ 2026-05-28 16:29 UTC (permalink / raw)
  To: Petr Vorel; +Cc: Jan Kara, ltp, AnonymeMeow

The test case for FAN_DELETE_SELF deletes the objects created in
do_setup() so we need to re-create them for the next iteration.

This bring up a problem with ext2 and filesystem that do not support
RENAME_EXCHANGE because overlayfs fails to re-create objects over a
whiteout.

It is generally not interesting to test overlayfs over such base fs,
but for now, let just exclude this type of base fs from the delete
self event test case.

Reported-by: AnonymeMeow <anonymemeow@gmail.com>
Link: https://lore.kernel.org/linux-fsdevel/20260527195056.337081-1-anonymemeow@gmail.com/
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---

Changes since v1:
- Address LTP AI Reviewer comments

 .../kernel/syscalls/fanotify/fanotify13.c     | 38 +++++++++++++++----
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
index 76d40eaf7..a00240a33 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify13.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
@@ -110,6 +110,7 @@ static int nofid_fd;
 static int fanotify_fd;
 static int at_handle_fid;
 static int filesystem_mark_unsupported;
+static int rename_exchange_unsupported;
 static char events_buf[BUF_SIZE];
 static struct event_t event_set[EVENT_MAX];
 
@@ -191,6 +192,13 @@ static void do_test(unsigned int number)
 		return;
 	}
 
+	if (tst_variant && (tc->mask & FAN_DELETE_SELF) &&
+	    (!ovl_bind_mounted || rename_exchange_unsupported)) {
+		/* The eviction of base fs inodes is defered due to overlay held reference */
+		tst_res(TCONF, "overlayfs base fs cannot be watched for delete self events");
+		return;
+	}
+
 	if (filesystem_mark_unsupported && mark->flag != FAN_MARK_INODE) {
 		FANOTIFY_MARK_FLAGS_ERR_MSG(mark, filesystem_mark_unsupported);
 		return;
@@ -212,11 +220,6 @@ static void do_test(unsigned int number)
 			tst_res(TCONF, "overlayfs base fs cannot be watched with mount mark");
 			goto out;
 		}
-		if (tc->mask & FAN_DELETE_SELF) {
-			/* The eviction of base fs inodes is defered due to overlay held reference */
-			tst_res(TCONF, "overlayfs base fs cannot be watched for delete self events");
-			goto out;
-		}
 		SAFE_MOUNT(OVL_MNT, MOUNT_PATH, "none", MS_BIND, NULL);
 	}
 
@@ -340,10 +343,18 @@ static void do_test(unsigned int number)
 			"Did not get an expected event (expected: %llx)",
 			event_set[i].expected_mask);
 	}
+
+	if (tc->mask & FAN_DELETE_SELF) {
+		create_objects();
+		get_object_stats();
+	}
 out:
 	SAFE_CLOSE(fanotify_fd);
 }
 
+#define TST_VARIANT_OVL_LOWER (tst_variant & 1)
+#define TST_VARIANT_OVL_WATCH (tst_variant > 2)
+
 static void do_setup(void)
 {
 	const char *mnt;
@@ -371,10 +382,9 @@ static void do_setup(void)
 		if (!ovl_mounted)
 			return;
 
-		mnt = tst_variant & 1 ? OVL_LOWER : OVL_UPPER;
+		mnt = TST_VARIANT_OVL_LOWER ? OVL_LOWER : OVL_UPPER;
 	} else {
 		mnt = OVL_BASE_MNTPOINT;
-
 	}
 	REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, mnt);
 	SAFE_MKDIR(MOUNT_PATH, 0755);
@@ -386,7 +396,19 @@ static void do_setup(void)
 	/* Create file and directory objects for testing on base fs */
 	create_objects();
 
-	if (tst_variant > 2) {
+	/* RENAME_EXCHANGE is required for create over whiteout in overlayfs */
+	if (TST_VARIANT_OVL_LOWER) {
+		rename_exchange_unsupported = renameat2(AT_FDCWD, FILE_PATH_ONE,
+							AT_FDCWD, FILE_PATH_TWO,
+							RENAME_EXCHANGE) == -1 &&
+					(errno == EOPNOTSUPP || errno == EINVAL);
+		if (rename_exchange_unsupported) {
+			tst_res(TCONF, "RENAME_EXCHANGE not supported on %s",
+				tst_device->fs_type);
+		}
+	}
+
+	if (TST_VARIANT_OVL_WATCH) {
 		/* Setup watches on overlayfs */
 		SAFE_MOUNT(OVL_MNT, MOUNT_PATH, "none", MS_BIND, NULL);
 		ovl_bind_mounted = 1;
-- 
2.54.0


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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [LTP] fanotify21: fix test failure when running iterations
  2026-05-28 16:29 [LTP] [PATCH v2 1/2] fanotify21: fix test failure when running iterations Amir Goldstein
  2026-05-28 16:29 ` [LTP] [PATCH v2 2/2] fanotify13: " Amir Goldstein
@ 2026-05-28 18:43 ` linuxtestproject.agent
  2026-05-29 16:25 ` [LTP] [PATCH v2 1/2] " Petr Vorel
  2 siblings, 0 replies; 15+ messages in thread
From: linuxtestproject.agent @ 2026-05-28 18:43 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: ltp

Hi Amir,

On Thu, 28 May 2026 18:29:19 +0200, Amir Goldstein wrote:
> fanotify21: fix test failure when running iterations

All good. Reviewed.

On Thu, 28 May 2026 18:29:20 +0200, Amir Goldstein wrote:
> fanotify13: fix test failure when running iterations

> +if (tst_variant && (tc->mask & FAN_DELETE_SELF) &&
> +    (!ovl_bind_mounted || rename_exchange_unsupported)) {
> +/* The eviction of base fs inodes is defered due to overlay held reference */

Typo: "defered" → "deferred". (Moved from old location.)

[...]

> +#define TST_VARIANT_OVL_LOWER (tst_variant & 1)
> +#define TST_VARIANT_OVL_WATCH (tst_variant > 2)

These macros are defined after do_test() but do_test() does not use them —
only do_setup() does. Placement is fine.

[...]

All good otherwise. Reviewed.

Pre-existing issues noticed in the surrounding code (not introduced
by this patch):

- fanotify21.c — fanotify_fd is a global static initialised to 0; if
  do_setup() exits early for tst_variant=0 before SAFE_FANOTIFY_INIT(),
  do_cleanup() would call SAFE_CLOSE(0), closing stdin. Same pattern
  exists in the pre-patch code.

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [LTP] [PATCH v2 1/2] fanotify21: fix test failure when running iterations
  2026-05-28 16:29 [LTP] [PATCH v2 1/2] fanotify21: fix test failure when running iterations Amir Goldstein
  2026-05-28 16:29 ` [LTP] [PATCH v2 2/2] fanotify13: " Amir Goldstein
  2026-05-28 18:43 ` [LTP] fanotify21: " linuxtestproject.agent
@ 2026-05-29 16:25 ` Petr Vorel
  2 siblings, 0 replies; 15+ messages in thread
From: Petr Vorel @ 2026-05-29 16:25 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Jan Kara, ltp, AnonymeMeow

Hi Amir,

this patch merged, thank you!

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [LTP] [PATCH v2 2/2] fanotify13: fix test failure when running iterations
  2026-05-28 16:29 ` [LTP] [PATCH v2 2/2] fanotify13: " Amir Goldstein
@ 2026-05-29 16:30   ` Petr Vorel
  2026-05-30 14:01     ` Amir Goldstein
  2026-05-29 16:34   ` Petr Vorel
  2026-05-31 23:41   ` [LTP] [PATCH] " AnonymeMeow
  2 siblings, 1 reply; 15+ messages in thread
From: Petr Vorel @ 2026-05-29 16:30 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Jan Kara, ltp, AnonymeMeow

Hi Amir,

> The test case for FAN_DELETE_SELF deletes the objects created in
> do_setup() so we need to re-create them for the next iteration.

> This bring up a problem with ext2 and filesystem that do not support
> RENAME_EXCHANGE because overlayfs fails to re-create objects over a
> whiteout.

> It is generally not interesting to test overlayfs over such base fs,
> but for now, let just exclude this type of base fs from the delete
> self event test case.

> Reported-by: AnonymeMeow <anonymemeow@gmail.com>
> Link: https://lore.kernel.org/linux-fsdevel/20260527195056.337081-1-anonymemeow@gmail.com/
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---

> Changes since v1:
> - Address LTP AI Reviewer comments

>  .../kernel/syscalls/fanotify/fanotify13.c     | 38 +++++++++++++++----
>  1 file changed, 30 insertions(+), 8 deletions(-)

> diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
> index 76d40eaf7..a00240a33 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify13.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
> @@ -110,6 +110,7 @@ static int nofid_fd;
>  static int fanotify_fd;
>  static int at_handle_fid;
>  static int filesystem_mark_unsupported;
> +static int rename_exchange_unsupported;
>  static char events_buf[BUF_SIZE];
>  static struct event_t event_set[EVENT_MAX];

> @@ -191,6 +192,13 @@ static void do_test(unsigned int number)
>  		return;
>  	}

> +	if (tst_variant && (tc->mask & FAN_DELETE_SELF) &&
> +	    (!ovl_bind_mounted || rename_exchange_unsupported)) {
> +		/* The eviction of base fs inodes is defered due to overlay held reference */
> +		tst_res(TCONF, "overlayfs base fs cannot be watched for delete self events");
> +		return;
> +	}
> +
>  	if (filesystem_mark_unsupported && mark->flag != FAN_MARK_INODE) {
>  		FANOTIFY_MARK_FLAGS_ERR_MSG(mark, filesystem_mark_unsupported);
>  		return;
> @@ -212,11 +220,6 @@ static void do_test(unsigned int number)
>  			tst_res(TCONF, "overlayfs base fs cannot be watched with mount mark");
>  			goto out;
>  		}
> -		if (tc->mask & FAN_DELETE_SELF) {
> -			/* The eviction of base fs inodes is defered due to overlay held reference */
> -			tst_res(TCONF, "overlayfs base fs cannot be watched for delete self events");
> -			goto out;
> -		}
>  		SAFE_MOUNT(OVL_MNT, MOUNT_PATH, "none", MS_BIND, NULL);
>  	}

> @@ -340,10 +343,18 @@ static void do_test(unsigned int number)
>  			"Did not get an expected event (expected: %llx)",
>  			event_set[i].expected_mask);
>  	}
> +
> +	if (tc->mask & FAN_DELETE_SELF) {
> +		create_objects();
> +		get_object_stats();
> +	}
>  out:
>  	SAFE_CLOSE(fanotify_fd);
>  }

> +#define TST_VARIANT_OVL_LOWER (tst_variant & 1)
> +#define TST_VARIANT_OVL_WATCH (tst_variant > 2)
> +
>  static void do_setup(void)
>  {
>  	const char *mnt;
> @@ -371,10 +382,9 @@ static void do_setup(void)
>  		if (!ovl_mounted)
>  			return;

> -		mnt = tst_variant & 1 ? OVL_LOWER : OVL_UPPER;
> +		mnt = TST_VARIANT_OVL_LOWER ? OVL_LOWER : OVL_UPPER;
>  	} else {
>  		mnt = OVL_BASE_MNTPOINT;
> -
>  	}
>  	REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, mnt);
>  	SAFE_MKDIR(MOUNT_PATH, 0755);
> @@ -386,7 +396,19 @@ static void do_setup(void)
>  	/* Create file and directory objects for testing on base fs */
>  	create_objects();

> -	if (tst_variant > 2) {
> +	/* RENAME_EXCHANGE is required for create over whiteout in overlayfs */
> +	if (TST_VARIANT_OVL_LOWER) {
> +		rename_exchange_unsupported = renameat2(AT_FDCWD, FILE_PATH_ONE,

LGTM, but renameat2() was added in Musl v1.2.6 (released just 2 months ago),
therefore it fails in our CI:

fanotify13.c:401:47: warning: implicit declaration of function 'renameat2'; did you mean 'renameat'? [-Wimplicit-function-declaration]
  401 |                 rename_exchange_unsupported = renameat2(AT_FDCWD, FILE_PATH_ONE,
      |                                               ^~~~~~~~~
      |                                               renameat

Could you please either use renameat() or use renameat2() as a raw syscall?

We even have it in testcases/kernel/syscalls/renameat2/renameat2.h, this
function should be moved to include/lapi/renameat2.h or include/lapi/stdio.h
(header which includes it). If I have time on Monday, I can do the cleanup and
fix it.

Kind regards,
Petr

> +							AT_FDCWD, FILE_PATH_TWO,
> +							RENAME_EXCHANGE) == -1 &&
> +					(errno == EOPNOTSUPP || errno == EINVAL);
> +		if (rename_exchange_unsupported) {
> +			tst_res(TCONF, "RENAME_EXCHANGE not supported on %s",
> +				tst_device->fs_type);
> +		}
> +	}
> +
> +	if (TST_VARIANT_OVL_WATCH) {
>  		/* Setup watches on overlayfs */
>  		SAFE_MOUNT(OVL_MNT, MOUNT_PATH, "none", MS_BIND, NULL);
>  		ovl_bind_mounted = 1;

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [LTP] [PATCH v2 2/2] fanotify13: fix test failure when running iterations
  2026-05-28 16:29 ` [LTP] [PATCH v2 2/2] fanotify13: " Amir Goldstein
  2026-05-29 16:30   ` Petr Vorel
@ 2026-05-29 16:34   ` Petr Vorel
  2026-05-31 23:41   ` [LTP] [PATCH] " AnonymeMeow
  2 siblings, 0 replies; 15+ messages in thread
From: Petr Vorel @ 2026-05-29 16:34 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Jan Kara, ltp, AnonymeMeow

Hi all,

it also fails on openSUSE Leap 42.2 due missing renameat2 (the same fix as for
musl described in previous email).

> +	if (tst_variant && (tc->mask & FAN_DELETE_SELF) &&
> +	    (!ovl_bind_mounted || rename_exchange_unsupported)) {
> +		/* The eviction of base fs inodes is defered due to overlay held reference */

While we are at it, we should fix old typo:
Typo: "defered" → "deferred".

Kind regards,
Petr

> +		tst_res(TCONF, "overlayfs base fs cannot be watched for delete self events");
> +		return;
> +	}
> +
>  	if (filesystem_mark_unsupported && mark->flag != FAN_MARK_INODE) {
>  		FANOTIFY_MARK_FLAGS_ERR_MSG(mark, filesystem_mark_unsupported);
>  		return;
> @@ -212,11 +220,6 @@ static void do_test(unsigned int number)
>  			tst_res(TCONF, "overlayfs base fs cannot be watched with mount mark");
>  			goto out;
>  		}
> -		if (tc->mask & FAN_DELETE_SELF) {
> -			/* The eviction of base fs inodes is defered due to overlay held reference */
> -			tst_res(TCONF, "overlayfs base fs cannot be watched for delete self events");
> -			goto out;
> -		}

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [LTP] [PATCH v2 2/2] fanotify13: fix test failure when running iterations
  2026-05-29 16:30   ` Petr Vorel
@ 2026-05-30 14:01     ` Amir Goldstein
  2026-06-03 13:33       ` Petr Vorel
  0 siblings, 1 reply; 15+ messages in thread
From: Amir Goldstein @ 2026-05-30 14:01 UTC (permalink / raw)
  To: Petr Vorel; +Cc: Jan Kara, ltp, AnonymeMeow

On Fri, May 29, 2026 at 6:31 PM Petr Vorel <pvorel@suse.cz> wrote:
>
> Hi Amir,
>
> > The test case for FAN_DELETE_SELF deletes the objects created in
> > do_setup() so we need to re-create them for the next iteration.
>
> > This bring up a problem with ext2 and filesystem that do not support
> > RENAME_EXCHANGE because overlayfs fails to re-create objects over a
> > whiteout.
>
> > It is generally not interesting to test overlayfs over such base fs,
> > but for now, let just exclude this type of base fs from the delete
> > self event test case.
>
> > Reported-by: AnonymeMeow <anonymemeow@gmail.com>
> > Link: https://lore.kernel.org/linux-fsdevel/20260527195056.337081-1-anonymemeow@gmail.com/
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
>
> > Changes since v1:
> > - Address LTP AI Reviewer comments
>
> >  .../kernel/syscalls/fanotify/fanotify13.c     | 38 +++++++++++++++----
> >  1 file changed, 30 insertions(+), 8 deletions(-)
>
> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
> > index 76d40eaf7..a00240a33 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify13.c
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
> > @@ -110,6 +110,7 @@ static int nofid_fd;
> >  static int fanotify_fd;
> >  static int at_handle_fid;
> >  static int filesystem_mark_unsupported;
> > +static int rename_exchange_unsupported;
> >  static char events_buf[BUF_SIZE];
> >  static struct event_t event_set[EVENT_MAX];
>
> > @@ -191,6 +192,13 @@ static void do_test(unsigned int number)
> >               return;
> >       }
>
> > +     if (tst_variant && (tc->mask & FAN_DELETE_SELF) &&
> > +         (!ovl_bind_mounted || rename_exchange_unsupported)) {
> > +             /* The eviction of base fs inodes is defered due to overlay held reference */
> > +             tst_res(TCONF, "overlayfs base fs cannot be watched for delete self events");
> > +             return;
> > +     }
> > +
> >       if (filesystem_mark_unsupported && mark->flag != FAN_MARK_INODE) {
> >               FANOTIFY_MARK_FLAGS_ERR_MSG(mark, filesystem_mark_unsupported);
> >               return;
> > @@ -212,11 +220,6 @@ static void do_test(unsigned int number)
> >                       tst_res(TCONF, "overlayfs base fs cannot be watched with mount mark");
> >                       goto out;
> >               }
> > -             if (tc->mask & FAN_DELETE_SELF) {
> > -                     /* The eviction of base fs inodes is defered due to overlay held reference */
> > -                     tst_res(TCONF, "overlayfs base fs cannot be watched for delete self events");
> > -                     goto out;
> > -             }
> >               SAFE_MOUNT(OVL_MNT, MOUNT_PATH, "none", MS_BIND, NULL);
> >       }
>
> > @@ -340,10 +343,18 @@ static void do_test(unsigned int number)
> >                       "Did not get an expected event (expected: %llx)",
> >                       event_set[i].expected_mask);
> >       }
> > +
> > +     if (tc->mask & FAN_DELETE_SELF) {
> > +             create_objects();
> > +             get_object_stats();
> > +     }
> >  out:
> >       SAFE_CLOSE(fanotify_fd);
> >  }
>
> > +#define TST_VARIANT_OVL_LOWER (tst_variant & 1)
> > +#define TST_VARIANT_OVL_WATCH (tst_variant > 2)
> > +
> >  static void do_setup(void)
> >  {
> >       const char *mnt;
> > @@ -371,10 +382,9 @@ static void do_setup(void)
> >               if (!ovl_mounted)
> >                       return;
>
> > -             mnt = tst_variant & 1 ? OVL_LOWER : OVL_UPPER;
> > +             mnt = TST_VARIANT_OVL_LOWER ? OVL_LOWER : OVL_UPPER;
> >       } else {
> >               mnt = OVL_BASE_MNTPOINT;
> > -
> >       }
> >       REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, mnt);
> >       SAFE_MKDIR(MOUNT_PATH, 0755);
> > @@ -386,7 +396,19 @@ static void do_setup(void)
> >       /* Create file and directory objects for testing on base fs */
> >       create_objects();
>
> > -     if (tst_variant > 2) {
> > +     /* RENAME_EXCHANGE is required for create over whiteout in overlayfs */
> > +     if (TST_VARIANT_OVL_LOWER) {
> > +             rename_exchange_unsupported = renameat2(AT_FDCWD, FILE_PATH_ONE,
>
> LGTM, but renameat2() was added in Musl v1.2.6 (released just 2 months ago),
> therefore it fails in our CI:
>
> fanotify13.c:401:47: warning: implicit declaration of function 'renameat2'; did you mean 'renameat'? [-Wimplicit-function-declaration]
>   401 |                 rename_exchange_unsupported = renameat2(AT_FDCWD, FILE_PATH_ONE,
>       |                                               ^~~~~~~~~
>       |                                               renameat
>
> Could you please either use renameat() or use renameat2() as a raw syscall?

renameat() has no flags argument.

>
> We even have it in testcases/kernel/syscalls/renameat2/renameat2.h, this
> function should be moved to include/lapi/renameat2.h or include/lapi/stdio.h
> (header which includes it). If I have time on Monday, I can do the cleanup and
> fix it.
>

Sounds good. Please change the test to use the common header.

Thanks,
Amir.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [LTP] [PATCH] fanotify13: fix test failure when running iterations
  2026-05-28 16:29 ` [LTP] [PATCH v2 2/2] fanotify13: " Amir Goldstein
  2026-05-29 16:30   ` Petr Vorel
  2026-05-29 16:34   ` Petr Vorel
@ 2026-05-31 23:41   ` AnonymeMeow
  2026-06-01  4:11     ` [LTP] " linuxtestproject.agent
  2026-06-01  8:29     ` [LTP] [PATCH] " Amir Goldstein
  2 siblings, 2 replies; 15+ messages in thread
From: AnonymeMeow @ 2026-05-31 23:41 UTC (permalink / raw)
  To: amir73il; +Cc: jack, ltp, AnonymeMeow

The FAN_DELETE_SELF test case removes test files, causing later
iterations to crash. Recreate the removed files after the test to
restore the expected initial state. And adjust the overlayfs mount
timing to avoid creating files directly in the filesystem underneath
the already mounted overlayfs.

Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
---

Hi, Amir,

I think we shouldn't unconditionally recreate the files through the
overlayfs. Because files created through overlayfs end up in the upper
dir, but test variant 3 is intended to verify events reported for lower
dir files opened through overlayfs, then starting from the second
iteration variant 3 would test events for upper dir files instead, which
does not match the intended test scenario.

Therefore, I updated my previous patch to unmount the overlayfs mount
and its bind mount before cleaning the upper dir, and mount them back
afterwards. This avoids modifying the underlying fs while the overlayfs
is still mounted.

And since modifying the backing fs of a mounted overlayfs is undefined
behavior, I adjusted the overlayfs setup timing so that the test files
are created before overlayfs is mounted.

With Best Regards,
AnonymeMeow

---
 .../kernel/syscalls/fanotify/fanotify13.c     | 50 +++++++++++++++----
 1 file changed, 41 insertions(+), 9 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
index 76d40eaf7..c4223c3e9 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify13.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
@@ -53,6 +53,9 @@
 #define FILE_PATH_ONE MOUNT_PATH"/"FILE_ONE
 #define FILE_PATH_TWO MOUNT_PATH"/"FILE_TWO
 
+#define TST_VARIANT_OVL_LOWER (tst_variant & 1)
+#define TST_VARIANT_OVL_WATCH (tst_variant > 2)
+
 #if defined(HAVE_NAME_TO_HANDLE_AT)
 struct event_t {
 	unsigned long long expected_mask;
@@ -137,6 +140,22 @@ static void delete_objects(void)
 	}
 }
 
+static void clean_upper_dir(void)
+{
+	unsigned int i;
+
+	SAFE_UMOUNT(MOUNT_PATH);
+	SAFE_UMOUNT(OVL_MNT);
+
+	SAFE_MOUNT(OVL_UPPER, MOUNT_PATH, "none", MS_BIND, NULL);
+	for (i = 0; i < ARRAY_SIZE(objects); i++)
+		SAFE_UNLINK(objects[i].path);
+	SAFE_UMOUNT(MOUNT_PATH);
+
+	SAFE_MOUNT_OVERLAY();
+	SAFE_MOUNT(OVL_MNT, MOUNT_PATH, "none", MS_BIND, NULL);
+}
+
 static void get_object_stats(void)
 {
 	unsigned int i;
@@ -340,6 +359,15 @@ static void do_test(unsigned int number)
 			"Did not get an expected event (expected: %llx)",
 			event_set[i].expected_mask);
 	}
+
+	if (tc->mask & FAN_DELETE_SELF) {
+		if (TST_VARIANT_OVL_LOWER) {
+			clean_upper_dir();
+		} else {
+			create_objects();
+			get_object_stats();
+		}
+	}
 out:
 	SAFE_CLOSE(fanotify_fd);
 }
@@ -367,14 +395,11 @@ static void do_setup(void)
 	 */
 	if (tst_variant) {
 		REQUIRE_HANDLE_TYPE_SUPPORTED_BY_KERNEL(AT_HANDLE_FID);
-		ovl_mounted = TST_MOUNT_OVERLAY();
-		if (!ovl_mounted)
-			return;
-
-		mnt = tst_variant & 1 ? OVL_LOWER : OVL_UPPER;
+		
+		tst_create_overlay_dirs();
+		mnt = TST_VARIANT_OVL_LOWER ? OVL_LOWER : OVL_UPPER;
 	} else {
 		mnt = OVL_BASE_MNTPOINT;
-
 	}
 	REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, mnt);
 	SAFE_MKDIR(MOUNT_PATH, 0755);
@@ -385,8 +410,14 @@ static void do_setup(void)
 
 	/* Create file and directory objects for testing on base fs */
 	create_objects();
+	
+	if (tst_variant) {
+		ovl_mounted = TST_MOUNT_OVERLAY();
+		if (!ovl_mounted)
+			return;
+	}
 
-	if (tst_variant > 2) {
+	if (TST_VARIANT_OVL_WATCH) {
 		/* Setup watches on overlayfs */
 		SAFE_MOUNT(OVL_MNT, MOUNT_PATH, "none", MS_BIND, NULL);
 		ovl_bind_mounted = 1;
@@ -416,12 +447,13 @@ static void do_cleanup(void)
 		SAFE_CLOSE(fanotify_fd);
 	if (ovl_bind_mounted)
 		SAFE_UMOUNT(MOUNT_PATH);
+	if (ovl_mounted)
+		SAFE_UMOUNT(OVL_MNT);
 	if (bind_mounted) {
+		delete_objects();
 		SAFE_UMOUNT(MOUNT_PATH);
 		SAFE_RMDIR(MOUNT_PATH);
 	}
-	if (ovl_mounted)
-		SAFE_UMOUNT(OVL_MNT);
 }
 
 static struct tst_test test = {
-- 
2.54.0


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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [LTP] fanotify13: fix test failure when running iterations
  2026-05-31 23:41   ` [LTP] [PATCH] " AnonymeMeow
@ 2026-06-01  4:11     ` linuxtestproject.agent
  2026-06-01  8:29     ` [LTP] [PATCH] " Amir Goldstein
  1 sibling, 0 replies; 15+ messages in thread
From: linuxtestproject.agent @ 2026-06-01  4:11 UTC (permalink / raw)
  To: AnonymeMeow; +Cc: ltp

Hi AnonymeMeow,

Thank you for the patch. One issue below.

> fanotify13: fix test failure when running iterations
>
> The FAN_DELETE_SELF test case removes test files, causing later
> iterations to crash. Recreate the removed files after the test to
> restore the expected initial state. And adjust the overlayfs mount
> timing to avoid creating files directly in the filesystem underneath
> the already mounted overlayfs.

The commit body describes two of the three meaningful changes in this
patch but omits the do_cleanup() fixes, which are a significant part
of the fix.

In do_cleanup(), the patch:
  1. Moves SAFE_UMOUNT(OVL_MNT) before the bind_mounted block (old
     ordering could leave the overlay mounted when clearing the bind
     mount in some mount stacking scenarios).
  2. Adds delete_objects() before unmounting MOUNT_PATH (ensures test
     files are removed from OVL_LOWER/OVL_UPPER so a persistent
     device does not carry stale state into the next iteration).

These are real correctness fixes that belong in the commit description.

Please expand the body to cover all three changes.

---

The code changes themselves look correct:

  * The new macros TST_VARIANT_OVL_LOWER = (tst_variant & 1) and
    TST_VARIANT_OVL_WATCH = (tst_variant > 2) faithfully replace the
    open-coded expressions and improve readability.

  * Moving TST_MOUNT_OVERLAY() after create_objects() is the right
    fix: objects are now created in OVL_LOWER before the overlay is
    stacked on top, avoiding undefined behaviour when modifying a
    lower directory while overlay is already mounted.

  * clean_upper_dir() correctly uses SAFE_UNLINK() for all objects
    (including the directory). After SAFE_RMDIR() on a lower-only
    directory through overlayfs, the kernel writes a char-device
    whiteout (0:0) to OVL_UPPER at that path — not an opaque
    directory — so SAFE_UNLINK() succeeds. The function is only
    reachable via variant 3 (variant 1's FAN_DELETE_SELF path is
    always TCONF'd by the "base fs cannot be watched for delete self
    events" guard), so this assumption always holds.

  * The new do_cleanup() ordering — ovl_bind → OVL_MNT → bind — is
    correct. For variant 3 after clean_upper_dir() the mounts are
    re-stacked identically to the post-setup state, so cleanup
    unwinds them in the right order.

Verdict: Needs revision (commit message only).

LTP AI Reviewer

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [LTP] [PATCH] fanotify13: fix test failure when running iterations
  2026-05-31 23:41   ` [LTP] [PATCH] " AnonymeMeow
  2026-06-01  4:11     ` [LTP] " linuxtestproject.agent
@ 2026-06-01  8:29     ` Amir Goldstein
  2026-06-03  1:58       ` [LTP] [PATCH v2] " AnonymeMeow
  1 sibling, 1 reply; 15+ messages in thread
From: Amir Goldstein @ 2026-06-01  8:29 UTC (permalink / raw)
  To: AnonymeMeow; +Cc: jack, ltp

On Mon, Jun 1, 2026 at 1:41 AM AnonymeMeow <anonymemeow@gmail.com> wrote:
>
> The FAN_DELETE_SELF test case removes test files, causing later
> iterations to crash. Recreate the removed files after the test to
> restore the expected initial state. And adjust the overlayfs mount
> timing to avoid creating files directly in the filesystem underneath
> the already mounted overlayfs.
>
> Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
> ---
>
> Hi, Amir,
>
> I think we shouldn't unconditionally recreate the files through the
> overlayfs. Because files created through overlayfs end up in the upper
> dir, but test variant 3 is intended to verify events reported for lower
> dir files opened through overlayfs, then starting from the second
> iteration variant 3 would test events for upper dir files instead, which
> does not match the intended test scenario.

Right. You caught me ;)

>
> Therefore, I updated my previous patch to unmount the overlayfs mount
> and its bind mount before cleaning the upper dir, and mount them back
> afterwards. This avoids modifying the underlying fs while the overlayfs
> is still mounted.
>
> And since modifying the backing fs of a mounted overlayfs is undefined
> behavior, I adjusted the overlayfs setup timing so that the test files
> are created before overlayfs is mounted.

Ok. Let's work on your patch.
Basically, it is correct and I have no objections with merging as is for
the purpose of fixing the problem.

But you strike me as one who appreciates the value (or aesthetics)
of a simpler and more elegant solution, so let me propose it.

>
> With Best Regards,
> AnonymeMeow
>
> ---
>  .../kernel/syscalls/fanotify/fanotify13.c     | 50 +++++++++++++++----
>  1 file changed, 41 insertions(+), 9 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
> index 76d40eaf7..c4223c3e9 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify13.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
> @@ -53,6 +53,9 @@
>  #define FILE_PATH_ONE MOUNT_PATH"/"FILE_ONE
>  #define FILE_PATH_TWO MOUNT_PATH"/"FILE_TWO
>
> +#define TST_VARIANT_OVL_LOWER (tst_variant & 1)
> +#define TST_VARIANT_OVL_WATCH (tst_variant > 2)
> +
>  #if defined(HAVE_NAME_TO_HANDLE_AT)
>  struct event_t {
>         unsigned long long expected_mask;
> @@ -137,6 +140,22 @@ static void delete_objects(void)
>         }
>  }
>
> +static void clean_upper_dir(void)
> +{
> +       unsigned int i;
> +
> +       SAFE_UMOUNT(MOUNT_PATH);
> +       SAFE_UMOUNT(OVL_MNT);
> +
> +       SAFE_MOUNT(OVL_UPPER, MOUNT_PATH, "none", MS_BIND, NULL);
> +       for (i = 0; i < ARRAY_SIZE(objects); i++)
> +               SAFE_UNLINK(objects[i].path);
> +       SAFE_UMOUNT(MOUNT_PATH);
> +
> +       SAFE_MOUNT_OVERLAY();
> +       SAFE_MOUNT(OVL_MNT, MOUNT_PATH, "none", MS_BIND, NULL);
> +}
> +

static void ovl_upper_dir_rotate(void)
{
        char dname[PATH_MAX];
        static int iter;

        SAFE_UMOUNT(MOUNT_PATH);
        SAFE_UMOUNT(OVL_MNT);

        sprintf(dname, "%s.%d", OVL_UPPER, iter++);
        SAFE_RENAME(OVL_UPPER, dname);
        SAFE_MKDIR(OVL_UPPER, 755);

        SAFE_MOUNT_OVERLAY();
        SAFE_MOUNT(OVL_MNT, MOUNT_PATH, "none", MS_BIND, NULL);
}

static void restore_objects(void)
{
        if (TST_VARIANT_OVL_LOWER)
                ovl_upper_dir_rotate();
        else
                create_objects();

        /* Get the re-created object ids */
        get_object_stats();
}


>  static void get_object_stats(void)
>  {
>         unsigned int i;
> @@ -340,6 +359,15 @@ static void do_test(unsigned int number)
>                         "Did not get an expected event (expected: %llx)",
>                         event_set[i].expected_mask);
>         }
> +
> +       if (tc->mask & FAN_DELETE_SELF) {
> +               if (TST_VARIANT_OVL_LOWER) {
> +                       clean_upper_dir();
> +               } else {
> +                       create_objects();
> +                       get_object_stats();
> +               }
> +       }

        /* Restore to state before delete_objects() */
        if (tc->mask & FAN_DELETE_SELF)
                restore_objects();

>  out:
>         SAFE_CLOSE(fanotify_fd);
>  }
> @@ -367,14 +395,11 @@ static void do_setup(void)
>          */
>         if (tst_variant) {
>                 REQUIRE_HANDLE_TYPE_SUPPORTED_BY_KERNEL(AT_HANDLE_FID);
> -               ovl_mounted = TST_MOUNT_OVERLAY();
> -               if (!ovl_mounted)
> -                       return;
> -
> -               mnt = tst_variant & 1 ? OVL_LOWER : OVL_UPPER;
> +

extra whitespaces and newline not needed

> +               tst_create_overlay_dirs();

trick: if you leave the newline here. diff will be cleaner
(tst_create_overlay_dirs
replaces TST_MOUNT_OVERLAY)

> +               mnt = TST_VARIANT_OVL_LOWER ? OVL_LOWER : OVL_UPPER;
>         } else {
>                 mnt = OVL_BASE_MNTPOINT;
> -
>         }
>         REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, mnt);
>         SAFE_MKDIR(MOUNT_PATH, 0755);
> @@ -385,8 +410,14 @@ static void do_setup(void)
>
>         /* Create file and directory objects for testing on base fs */
>         create_objects();
> +
> +       if (tst_variant) {
> +               ovl_mounted = TST_MOUNT_OVERLAY();
> +               if (!ovl_mounted)
> +                       return;
> +       }
>
> -       if (tst_variant > 2) {
> +       if (TST_VARIANT_OVL_WATCH) {
>                 /* Setup watches on overlayfs */
>                 SAFE_MOUNT(OVL_MNT, MOUNT_PATH, "none", MS_BIND, NULL);
>                 ovl_bind_mounted = 1;
> @@ -416,12 +447,13 @@ static void do_cleanup(void)
>                 SAFE_CLOSE(fanotify_fd);
>         if (ovl_bind_mounted)
>                 SAFE_UMOUNT(MOUNT_PATH);
> +       if (ovl_mounted)
> +               SAFE_UMOUNT(OVL_MNT);
>         if (bind_mounted) {
> +               delete_objects();

This clean is not needed because the entire filesystem was formatted
and mounted and is going to be torn down by the test framework.

>                 SAFE_UMOUNT(MOUNT_PATH);
>                 SAFE_RMDIR(MOUNT_PATH);
>         }
> -       if (ovl_mounted)
> -               SAFE_UMOUNT(OVL_MNT);


Although it is nice to unmount in the reverse order of mounts, the
order of unmounts here is not really significant, so I would avoid the churn.

Thanks,
Amir.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [LTP] [PATCH v2] fanotify13: fix test failure when running iterations
  2026-06-01  8:29     ` [LTP] [PATCH] " Amir Goldstein
@ 2026-06-03  1:58       ` AnonymeMeow
  2026-06-03  4:15         ` [LTP] " linuxtestproject.agent
  2026-06-03 10:24         ` [LTP] [PATCH v2] " Amir Goldstein
  0 siblings, 2 replies; 15+ messages in thread
From: AnonymeMeow @ 2026-06-03  1:58 UTC (permalink / raw)
  To: amir73il; +Cc: jack, ltp, AnonymeMeow

The FAN_DELETE_SELF test case removes test files, causing later
iterations to crash. Recreate the removed files after the test to
restore the expected initial state. And adjust the overlayfs mount
timing to avoid creating files directly in the filesystem underneath
the already mounted overlayfs.

Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
---

Hi, Amir,

I tested your suggested solution, but the overlayfs mount failed with:

fanotify13.c:165: TBROK: overlayfs mount failed: ESTALE (116)

So I kept clean_upper_dir() in restore_objects() to restore the expected
initial state.

With Best Regards,
AnonymeMeow

---
 .../kernel/syscalls/fanotify/fanotify13.c     | 49 ++++++++++++++++---
 1 file changed, 43 insertions(+), 6 deletions(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
index 76d40eaf7..182858470 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify13.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
@@ -53,6 +53,9 @@
 #define FILE_PATH_ONE MOUNT_PATH"/"FILE_ONE
 #define FILE_PATH_TWO MOUNT_PATH"/"FILE_TWO
 
+#define TST_VARIANT_OVL_LOWER (tst_variant & 1)
+#define TST_VARIANT_OVL_WATCH (tst_variant > 2)
+
 #if defined(HAVE_NAME_TO_HANDLE_AT)
 struct event_t {
 	unsigned long long expected_mask;
@@ -137,6 +140,22 @@ static void delete_objects(void)
 	}
 }
 
+static void clean_upper_dir(void)
+{
+	unsigned int i;
+
+	SAFE_UMOUNT(MOUNT_PATH);
+	SAFE_UMOUNT(OVL_MNT);
+
+	SAFE_MOUNT(OVL_UPPER, MOUNT_PATH, "none", MS_BIND, NULL);
+	for (i = 0; i < ARRAY_SIZE(objects); i++)
+		SAFE_UNLINK(objects[i].path);
+	SAFE_UMOUNT(MOUNT_PATH);
+
+	SAFE_MOUNT_OVERLAY();
+	SAFE_MOUNT(OVL_MNT, MOUNT_PATH, "none", MS_BIND, NULL);
+}
+
 static void get_object_stats(void)
 {
 	unsigned int i;
@@ -147,6 +166,17 @@ static void get_object_stats(void)
 	}
 }
 
+static void restore_objects(void)
+{
+	if (TST_VARIANT_OVL_LOWER)
+		clean_upper_dir();
+	else
+		create_objects();
+
+	/* Get the re-created object ids */
+	get_object_stats();
+}
+
 static int setup_marks(unsigned int fd, struct test_case_t *tc)
 {
 	unsigned int i;
@@ -340,6 +370,10 @@ static void do_test(unsigned int number)
 			"Did not get an expected event (expected: %llx)",
 			event_set[i].expected_mask);
 	}
+
+	/* Restore to state before delete_objects() */
+	if (tc->mask & FAN_DELETE_SELF)
+		restore_objects();
 out:
 	SAFE_CLOSE(fanotify_fd);
 }
@@ -367,14 +401,11 @@ static void do_setup(void)
 	 */
 	if (tst_variant) {
 		REQUIRE_HANDLE_TYPE_SUPPORTED_BY_KERNEL(AT_HANDLE_FID);
-		ovl_mounted = TST_MOUNT_OVERLAY();
-		if (!ovl_mounted)
-			return;
+		tst_create_overlay_dirs();
 
-		mnt = tst_variant & 1 ? OVL_LOWER : OVL_UPPER;
+		mnt = TST_VARIANT_OVL_LOWER ? OVL_LOWER : OVL_UPPER;
 	} else {
 		mnt = OVL_BASE_MNTPOINT;
-
 	}
 	REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, mnt);
 	SAFE_MKDIR(MOUNT_PATH, 0755);
@@ -385,8 +416,14 @@ static void do_setup(void)
 
 	/* Create file and directory objects for testing on base fs */
 	create_objects();
+	
+	if (tst_variant) {
+		ovl_mounted = TST_MOUNT_OVERLAY();
+		if (!ovl_mounted)
+			return;
+	}
 
-	if (tst_variant > 2) {
+	if (TST_VARIANT_OVL_WATCH) {
 		/* Setup watches on overlayfs */
 		SAFE_MOUNT(OVL_MNT, MOUNT_PATH, "none", MS_BIND, NULL);
 		ovl_bind_mounted = 1;
-- 
2.54.0


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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [LTP] fanotify13: fix test failure when running iterations
  2026-06-03  1:58       ` [LTP] [PATCH v2] " AnonymeMeow
@ 2026-06-03  4:15         ` linuxtestproject.agent
  2026-06-03 10:24         ` [LTP] [PATCH v2] " Amir Goldstein
  1 sibling, 0 replies; 15+ messages in thread
From: linuxtestproject.agent @ 2026-06-03  4:15 UTC (permalink / raw)
  To: AnonymeMeow; +Cc: ltp

Hi AnonymeMeow,

Thank you for the patch. The overall approach is correct — the two problems
identified (missing state restoration after FAN_DELETE_SELF and the overlayfs
mount-timing issue in setup) are real, and the split between
clean_upper_dir() for the lower-only case and create_objects() for the rest
is sound. However there is one correctness gap that needs fixing before this
can be applied.

---

[BUG] restore_objects() does not re-add the nofid_fd mark

In do_setup(), a mark without FAN_REPORT_FID is placed on FILE_PATH_ONE
specifically to exercise the "uninitialized connector->fsid cache" path
(regression test for c285a2f01d69):

    nofid_fd = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF, O_RDONLY);
    ...
    SAFE_FANOTIFY_MARK(nofid_fd, FAN_MARK_ADD, FAN_CLOSE_WRITE, AT_FDCWD,
                      FILE_PATH_ONE);

The new restore_objects() correctly rebuilds files and refreshes fids, but it
calls create_objects() for variants 0 and 4 (TST_VARIANT_OVL_LOWER == 0).
Those calls allocate new inodes at the same paths. The nofid_fd mark remains
bound to the *old*, now-orphaned inode; the new FILE_PATH_ONE inode has no
nofid mark at all. On the second iteration, setup_marks() attaches fanotify_fd
(FAN_REPORT_FID) to a fresh inode that has never had a non-FID mark, so the
very scenario that c285a2f01d69 fixes is no longer exercised.

For variant 3 (TST_VARIANT_OVL_LOWER == 1), clean_upper_dir() only removes
overlayfs whiteouts from OVL_UPPER and remounts the overlay; the original
OVL_LOWER inodes are never freed, so the nofid_fd mark is preserved. But
variants 0 and 4 both go through create_objects(), where the inodes are
replaced.

+static void restore_objects(void)
+{
+if (TST_VARIANT_OVL_LOWER)
+clean_upper_dir();
+else
+create_objects();
+
+/* Get the re-created object ids */
+get_object_stats();
+}

Fix: re-add the nofid_fd mark after re-creating the objects so that the
connector state matches do_setup():

 static void restore_objects(void)
 {
 if (TST_VARIANT_OVL_LOWER)
 clean_upper_dir();
 else
 create_objects();
 
+/* Re-add the nofid mark; create_objects() creates new inodes */
+SAFE_FANOTIFY_MARK(nofid_fd, FAN_MARK_ADD, FAN_CLOSE_WRITE,
+   AT_FDCWD, FILE_PATH_ONE);
+
 /* Get the re-created object ids */
 get_object_stats();
 }

For variant 3 this is a harmless no-op (FAN_MARK_ADD on an already-marked
inode succeeds and leaves the mark unchanged). For variants 0 and 4 it
restores full coverage of c285a2f01d69.

---

[NIT] Commit message: "crash" is imprecise

> The FAN_DELETE_SELF test case removes test files, causing later
> iterations to crash.

SAFE_OPEN() failing due to ENOENT causes tst_brk(TBROK, ...), which
terminates the iteration — not a crash. "fail" or "abort" would be more
accurate LTP terminology.

---

[NIT] Trailing whitespace in do_setup()

The blank line inserted after create_objects(); in do_setup() carries a
trailing TAB/space. checkpatch.pl will flag it. Please remove.

 /* Create file and directory objects for testing on base fs */
 create_objects();
-<trailing whitespace here>
+
 if (tst_variant) {

---

Everything else looks good:

- The macro definitions for TST_VARIANT_OVL_LOWER / TST_VARIANT_OVL_WATCH are
  correct for all 5 variants and replace the two open-coded expressions neatly.
- clean_upper_dir() correctly handles directory whiteouts: since all objects
  (including DIR_ONE) were created in OVL_LOWER and OVL_UPPER starts empty,
  overlayfs creates char-device (c 0 0) whiteouts in OVL_UPPER for all three
  deleted objects, so SAFE_UNLINK() works uniformly.
- Moving TST_MOUNT_OVERLAY() to after create_objects() correctly avoids
  creating files directly in the lower/upper directory while the overlay is
  already mounted.
- The mount/umount sequence in clean_upper_dir() interacts correctly with
  do_cleanup(): the OVL_LOWER bind mount (bind_mounted) remains on the bottom
  of the MOUNT_PATH stack and is unmounted last in cleanup.
- Kernel commits c285a2f01d69, bc2473c90fca, and c45beebfde34a are all present
  in kernel 7.1 (current stable); no staging concerns.
- runtest entry confirmed: syscalls/fanotify13 fanotify13.

Verdict: Needs revision

LTP AI Reviewer

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [LTP] [PATCH v2] fanotify13: fix test failure when running iterations
  2026-06-03  1:58       ` [LTP] [PATCH v2] " AnonymeMeow
  2026-06-03  4:15         ` [LTP] " linuxtestproject.agent
@ 2026-06-03 10:24         ` Amir Goldstein
  2026-06-03 11:51           ` Petr Vorel
  1 sibling, 1 reply; 15+ messages in thread
From: Amir Goldstein @ 2026-06-03 10:24 UTC (permalink / raw)
  To: AnonymeMeow; +Cc: jack, ltp

On Wed, Jun 3, 2026 at 3:58 AM AnonymeMeow <anonymemeow@gmail.com> wrote:
>
> The FAN_DELETE_SELF test case removes test files, causing later
> iterations to crash. Recreate the removed files after the test to
> restore the expected initial state. And adjust the overlayfs mount
> timing to avoid creating files directly in the filesystem underneath
> the already mounted overlayfs.
>
> Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
> ---
>
> Hi, Amir,
>
> I tested your suggested solution, but the overlayfs mount failed with:
>
> fanotify13.c:165: TBROK: overlayfs mount failed: ESTALE (116)
>
> So I kept clean_upper_dir() in restore_objects() to restore the expected
> initial state.

Ok no problem. This is fine too.
Thanks,

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

>
> With Best Regards,
> AnonymeMeow
>
> ---
>  .../kernel/syscalls/fanotify/fanotify13.c     | 49 ++++++++++++++++---
>  1 file changed, 43 insertions(+), 6 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify13.c b/testcases/kernel/syscalls/fanotify/fanotify13.c
> index 76d40eaf7..182858470 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify13.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify13.c
> @@ -53,6 +53,9 @@
>  #define FILE_PATH_ONE MOUNT_PATH"/"FILE_ONE
>  #define FILE_PATH_TWO MOUNT_PATH"/"FILE_TWO
>
> +#define TST_VARIANT_OVL_LOWER (tst_variant & 1)
> +#define TST_VARIANT_OVL_WATCH (tst_variant > 2)
> +
>  #if defined(HAVE_NAME_TO_HANDLE_AT)
>  struct event_t {
>         unsigned long long expected_mask;
> @@ -137,6 +140,22 @@ static void delete_objects(void)
>         }
>  }
>
> +static void clean_upper_dir(void)
> +{
> +       unsigned int i;
> +
> +       SAFE_UMOUNT(MOUNT_PATH);
> +       SAFE_UMOUNT(OVL_MNT);
> +
> +       SAFE_MOUNT(OVL_UPPER, MOUNT_PATH, "none", MS_BIND, NULL);
> +       for (i = 0; i < ARRAY_SIZE(objects); i++)
> +               SAFE_UNLINK(objects[i].path);
> +       SAFE_UMOUNT(MOUNT_PATH);
> +
> +       SAFE_MOUNT_OVERLAY();
> +       SAFE_MOUNT(OVL_MNT, MOUNT_PATH, "none", MS_BIND, NULL);
> +}
> +
>  static void get_object_stats(void)
>  {
>         unsigned int i;
> @@ -147,6 +166,17 @@ static void get_object_stats(void)
>         }
>  }
>
> +static void restore_objects(void)
> +{
> +       if (TST_VARIANT_OVL_LOWER)
> +               clean_upper_dir();
> +       else
> +               create_objects();
> +
> +       /* Get the re-created object ids */
> +       get_object_stats();
> +}
> +
>  static int setup_marks(unsigned int fd, struct test_case_t *tc)
>  {
>         unsigned int i;
> @@ -340,6 +370,10 @@ static void do_test(unsigned int number)
>                         "Did not get an expected event (expected: %llx)",
>                         event_set[i].expected_mask);
>         }
> +
> +       /* Restore to state before delete_objects() */
> +       if (tc->mask & FAN_DELETE_SELF)
> +               restore_objects();
>  out:
>         SAFE_CLOSE(fanotify_fd);
>  }
> @@ -367,14 +401,11 @@ static void do_setup(void)
>          */
>         if (tst_variant) {
>                 REQUIRE_HANDLE_TYPE_SUPPORTED_BY_KERNEL(AT_HANDLE_FID);
> -               ovl_mounted = TST_MOUNT_OVERLAY();
> -               if (!ovl_mounted)
> -                       return;
> +               tst_create_overlay_dirs();
>
> -               mnt = tst_variant & 1 ? OVL_LOWER : OVL_UPPER;
> +               mnt = TST_VARIANT_OVL_LOWER ? OVL_LOWER : OVL_UPPER;
>         } else {
>                 mnt = OVL_BASE_MNTPOINT;
> -
>         }
>         REQUIRE_FANOTIFY_INIT_FLAGS_SUPPORTED_ON_FS(FAN_REPORT_FID, mnt);
>         SAFE_MKDIR(MOUNT_PATH, 0755);
> @@ -385,8 +416,14 @@ static void do_setup(void)
>
>         /* Create file and directory objects for testing on base fs */
>         create_objects();
> +
> +       if (tst_variant) {
> +               ovl_mounted = TST_MOUNT_OVERLAY();
> +               if (!ovl_mounted)
> +                       return;
> +       }
>
> -       if (tst_variant > 2) {
> +       if (TST_VARIANT_OVL_WATCH) {
>                 /* Setup watches on overlayfs */
>                 SAFE_MOUNT(OVL_MNT, MOUNT_PATH, "none", MS_BIND, NULL);
>                 ovl_bind_mounted = 1;
> --
> 2.54.0
>

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [LTP] [PATCH v2] fanotify13: fix test failure when running iterations
  2026-06-03 10:24         ` [LTP] [PATCH v2] " Amir Goldstein
@ 2026-06-03 11:51           ` Petr Vorel
  0 siblings, 0 replies; 15+ messages in thread
From: Petr Vorel @ 2026-06-03 11:51 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: jack, ltp, AnonymeMeow

Hi all,

> On Wed, Jun 3, 2026 at 3:58 AM AnonymeMeow <anonymemeow@gmail.com> wrote:

> > The FAN_DELETE_SELF test case removes test files, causing later
> > iterations to crash. Recreate the removed files after the test to
> > restore the expected initial state. And adjust the overlayfs mount
> > timing to avoid creating files directly in the filesystem underneath
> > the already mounted overlayfs.

> > Signed-off-by: AnonymeMeow <anonymemeow@gmail.com>
> > ---

> > Hi, Amir,

> > I tested your suggested solution, but the overlayfs mount failed with:

> > fanotify13.c:165: TBROK: overlayfs mount failed: ESTALE (116)

> > So I kept clean_upper_dir() in restore_objects() to restore the expected
> > initial state.

> Ok no problem. This is fine too.
> Thanks,

> Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Good, merged, thank you!

Kind regards,
Petr

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [LTP] [PATCH v2 2/2] fanotify13: fix test failure when running iterations
  2026-05-30 14:01     ` Amir Goldstein
@ 2026-06-03 13:33       ` Petr Vorel
  0 siblings, 0 replies; 15+ messages in thread
From: Petr Vorel @ 2026-06-03 13:33 UTC (permalink / raw)
  To: Amir Goldstein; +Cc: Jan Kara, ltp

Hi Amir,

...
> > We even have it in testcases/kernel/syscalls/renameat2/renameat2.h, this
> > function should be moved to include/lapi/renameat2.h or include/lapi/stdio.h
> > (header which includes it). If I have time on Monday, I can do the cleanup and
> > fix it.

> Sounds good. Please change the test to use the common header.

OK, alternative solution which does not need renameat2() was merged.
But FYI I prepared the solution in case renameat2() is needed in the future.

https://patchwork.ozlabs.org/project/ltp/list/?series=507298&state=*
https://lore.kernel.org/ltp/20260603115828.587769-1-pvorel@suse.cz/T/#t

And again, thank you (and Jan) for your work to keep fanotify tests in LTP updated.

Kind regards,
Petr

> Thanks,
> Amir.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2026-06-03 13:34 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-28 16:29 [LTP] [PATCH v2 1/2] fanotify21: fix test failure when running iterations Amir Goldstein
2026-05-28 16:29 ` [LTP] [PATCH v2 2/2] fanotify13: " Amir Goldstein
2026-05-29 16:30   ` Petr Vorel
2026-05-30 14:01     ` Amir Goldstein
2026-06-03 13:33       ` Petr Vorel
2026-05-29 16:34   ` Petr Vorel
2026-05-31 23:41   ` [LTP] [PATCH] " AnonymeMeow
2026-06-01  4:11     ` [LTP] " linuxtestproject.agent
2026-06-01  8:29     ` [LTP] [PATCH] " Amir Goldstein
2026-06-03  1:58       ` [LTP] [PATCH v2] " AnonymeMeow
2026-06-03  4:15         ` [LTP] " linuxtestproject.agent
2026-06-03 10:24         ` [LTP] [PATCH v2] " Amir Goldstein
2026-06-03 11:51           ` Petr Vorel
2026-05-28 18:43 ` [LTP] fanotify21: " linuxtestproject.agent
2026-05-29 16:25 ` [LTP] [PATCH v2 1/2] " Petr Vorel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.