Re: [LTP] [PATCH v2 2/2] fanotify13: fix test failure when running iterations

[Petr Vorel <pvorel@suse.cz>]

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