diff mbox series

[5/5] syscalls/fanotify10: Add test cases for evictable ignore mark

Message ID 20220613143826.1328830-6-amir73il@gmail.com
State Accepted
Headers show
Series Fanotify tests for FAN_MARK_EVICTABLE | expand

Commit Message

Amir Goldstein June 13, 2022, 2:38 p.m. UTC
Test multiple groups with evictable mark with ignore mask

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 .../kernel/syscalls/fanotify/fanotify10.c     | 78 +++++++++++++++++++
 1 file changed, 78 insertions(+)

Comments

Jan Kara June 14, 2022, 1:04 p.m. UTC | #1
On Mon 13-06-22 17:38:26, Amir Goldstein wrote:
> Test multiple groups with evictable mark with ignore mask
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  .../kernel/syscalls/fanotify/fanotify10.c     | 78 +++++++++++++++++++
>  1 file changed, 78 insertions(+)
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> index b9a50672d..52277d0b7 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> @@ -71,6 +71,7 @@ static char event_buf[EVENT_BUF_LEN];
>  static int exec_events_unsupported;
>  static int fan_report_dfid_unsupported;
>  static int filesystem_mark_unsupported;
> +static int evictable_mark_unsupported;
>  
>  #define MOUNT_PATH "fs_mnt"
>  #define MNT2_PATH "mntpoint"
> @@ -90,6 +91,10 @@ static int filesystem_mark_unsupported;
>  #define FILE_EXEC_PATH2 MNT2_PATH"/"TEST_APP
>  #define FILE2_EXEC_PATH2 MNT2_PATH"/"TEST_APP2
>  
> +#define DROP_CACHES_FILE "/proc/sys/vm/drop_caches"
> +#define CACHE_PRESSURE_FILE "/proc/sys/vm/vfs_cache_pressure"
> +
> +static int old_cache_pressure;
>  static pid_t child_pid;
>  static int bind_mount_created;
>  static unsigned int num_classes = NUM_CLASSES;
> @@ -98,12 +103,14 @@ enum {
>  	FANOTIFY_INODE,
>  	FANOTIFY_MOUNT,
>  	FANOTIFY_FILESYSTEM,
> +	FANOTIFY_EVICTABLE,
>  };
>  
>  static struct fanotify_mark_type fanotify_mark_types[] = {
>  	INIT_FANOTIFY_MARK_TYPE(INODE),
>  	INIT_FANOTIFY_MARK_TYPE(MOUNT),
>  	INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
> +	INIT_FANOTIFY_MARK_TYPE(EVICTABLE),
>  };
>  
>  static struct tcase {
> @@ -289,14 +296,59 @@ static struct tcase {
>  		0,
>  		FILE_PATH, FAN_OPEN, FAN_OPEN
>  	},
> +	/* Evictable ignore mark test cases */
> +	{
> +		"don't ignore mount events created on file with evicted ignore mark",
> +		MOUNT_PATH, FANOTIFY_MOUNT,
> +		FILE_PATH, FANOTIFY_EVICTABLE,
> +		0,
> +		FILE_PATH, FAN_OPEN, FAN_OPEN
> +	},
> +	{
> +		"don't ignore fs events created on a file with evicted ignore mark",
> +		MOUNT_PATH, FANOTIFY_FILESYSTEM,
> +		FILE_PATH, FANOTIFY_EVICTABLE,
> +		0,
> +		FILE_PATH, FAN_OPEN, FAN_OPEN
> +	},
> +	{
> +		"don't ignore mount events created inside a parent with evicted ignore mark",
> +		MOUNT_PATH, FANOTIFY_MOUNT,
> +		DIR_PATH, FANOTIFY_EVICTABLE,
> +		FAN_EVENT_ON_CHILD,
> +		FILE_PATH, FAN_OPEN, FAN_OPEN
> +	},
> +	{
> +		"don't ignore fs events created inside a parent with evicted ignore mark",
> +		MOUNT_PATH, FANOTIFY_FILESYSTEM,
> +		DIR_PATH, FANOTIFY_EVICTABLE,
> +		FAN_EVENT_ON_CHILD,
> +		FILE_PATH, FAN_OPEN, FAN_OPEN
> +	},
>  };
>  
> +static void show_fanotify_marks(int fd)
> +{
> +	unsigned int mflags, mask, ignored_mask;
> +	char procfdinfo[100];
> +
> +	sprintf(procfdinfo, "/proc/%d/fdinfo/%d", (int)getpid(), fd);
> +	if (FILE_LINES_SCANF(procfdinfo, "fanotify ino:%*x sdev:%*x mflags: %x mask:%x ignored_mask:%x",
> +				&mflags, &mask, &ignored_mask)) {
> +		tst_res(TPASS, "No fanotify inode marks as expected");
> +	} else {
> +		tst_res(TFAIL, "Unexpected inode mark (mflags=%x, mask=%x ignored_mask=%x)",
> +				mflags, mask, ignored_mask);
> +	}
> +}
> +
>  static int create_fanotify_groups(unsigned int n)
>  {
>  	struct tcase *tc = &tcases[n];
>  	struct fanotify_mark_type *mark, *ignore_mark;
>  	unsigned int mark_ignored, mask;
>  	unsigned int p, i;
> +	int evictable_ignored = (tc->ignore_mark_type == FANOTIFY_EVICTABLE);
>  
>  	mark = &fanotify_mark_types[tc->mark_type];
>  	ignore_mark = &fanotify_mark_types[tc->ignore_mark_type];
> @@ -345,6 +397,20 @@ add_mark:
>  			}
>  		}
>  	}
> +
> +	/*
> +	 * drop_caches should evict inode from cache and remove evictable marks
> +	 */
> +	if (evictable_ignored) {
> +		SAFE_FILE_PRINTF(DROP_CACHES_FILE, "3");
> +		for (p = 0; p < num_classes; p++) {
> +			for (i = 0; i < GROUPS_PER_PRIO; i++) {
> +				if (fd_notify[p][i] > 0)
> +					show_fanotify_marks(fd_notify[p][i]);
> +			}
> +		}
> +	}
> +
>  	return 0;
>  }
>  
> @@ -439,6 +505,11 @@ static void test_fanotify(unsigned int n)
>  		return;
>  	}
>  
> +	if (evictable_mark_unsupported && tc->ignore_mark_type == FANOTIFY_EVICTABLE) {
> +		tst_res(TCONF, "FAN_MARK_EVICTABLE not supported in kernel?");
> +		return;
> +	}
> +
>  	if (tc->ignored_onchild && tst_kvercmp(5, 9, 0) < 0) {
>  		tst_res(TCONF, "ignored mask in combination with flag FAN_EVENT_ON_CHILD"
>  				" has undefined behavior on kernel < 5.9");
> @@ -527,6 +598,7 @@ static void setup(void)
>  	exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC,
>  								      FAN_CLASS_CONTENT, 0);
>  	filesystem_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_FILESYSTEM);
> +	evictable_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_EVICTABLE);
>  	fan_report_dfid_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_DFID_NAME,
>  									  MOUNT_PATH);
>  	if (fan_report_dfid_unsupported) {
> @@ -545,6 +617,10 @@ static void setup(void)
>  	/* Create another bind mount at another path for generating events */
>  	SAFE_MKDIR(MNT2_PATH, 0755);
>  	mount_cycle();
> +
> +	SAFE_FILE_SCANF(CACHE_PRESSURE_FILE, "%d", &old_cache_pressure);
> +	/* Set high priority for evicting inodes */
> +	SAFE_FILE_PRINTF(CACHE_PRESSURE_FILE, "500");
>  }
>  
>  static void cleanup(void)
> @@ -553,6 +629,8 @@ static void cleanup(void)
>  
>  	if (bind_mount_created)
>  		SAFE_UMOUNT(MNT2_PATH);
> +
> +	SAFE_FILE_PRINTF(CACHE_PRESSURE_FILE, "%d", old_cache_pressure);
>  }
>  
>  static const char *const resource_files[] = {
> -- 
> 2.25.1
>
Jan Stancek June 30, 2022, 6:27 a.m. UTC | #2
On Mon, Jun 13, 2022 at 4:39 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> Test multiple groups with evictable mark with ignore mask
>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  .../kernel/syscalls/fanotify/fanotify10.c     | 78 +++++++++++++++++++
>  1 file changed, 78 insertions(+)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> index b9a50672d..52277d0b7 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> @@ -71,6 +71,7 @@ static char event_buf[EVENT_BUF_LEN];
>  static int exec_events_unsupported;
>  static int fan_report_dfid_unsupported;
>  static int filesystem_mark_unsupported;
> +static int evictable_mark_unsupported;
>
>  #define MOUNT_PATH "fs_mnt"
>  #define MNT2_PATH "mntpoint"
> @@ -90,6 +91,10 @@ static int filesystem_mark_unsupported;
>  #define FILE_EXEC_PATH2 MNT2_PATH"/"TEST_APP
>  #define FILE2_EXEC_PATH2 MNT2_PATH"/"TEST_APP2
>
> +#define DROP_CACHES_FILE "/proc/sys/vm/drop_caches"
> +#define CACHE_PRESSURE_FILE "/proc/sys/vm/vfs_cache_pressure"
> +
> +static int old_cache_pressure;
>  static pid_t child_pid;
>  static int bind_mount_created;
>  static unsigned int num_classes = NUM_CLASSES;
> @@ -98,12 +103,14 @@ enum {
>         FANOTIFY_INODE,
>         FANOTIFY_MOUNT,
>         FANOTIFY_FILESYSTEM,
> +       FANOTIFY_EVICTABLE,
>  };
>
>  static struct fanotify_mark_type fanotify_mark_types[] = {
>         INIT_FANOTIFY_MARK_TYPE(INODE),
>         INIT_FANOTIFY_MARK_TYPE(MOUNT),
>         INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
> +       INIT_FANOTIFY_MARK_TYPE(EVICTABLE),
>  };
>
>  static struct tcase {
> @@ -289,14 +296,59 @@ static struct tcase {
>                 0,
>                 FILE_PATH, FAN_OPEN, FAN_OPEN
>         },
> +       /* Evictable ignore mark test cases */
> +       {
> +               "don't ignore mount events created on file with evicted ignore mark",
> +               MOUNT_PATH, FANOTIFY_MOUNT,
> +               FILE_PATH, FANOTIFY_EVICTABLE,
> +               0,
> +               FILE_PATH, FAN_OPEN, FAN_OPEN
> +       },
> +       {
> +               "don't ignore fs events created on a file with evicted ignore mark",
> +               MOUNT_PATH, FANOTIFY_FILESYSTEM,
> +               FILE_PATH, FANOTIFY_EVICTABLE,
> +               0,
> +               FILE_PATH, FAN_OPEN, FAN_OPEN
> +       },
> +       {
> +               "don't ignore mount events created inside a parent with evicted ignore mark",
> +               MOUNT_PATH, FANOTIFY_MOUNT,
> +               DIR_PATH, FANOTIFY_EVICTABLE,
> +               FAN_EVENT_ON_CHILD,
> +               FILE_PATH, FAN_OPEN, FAN_OPEN
> +       },
> +       {
> +               "don't ignore fs events created inside a parent with evicted ignore mark",
> +               MOUNT_PATH, FANOTIFY_FILESYSTEM,
> +               DIR_PATH, FANOTIFY_EVICTABLE,
> +               FAN_EVENT_ON_CHILD,
> +               FILE_PATH, FAN_OPEN, FAN_OPEN
> +       },

Hi,

we are seeing some sporadic failures from this last testcase, with
recent upstream kernels (v5.19-rc4-14-g941e3e791269).
Has anyone also ran into it and knows if it's bug on test side or kernel?

fanotify10.c:496: TINFO: Test #27: don't ignore fs events created
inside a parent with evicted ignore mark
fanotify10.c:338: TPASS: No fanotify inode marks as expected
fanotify10.c:338: TPASS: No fanotify inode marks as expected
fanotify10.c:338: TPASS: No fanotify inode marks as expected
fanotify10.c:340: TFAIL: Unexpected inode mark (mflags=240,
mask=8000020 ignored_mask=20)
fanotify10.c:340: TFAIL: Unexpected inode mark (mflags=240,
mask=8000020 ignored_mask=20)
fanotify10.c:340: TFAIL: Unexpected inode mark (mflags=240,
mask=8000020 ignored_mask=20)
fanotify10.c:340: TFAIL: Unexpected inode mark (mflags=240,
mask=8000020 ignored_mask=20)
fanotify10.c:340: TFAIL: Unexpected inode mark (mflags=240,
mask=8000020 ignored_mask=20)
fanotify10.c:340: TFAIL: Unexpected inode mark (mflags=240,
mask=8000020 ignored_mask=20)
fanotify10.c:340: TFAIL: Unexpected inode mark (mflags=240,
mask=8000020 ignored_mask=20)
fanotify10.c:340: TFAIL: Unexpected inode mark (mflags=240,
mask=8000020 ignored_mask=20)
fanotify10.c:340: TFAIL: Unexpected inode mark (mflags=240,
mask=8000020 ignored_mask=20)
fanotify10.c:455: TPASS: group 0 (8) got event: mask 20 pid=13307 fd=15
fanotify10.c:455: TPASS: group 1 (8) got event: mask 20 pid=13307 fd=15
fanotify10.c:455: TPASS: group 2 (8) got event: mask 20 pid=13307 fd=15
fanotify10.c:538: TFAIL: group 0 (4) with FAN_MARK_FILESYSTEM did not get event
fanotify10.c:538: TFAIL: group 1 (4) with FAN_MARK_FILESYSTEM did not get event
fanotify10.c:538: TFAIL: group 2 (4) with FAN_MARK_FILESYSTEM did not get event
fanotify10.c:538: TFAIL: group 0 (0) with FAN_MARK_FILESYSTEM did not get event
fanotify10.c:538: TFAIL: group 1 (0) with FAN_MARK_FILESYSTEM did not get event
fanotify10.c:538: TFAIL: group 2 (0) with FAN_MARK_FILESYSTEM did not get event
fanotify10.c:538: TFAIL: group 0 (e00) with FAN_MARK_FILESYSTEM did
not get event
fanotify10.c:538: TFAIL: group 1 (e00) with FAN_MARK_FILESYSTEM did
not get event
fanotify10.c:538: TFAIL: group 2 (e00) with FAN_MARK_FILESYSTEM did
not get event

Thanks,
Jan
Amir Goldstein June 30, 2022, 8:20 a.m. UTC | #3
On Thu, Jun 30, 2022 at 9:27 AM Jan Stancek <jstancek@redhat.com> wrote:
>
> On Mon, Jun 13, 2022 at 4:39 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > Test multiple groups with evictable mark with ignore mask
> >
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >  .../kernel/syscalls/fanotify/fanotify10.c     | 78 +++++++++++++++++++
> >  1 file changed, 78 insertions(+)
> >
> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > index b9a50672d..52277d0b7 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > @@ -71,6 +71,7 @@ static char event_buf[EVENT_BUF_LEN];
> >  static int exec_events_unsupported;
> >  static int fan_report_dfid_unsupported;
> >  static int filesystem_mark_unsupported;
> > +static int evictable_mark_unsupported;
> >
> >  #define MOUNT_PATH "fs_mnt"
> >  #define MNT2_PATH "mntpoint"
> > @@ -90,6 +91,10 @@ static int filesystem_mark_unsupported;
> >  #define FILE_EXEC_PATH2 MNT2_PATH"/"TEST_APP
> >  #define FILE2_EXEC_PATH2 MNT2_PATH"/"TEST_APP2
> >
> > +#define DROP_CACHES_FILE "/proc/sys/vm/drop_caches"
> > +#define CACHE_PRESSURE_FILE "/proc/sys/vm/vfs_cache_pressure"
> > +
> > +static int old_cache_pressure;
> >  static pid_t child_pid;
> >  static int bind_mount_created;
> >  static unsigned int num_classes = NUM_CLASSES;
> > @@ -98,12 +103,14 @@ enum {
> >         FANOTIFY_INODE,
> >         FANOTIFY_MOUNT,
> >         FANOTIFY_FILESYSTEM,
> > +       FANOTIFY_EVICTABLE,
> >  };
> >
> >  static struct fanotify_mark_type fanotify_mark_types[] = {
> >         INIT_FANOTIFY_MARK_TYPE(INODE),
> >         INIT_FANOTIFY_MARK_TYPE(MOUNT),
> >         INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
> > +       INIT_FANOTIFY_MARK_TYPE(EVICTABLE),
> >  };
> >
> >  static struct tcase {
> > @@ -289,14 +296,59 @@ static struct tcase {
> >                 0,
> >                 FILE_PATH, FAN_OPEN, FAN_OPEN
> >         },
> > +       /* Evictable ignore mark test cases */
> > +       {
> > +               "don't ignore mount events created on file with evicted ignore mark",
> > +               MOUNT_PATH, FANOTIFY_MOUNT,
> > +               FILE_PATH, FANOTIFY_EVICTABLE,
> > +               0,
> > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > +       },
> > +       {
> > +               "don't ignore fs events created on a file with evicted ignore mark",
> > +               MOUNT_PATH, FANOTIFY_FILESYSTEM,
> > +               FILE_PATH, FANOTIFY_EVICTABLE,
> > +               0,
> > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > +       },
> > +       {
> > +               "don't ignore mount events created inside a parent with evicted ignore mark",
> > +               MOUNT_PATH, FANOTIFY_MOUNT,
> > +               DIR_PATH, FANOTIFY_EVICTABLE,
> > +               FAN_EVENT_ON_CHILD,
> > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > +       },
> > +       {
> > +               "don't ignore fs events created inside a parent with evicted ignore mark",
> > +               MOUNT_PATH, FANOTIFY_FILESYSTEM,
> > +               DIR_PATH, FANOTIFY_EVICTABLE,
> > +               FAN_EVENT_ON_CHILD,
> > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > +       },
>
> Hi,
>
> we are seeing some sporadic failures from this last testcase, with
> recent upstream kernels (v5.19-rc4-14-g941e3e791269).
> Has anyone also ran into it and knows if it's bug on test side or kernel?
>
> fanotify10.c:496: TINFO: Test #27: don't ignore fs events created
> inside a parent with evicted ignore mark
> fanotify10.c:338: TPASS: No fanotify inode marks as expected
> fanotify10.c:338: TPASS: No fanotify inode marks as expected
> fanotify10.c:338: TPASS: No fanotify inode marks as expected
> fanotify10.c:340: TFAIL: Unexpected inode mark (mflags=240,
> mask=8000020 ignored_mask=20)

It is a test bug.
The problem is that we want to evict an inode, but there is no
reliable mechanism to do that.

This is the reason for this workaround in fanotify23:

        /* Shrinkers on other fs do not work reliably enough to
guarantee mark eviction on drop_caches */
        .dev_fs_type = "ext2",

I did not encounter the problem with fanotify10 myself, but it should
be the same.
fanotify10 is not filesystem dependent, so if you can apply the same workaround
from fanotify23 and it works on your systems we can do that.

Thanks,
Amir.
Jan Stancek July 7, 2022, 12:49 p.m. UTC | #4
On Thu, Jun 30, 2022 at 10:21 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Thu, Jun 30, 2022 at 9:27 AM Jan Stancek <jstancek@redhat.com> wrote:
> >
> > On Mon, Jun 13, 2022 at 4:39 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > Test multiple groups with evictable mark with ignore mask
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > ---
> > >  .../kernel/syscalls/fanotify/fanotify10.c     | 78 +++++++++++++++++++
> > >  1 file changed, 78 insertions(+)
> > >
> > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > index b9a50672d..52277d0b7 100644
> > > --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > @@ -71,6 +71,7 @@ static char event_buf[EVENT_BUF_LEN];
> > >  static int exec_events_unsupported;
> > >  static int fan_report_dfid_unsupported;
> > >  static int filesystem_mark_unsupported;
> > > +static int evictable_mark_unsupported;
> > >
> > >  #define MOUNT_PATH "fs_mnt"
> > >  #define MNT2_PATH "mntpoint"
> > > @@ -90,6 +91,10 @@ static int filesystem_mark_unsupported;
> > >  #define FILE_EXEC_PATH2 MNT2_PATH"/"TEST_APP
> > >  #define FILE2_EXEC_PATH2 MNT2_PATH"/"TEST_APP2
> > >
> > > +#define DROP_CACHES_FILE "/proc/sys/vm/drop_caches"
> > > +#define CACHE_PRESSURE_FILE "/proc/sys/vm/vfs_cache_pressure"
> > > +
> > > +static int old_cache_pressure;
> > >  static pid_t child_pid;
> > >  static int bind_mount_created;
> > >  static unsigned int num_classes = NUM_CLASSES;
> > > @@ -98,12 +103,14 @@ enum {
> > >         FANOTIFY_INODE,
> > >         FANOTIFY_MOUNT,
> > >         FANOTIFY_FILESYSTEM,
> > > +       FANOTIFY_EVICTABLE,
> > >  };
> > >
> > >  static struct fanotify_mark_type fanotify_mark_types[] = {
> > >         INIT_FANOTIFY_MARK_TYPE(INODE),
> > >         INIT_FANOTIFY_MARK_TYPE(MOUNT),
> > >         INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
> > > +       INIT_FANOTIFY_MARK_TYPE(EVICTABLE),
> > >  };
> > >
> > >  static struct tcase {
> > > @@ -289,14 +296,59 @@ static struct tcase {
> > >                 0,
> > >                 FILE_PATH, FAN_OPEN, FAN_OPEN
> > >         },
> > > +       /* Evictable ignore mark test cases */
> > > +       {
> > > +               "don't ignore mount events created on file with evicted ignore mark",
> > > +               MOUNT_PATH, FANOTIFY_MOUNT,
> > > +               FILE_PATH, FANOTIFY_EVICTABLE,
> > > +               0,
> > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > +       },
> > > +       {
> > > +               "don't ignore fs events created on a file with evicted ignore mark",
> > > +               MOUNT_PATH, FANOTIFY_FILESYSTEM,
> > > +               FILE_PATH, FANOTIFY_EVICTABLE,
> > > +               0,
> > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > +       },
> > > +       {
> > > +               "don't ignore mount events created inside a parent with evicted ignore mark",
> > > +               MOUNT_PATH, FANOTIFY_MOUNT,
> > > +               DIR_PATH, FANOTIFY_EVICTABLE,
> > > +               FAN_EVENT_ON_CHILD,
> > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > +       },
> > > +       {
> > > +               "don't ignore fs events created inside a parent with evicted ignore mark",
> > > +               MOUNT_PATH, FANOTIFY_FILESYSTEM,
> > > +               DIR_PATH, FANOTIFY_EVICTABLE,
> > > +               FAN_EVENT_ON_CHILD,
> > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > +       },
> >
> > Hi,
> >
> > we are seeing some sporadic failures from this last testcase, with
> > recent upstream kernels (v5.19-rc4-14-g941e3e791269).
> > Has anyone also ran into it and knows if it's bug on test side or kernel?
> >
> > fanotify10.c:496: TINFO: Test #27: don't ignore fs events created
> > inside a parent with evicted ignore mark
> > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > fanotify10.c:340: TFAIL: Unexpected inode mark (mflags=240,
> > mask=8000020 ignored_mask=20)
>
> It is a test bug.
> The problem is that we want to evict an inode, but there is no
> reliable mechanism to do that.
>
> This is the reason for this workaround in fanotify23:
>
>         /* Shrinkers on other fs do not work reliably enough to
> guarantee mark eviction on drop_caches */
>         .dev_fs_type = "ext2",
>
> I did not encounter the problem with fanotify10 myself, but it should
> be the same.
> fanotify10 is not filesystem dependent, so if you can apply the same workaround
> from fanotify23 and it works on your systems we can do that.

Test is using default fs type, which should already be ext2.
Here's a more complete log from failed test:
https://s3.us-east-1.amazonaws.com/arr-cki-prod-datawarehouse-public/datawarehouse-public/2022/06/30/redhat:576928171/build_ppc64le_redhat:576928171_ppc64le/tests/1/results_0001/job.01/recipes/12221009/tasks/8/logs/syscalls.fail.log
Amir Goldstein July 9, 2022, 10:09 a.m. UTC | #5
On Thu, Jul 7, 2022 at 3:49 PM Jan Stancek <jstancek@redhat.com> wrote:
>
> On Thu, Jun 30, 2022 at 10:21 AM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > On Thu, Jun 30, 2022 at 9:27 AM Jan Stancek <jstancek@redhat.com> wrote:
> > >
> > > On Mon, Jun 13, 2022 at 4:39 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > > >
> > > > Test multiple groups with evictable mark with ignore mask
> > > >
> > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > > ---
> > > >  .../kernel/syscalls/fanotify/fanotify10.c     | 78 +++++++++++++++++++
> > > >  1 file changed, 78 insertions(+)
> > > >
> > > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > > index b9a50672d..52277d0b7 100644
> > > > --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > > +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > > @@ -71,6 +71,7 @@ static char event_buf[EVENT_BUF_LEN];
> > > >  static int exec_events_unsupported;
> > > >  static int fan_report_dfid_unsupported;
> > > >  static int filesystem_mark_unsupported;
> > > > +static int evictable_mark_unsupported;
> > > >
> > > >  #define MOUNT_PATH "fs_mnt"
> > > >  #define MNT2_PATH "mntpoint"
> > > > @@ -90,6 +91,10 @@ static int filesystem_mark_unsupported;
> > > >  #define FILE_EXEC_PATH2 MNT2_PATH"/"TEST_APP
> > > >  #define FILE2_EXEC_PATH2 MNT2_PATH"/"TEST_APP2
> > > >
> > > > +#define DROP_CACHES_FILE "/proc/sys/vm/drop_caches"
> > > > +#define CACHE_PRESSURE_FILE "/proc/sys/vm/vfs_cache_pressure"
> > > > +
> > > > +static int old_cache_pressure;
> > > >  static pid_t child_pid;
> > > >  static int bind_mount_created;
> > > >  static unsigned int num_classes = NUM_CLASSES;
> > > > @@ -98,12 +103,14 @@ enum {
> > > >         FANOTIFY_INODE,
> > > >         FANOTIFY_MOUNT,
> > > >         FANOTIFY_FILESYSTEM,
> > > > +       FANOTIFY_EVICTABLE,
> > > >  };
> > > >
> > > >  static struct fanotify_mark_type fanotify_mark_types[] = {
> > > >         INIT_FANOTIFY_MARK_TYPE(INODE),
> > > >         INIT_FANOTIFY_MARK_TYPE(MOUNT),
> > > >         INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
> > > > +       INIT_FANOTIFY_MARK_TYPE(EVICTABLE),
> > > >  };
> > > >
> > > >  static struct tcase {
> > > > @@ -289,14 +296,59 @@ static struct tcase {
> > > >                 0,
> > > >                 FILE_PATH, FAN_OPEN, FAN_OPEN
> > > >         },
> > > > +       /* Evictable ignore mark test cases */
> > > > +       {
> > > > +               "don't ignore mount events created on file with evicted ignore mark",
> > > > +               MOUNT_PATH, FANOTIFY_MOUNT,
> > > > +               FILE_PATH, FANOTIFY_EVICTABLE,
> > > > +               0,
> > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > +       },
> > > > +       {
> > > > +               "don't ignore fs events created on a file with evicted ignore mark",
> > > > +               MOUNT_PATH, FANOTIFY_FILESYSTEM,
> > > > +               FILE_PATH, FANOTIFY_EVICTABLE,
> > > > +               0,
> > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > +       },
> > > > +       {
> > > > +               "don't ignore mount events created inside a parent with evicted ignore mark",
> > > > +               MOUNT_PATH, FANOTIFY_MOUNT,
> > > > +               DIR_PATH, FANOTIFY_EVICTABLE,
> > > > +               FAN_EVENT_ON_CHILD,
> > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > +       },
> > > > +       {
> > > > +               "don't ignore fs events created inside a parent with evicted ignore mark",
> > > > +               MOUNT_PATH, FANOTIFY_FILESYSTEM,
> > > > +               DIR_PATH, FANOTIFY_EVICTABLE,
> > > > +               FAN_EVENT_ON_CHILD,
> > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > +       },
> > >
> > > Hi,
> > >
> > > we are seeing some sporadic failures from this last testcase, with
> > > recent upstream kernels (v5.19-rc4-14-g941e3e791269).
> > > Has anyone also ran into it and knows if it's bug on test side or kernel?

Hi Jan,

I am traveling so I cannot work on solving the problem for the next week,
but I can explain the problem and offer suggestions for short term solutions
and maybe a longer term solution.

> > >
> > > fanotify10.c:496: TINFO: Test #27: don't ignore fs events created
> > > inside a parent with evicted ignore mark
> > > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > > fanotify10.c:340: TFAIL: Unexpected inode mark (mflags=240,
> > > mask=8000020 ignored_mask=20)
> >
> > It is a test bug.
> > The problem is that we want to evict an inode, but there is no
> > reliable mechanism to do that.
> >
> > This is the reason for this workaround in fanotify23:
> >
> >         /* Shrinkers on other fs do not work reliably enough to
> > guarantee mark eviction on drop_caches */
> >         .dev_fs_type = "ext2",
> >
> > I did not encounter the problem with fanotify10 myself, but it should
> > be the same.
> > fanotify10 is not filesystem dependent, so if you can apply the same workaround
> > from fanotify23 and it works on your systems we can do that.
>
> Test is using default fs type, which should already be ext2.

I see. You can try xfs on your system to see if it behaves better,
because xfs has a specialized inode shriker.

> Here's a more complete log from failed test:
> https://s3.us-east-1.amazonaws.com/arr-cki-prod-datawarehouse-public/datawarehouse-public/2022/06/30/redhat:576928171/build_ppc64le_redhat:576928171_ppc64le/tests/1/results_0001/job.01/recipes/12221009/tasks/8/logs/syscalls.fail.log
>

Are the failures only in the first test iteration?

As a long shot, I would try to remove mount_cycle() from setup().
I see that fanotify23 does not have it and it is not really needed, so
it may help
fanotify10 pass the first iteration.

The problem is that the drop_caches knob is not a reliable way to evict inodes,
so when a test like fanotify10 needs to evict an inode, random factors are mixed
into the test run.

As a quick band aid, I suggest to replace the TFAIL, "Unexpected inode mark"
in show_fanotify_marks() with TCONF, because the test failed to setup an
"evictable ignored mark that gets evicted", propagate a failure return
value from
create_fanotify_groups() => show_fanotify_marks() and skip the test case
instead of failing it.

For long term, it would be nice if LTP could provide a
drop_inode_cache() library
function that tries harder to make inode eviction work using tricks
like fanotify10
and fanotify23 increases vfs_cache_pressure.
To pressure memory shrinkers to evict more inodes, need to check the amount
of total RAM in the system and dirty pages and add enough dirty memory pages
that cannot be evicted to drive the inode shrinker to work harder.
Utilizing memory cgroups with some of the new per memcg cache eviction knobs
could also be an option, but I did not look into it and not sure on
which kernels
this is available.

Bottom line is that there is a lot of black magic involved with memory shrinkers
and as your report shows, hacks and trick may impact different systems in
different ways, so it is better it the infrastructure is developed and tested in
LTP lib and not in individual tests.

Hope this helps.

Thanks,
Amir.
Jan Stancek July 12, 2022, 8:19 a.m. UTC | #6
On Sat, Jul 9, 2022 at 12:09 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Thu, Jul 7, 2022 at 3:49 PM Jan Stancek <jstancek@redhat.com> wrote:
> >
> > On Thu, Jun 30, 2022 at 10:21 AM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > On Thu, Jun 30, 2022 at 9:27 AM Jan Stancek <jstancek@redhat.com> wrote:
> > > >
> > > > On Mon, Jun 13, 2022 at 4:39 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > > > >
> > > > > Test multiple groups with evictable mark with ignore mask
> > > > >
> > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > > > ---
> > > > >  .../kernel/syscalls/fanotify/fanotify10.c     | 78 +++++++++++++++++++
> > > > >  1 file changed, 78 insertions(+)
> > > > >
> > > > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > > > index b9a50672d..52277d0b7 100644
> > > > > --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > > > +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > > > @@ -71,6 +71,7 @@ static char event_buf[EVENT_BUF_LEN];
> > > > >  static int exec_events_unsupported;
> > > > >  static int fan_report_dfid_unsupported;
> > > > >  static int filesystem_mark_unsupported;
> > > > > +static int evictable_mark_unsupported;
> > > > >
> > > > >  #define MOUNT_PATH "fs_mnt"
> > > > >  #define MNT2_PATH "mntpoint"
> > > > > @@ -90,6 +91,10 @@ static int filesystem_mark_unsupported;
> > > > >  #define FILE_EXEC_PATH2 MNT2_PATH"/"TEST_APP
> > > > >  #define FILE2_EXEC_PATH2 MNT2_PATH"/"TEST_APP2
> > > > >
> > > > > +#define DROP_CACHES_FILE "/proc/sys/vm/drop_caches"
> > > > > +#define CACHE_PRESSURE_FILE "/proc/sys/vm/vfs_cache_pressure"
> > > > > +
> > > > > +static int old_cache_pressure;
> > > > >  static pid_t child_pid;
> > > > >  static int bind_mount_created;
> > > > >  static unsigned int num_classes = NUM_CLASSES;
> > > > > @@ -98,12 +103,14 @@ enum {
> > > > >         FANOTIFY_INODE,
> > > > >         FANOTIFY_MOUNT,
> > > > >         FANOTIFY_FILESYSTEM,
> > > > > +       FANOTIFY_EVICTABLE,
> > > > >  };
> > > > >
> > > > >  static struct fanotify_mark_type fanotify_mark_types[] = {
> > > > >         INIT_FANOTIFY_MARK_TYPE(INODE),
> > > > >         INIT_FANOTIFY_MARK_TYPE(MOUNT),
> > > > >         INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
> > > > > +       INIT_FANOTIFY_MARK_TYPE(EVICTABLE),
> > > > >  };
> > > > >
> > > > >  static struct tcase {
> > > > > @@ -289,14 +296,59 @@ static struct tcase {
> > > > >                 0,
> > > > >                 FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > >         },
> > > > > +       /* Evictable ignore mark test cases */
> > > > > +       {
> > > > > +               "don't ignore mount events created on file with evicted ignore mark",
> > > > > +               MOUNT_PATH, FANOTIFY_MOUNT,
> > > > > +               FILE_PATH, FANOTIFY_EVICTABLE,
> > > > > +               0,
> > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > +       },
> > > > > +       {
> > > > > +               "don't ignore fs events created on a file with evicted ignore mark",
> > > > > +               MOUNT_PATH, FANOTIFY_FILESYSTEM,
> > > > > +               FILE_PATH, FANOTIFY_EVICTABLE,
> > > > > +               0,
> > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > +       },
> > > > > +       {
> > > > > +               "don't ignore mount events created inside a parent with evicted ignore mark",
> > > > > +               MOUNT_PATH, FANOTIFY_MOUNT,
> > > > > +               DIR_PATH, FANOTIFY_EVICTABLE,
> > > > > +               FAN_EVENT_ON_CHILD,
> > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > +       },
> > > > > +       {
> > > > > +               "don't ignore fs events created inside a parent with evicted ignore mark",
> > > > > +               MOUNT_PATH, FANOTIFY_FILESYSTEM,
> > > > > +               DIR_PATH, FANOTIFY_EVICTABLE,
> > > > > +               FAN_EVENT_ON_CHILD,
> > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > +       },
> > > >
> > > > Hi,
> > > >
> > > > we are seeing some sporadic failures from this last testcase, with
> > > > recent upstream kernels (v5.19-rc4-14-g941e3e791269).
> > > > Has anyone also ran into it and knows if it's bug on test side or kernel?
>
> Hi Jan,
>
> I am traveling so I cannot work on solving the problem for the next week,
> but I can explain the problem and offer suggestions for short term solutions
> and maybe a longer term solution.
>
> > > >
> > > > fanotify10.c:496: TINFO: Test #27: don't ignore fs events created
> > > > inside a parent with evicted ignore mark
> > > > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > > > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > > > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > > > fanotify10.c:340: TFAIL: Unexpected inode mark (mflags=240,
> > > > mask=8000020 ignored_mask=20)
> > >
> > > It is a test bug.
> > > The problem is that we want to evict an inode, but there is no
> > > reliable mechanism to do that.
> > >
> > > This is the reason for this workaround in fanotify23:
> > >
> > >         /* Shrinkers on other fs do not work reliably enough to
> > > guarantee mark eviction on drop_caches */
> > >         .dev_fs_type = "ext2",
> > >
> > > I did not encounter the problem with fanotify10 myself, but it should
> > > be the same.
> > > fanotify10 is not filesystem dependent, so if you can apply the same workaround
> > > from fanotify23 and it works on your systems we can do that.
> >
> > Test is using default fs type, which should already be ext2.
>
> I see. You can try xfs on your system to see if it behaves better,
> because xfs has a specialized inode shriker.
>
> > Here's a more complete log from failed test:
> > https://s3.us-east-1.amazonaws.com/arr-cki-prod-datawarehouse-public/datawarehouse-public/2022/06/30/redhat:576928171/build_ppc64le_redhat:576928171_ppc64le/tests/1/results_0001/job.01/recipes/12221009/tasks/8/logs/syscalls.fail.log
> >
>
> Are the failures only in the first test iteration?

I think so. It runs in automated environment, where we see failures
about once a week. I haven't managed to reproduce it by hand yet.

>
> As a long shot, I would try to remove mount_cycle() from setup().
> I see that fanotify23 does not have it and it is not really needed, so
> it may help
> fanotify10 pass the first iteration.
>
> The problem is that the drop_caches knob is not a reliable way to evict inodes,
> so when a test like fanotify10 needs to evict an inode, random factors are mixed
> into the test run.
>
> As a quick band aid, I suggest to replace the TFAIL, "Unexpected inode mark"
> in show_fanotify_marks() with TCONF, because the test failed to setup an
> "evictable ignored mark that gets evicted", propagate a failure return
> value from
> create_fanotify_groups() => show_fanotify_marks() and skip the test case
> instead of failing it.

Thanks for suggestion.

>
> For long term, it would be nice if LTP could provide a
> drop_inode_cache() library
> function that tries harder to make inode eviction work using tricks
> like fanotify10
> and fanotify23 increases vfs_cache_pressure.
> To pressure memory shrinkers to evict more inodes, need to check the amount
> of total RAM in the system and dirty pages and add enough dirty memory pages
> that cannot be evicted to drive the inode shrinker to work harder.
> Utilizing memory cgroups with some of the new per memcg cache eviction knobs
> could also be an option, but I did not look into it and not sure on
> which kernels
> this is available.
>
> Bottom line is that there is a lot of black magic involved with memory shrinkers
> and as your report shows, hacks and trick may impact different systems in
> different ways, so it is better it the infrastructure is developed and tested in
> LTP lib and not in individual tests.
>
> Hope this helps.
>
> Thanks,
> Amir.
>
Jan Kara Aug. 24, 2022, 3:24 p.m. UTC | #7
On Tue 12-07-22 10:19:19, Jan Stancek wrote:
> On Sat, Jul 9, 2022 at 12:09 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > On Thu, Jul 7, 2022 at 3:49 PM Jan Stancek <jstancek@redhat.com> wrote:
> > >
> > > On Thu, Jun 30, 2022 at 10:21 AM Amir Goldstein <amir73il@gmail.com> wrote:
> > > >
> > > > On Thu, Jun 30, 2022 at 9:27 AM Jan Stancek <jstancek@redhat.com> wrote:
> > > > >
> > > > > On Mon, Jun 13, 2022 at 4:39 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > > > > >
> > > > > > Test multiple groups with evictable mark with ignore mask
> > > > > >
> > > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > > > > ---
> > > > > >  .../kernel/syscalls/fanotify/fanotify10.c     | 78 +++++++++++++++++++
> > > > > >  1 file changed, 78 insertions(+)
> > > > > >
> > > > > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > > > > index b9a50672d..52277d0b7 100644
> > > > > > --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > > > > +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > > > > @@ -71,6 +71,7 @@ static char event_buf[EVENT_BUF_LEN];
> > > > > >  static int exec_events_unsupported;
> > > > > >  static int fan_report_dfid_unsupported;
> > > > > >  static int filesystem_mark_unsupported;
> > > > > > +static int evictable_mark_unsupported;
> > > > > >
> > > > > >  #define MOUNT_PATH "fs_mnt"
> > > > > >  #define MNT2_PATH "mntpoint"
> > > > > > @@ -90,6 +91,10 @@ static int filesystem_mark_unsupported;
> > > > > >  #define FILE_EXEC_PATH2 MNT2_PATH"/"TEST_APP
> > > > > >  #define FILE2_EXEC_PATH2 MNT2_PATH"/"TEST_APP2
> > > > > >
> > > > > > +#define DROP_CACHES_FILE "/proc/sys/vm/drop_caches"
> > > > > > +#define CACHE_PRESSURE_FILE "/proc/sys/vm/vfs_cache_pressure"
> > > > > > +
> > > > > > +static int old_cache_pressure;
> > > > > >  static pid_t child_pid;
> > > > > >  static int bind_mount_created;
> > > > > >  static unsigned int num_classes = NUM_CLASSES;
> > > > > > @@ -98,12 +103,14 @@ enum {
> > > > > >         FANOTIFY_INODE,
> > > > > >         FANOTIFY_MOUNT,
> > > > > >         FANOTIFY_FILESYSTEM,
> > > > > > +       FANOTIFY_EVICTABLE,
> > > > > >  };
> > > > > >
> > > > > >  static struct fanotify_mark_type fanotify_mark_types[] = {
> > > > > >         INIT_FANOTIFY_MARK_TYPE(INODE),
> > > > > >         INIT_FANOTIFY_MARK_TYPE(MOUNT),
> > > > > >         INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
> > > > > > +       INIT_FANOTIFY_MARK_TYPE(EVICTABLE),
> > > > > >  };
> > > > > >
> > > > > >  static struct tcase {
> > > > > > @@ -289,14 +296,59 @@ static struct tcase {
> > > > > >                 0,
> > > > > >                 FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > >         },
> > > > > > +       /* Evictable ignore mark test cases */
> > > > > > +       {
> > > > > > +               "don't ignore mount events created on file with evicted ignore mark",
> > > > > > +               MOUNT_PATH, FANOTIFY_MOUNT,
> > > > > > +               FILE_PATH, FANOTIFY_EVICTABLE,
> > > > > > +               0,
> > > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > +       },
> > > > > > +       {
> > > > > > +               "don't ignore fs events created on a file with evicted ignore mark",
> > > > > > +               MOUNT_PATH, FANOTIFY_FILESYSTEM,
> > > > > > +               FILE_PATH, FANOTIFY_EVICTABLE,
> > > > > > +               0,
> > > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > +       },
> > > > > > +       {
> > > > > > +               "don't ignore mount events created inside a parent with evicted ignore mark",
> > > > > > +               MOUNT_PATH, FANOTIFY_MOUNT,
> > > > > > +               DIR_PATH, FANOTIFY_EVICTABLE,
> > > > > > +               FAN_EVENT_ON_CHILD,
> > > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > +       },
> > > > > > +       {
> > > > > > +               "don't ignore fs events created inside a parent with evicted ignore mark",
> > > > > > +               MOUNT_PATH, FANOTIFY_FILESYSTEM,
> > > > > > +               DIR_PATH, FANOTIFY_EVICTABLE,
> > > > > > +               FAN_EVENT_ON_CHILD,
> > > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > +       },
> > > > >
> > > > > Hi,
> > > > >
> > > > > we are seeing some sporadic failures from this last testcase, with
> > > > > recent upstream kernels (v5.19-rc4-14-g941e3e791269).
> > > > > Has anyone also ran into it and knows if it's bug on test side or kernel?
> >
> > Hi Jan,
> >
> > I am traveling so I cannot work on solving the problem for the next week,
> > but I can explain the problem and offer suggestions for short term solutions
> > and maybe a longer term solution.
> >
> > > > >
> > > > > fanotify10.c:496: TINFO: Test #27: don't ignore fs events created
> > > > > inside a parent with evicted ignore mark
> > > > > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > > > > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > > > > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > > > > fanotify10.c:340: TFAIL: Unexpected inode mark (mflags=240,
> > > > > mask=8000020 ignored_mask=20)
> > > >
> > > > It is a test bug.
> > > > The problem is that we want to evict an inode, but there is no
> > > > reliable mechanism to do that.
> > > >
> > > > This is the reason for this workaround in fanotify23:
> > > >
> > > >         /* Shrinkers on other fs do not work reliably enough to
> > > > guarantee mark eviction on drop_caches */
> > > >         .dev_fs_type = "ext2",
> > > >
> > > > I did not encounter the problem with fanotify10 myself, but it should
> > > > be the same.
> > > > fanotify10 is not filesystem dependent, so if you can apply the same workaround
> > > > from fanotify23 and it works on your systems we can do that.
> > >
> > > Test is using default fs type, which should already be ext2.
> >
> > I see. You can try xfs on your system to see if it behaves better,
> > because xfs has a specialized inode shriker.
> >
> > > Here's a more complete log from failed test:
> > > https://s3.us-east-1.amazonaws.com/arr-cki-prod-datawarehouse-public/datawarehouse-public/2022/06/30/redhat:576928171/build_ppc64le_redhat:576928171_ppc64le/tests/1/results_0001/job.01/recipes/12221009/tasks/8/logs/syscalls.fail.log
> > >
> >
> > Are the failures only in the first test iteration?
> 
> I think so. It runs in automated environment, where we see failures
> about once a week. I haven't managed to reproduce it by hand yet.
> 
> >
> > As a long shot, I would try to remove mount_cycle() from setup().
> > I see that fanotify23 does not have it and it is not really needed, so
> > it may help
> > fanotify10 pass the first iteration.
> >
> > The problem is that the drop_caches knob is not a reliable way to evict inodes,
> > so when a test like fanotify10 needs to evict an inode, random factors are mixed
> > into the test run.
> >
> > As a quick band aid, I suggest to replace the TFAIL, "Unexpected inode mark"
> > in show_fanotify_marks() with TCONF, because the test failed to setup an
> > "evictable ignored mark that gets evicted", propagate a failure return
> > value from
> > create_fanotify_groups() => show_fanotify_marks() and skip the test case
> > instead of failing it.
> 
> Thanks for suggestion.

Is this still an issue? I didn't see anything happening in the fanotify10
test upstream. If the issue still happens, maybe something like the
attached patch may improve the situation? Jan, do you have a chance to test
it?

								Honza
Amir Goldstein Aug. 24, 2022, 6:13 p.m. UTC | #8
On Wed, Aug 24, 2022 at 6:24 PM Jan Kara <jack@suse.cz> wrote:
>
> On Tue 12-07-22 10:19:19, Jan Stancek wrote:
> > On Sat, Jul 9, 2022 at 12:09 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > On Thu, Jul 7, 2022 at 3:49 PM Jan Stancek <jstancek@redhat.com> wrote:
> > > >
> > > > On Thu, Jun 30, 2022 at 10:21 AM Amir Goldstein <amir73il@gmail.com> wrote:
> > > > >
> > > > > On Thu, Jun 30, 2022 at 9:27 AM Jan Stancek <jstancek@redhat.com> wrote:
> > > > > >
> > > > > > On Mon, Jun 13, 2022 at 4:39 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > > > > > >
> > > > > > > Test multiple groups with evictable mark with ignore mask
> > > > > > >
> > > > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > > > > > ---
> > > > > > >  .../kernel/syscalls/fanotify/fanotify10.c     | 78 +++++++++++++++++++
> > > > > > >  1 file changed, 78 insertions(+)
> > > > > > >
> > > > > > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > > > > > index b9a50672d..52277d0b7 100644
> > > > > > > --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > > > > > +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > > > > > @@ -71,6 +71,7 @@ static char event_buf[EVENT_BUF_LEN];
> > > > > > >  static int exec_events_unsupported;
> > > > > > >  static int fan_report_dfid_unsupported;
> > > > > > >  static int filesystem_mark_unsupported;
> > > > > > > +static int evictable_mark_unsupported;
> > > > > > >
> > > > > > >  #define MOUNT_PATH "fs_mnt"
> > > > > > >  #define MNT2_PATH "mntpoint"
> > > > > > > @@ -90,6 +91,10 @@ static int filesystem_mark_unsupported;
> > > > > > >  #define FILE_EXEC_PATH2 MNT2_PATH"/"TEST_APP
> > > > > > >  #define FILE2_EXEC_PATH2 MNT2_PATH"/"TEST_APP2
> > > > > > >
> > > > > > > +#define DROP_CACHES_FILE "/proc/sys/vm/drop_caches"
> > > > > > > +#define CACHE_PRESSURE_FILE "/proc/sys/vm/vfs_cache_pressure"
> > > > > > > +
> > > > > > > +static int old_cache_pressure;
> > > > > > >  static pid_t child_pid;
> > > > > > >  static int bind_mount_created;
> > > > > > >  static unsigned int num_classes = NUM_CLASSES;
> > > > > > > @@ -98,12 +103,14 @@ enum {
> > > > > > >         FANOTIFY_INODE,
> > > > > > >         FANOTIFY_MOUNT,
> > > > > > >         FANOTIFY_FILESYSTEM,
> > > > > > > +       FANOTIFY_EVICTABLE,
> > > > > > >  };
> > > > > > >
> > > > > > >  static struct fanotify_mark_type fanotify_mark_types[] = {
> > > > > > >         INIT_FANOTIFY_MARK_TYPE(INODE),
> > > > > > >         INIT_FANOTIFY_MARK_TYPE(MOUNT),
> > > > > > >         INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
> > > > > > > +       INIT_FANOTIFY_MARK_TYPE(EVICTABLE),
> > > > > > >  };
> > > > > > >
> > > > > > >  static struct tcase {
> > > > > > > @@ -289,14 +296,59 @@ static struct tcase {
> > > > > > >                 0,
> > > > > > >                 FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > >         },
> > > > > > > +       /* Evictable ignore mark test cases */
> > > > > > > +       {
> > > > > > > +               "don't ignore mount events created on file with evicted ignore mark",
> > > > > > > +               MOUNT_PATH, FANOTIFY_MOUNT,
> > > > > > > +               FILE_PATH, FANOTIFY_EVICTABLE,
> > > > > > > +               0,
> > > > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > > +       },
> > > > > > > +       {
> > > > > > > +               "don't ignore fs events created on a file with evicted ignore mark",
> > > > > > > +               MOUNT_PATH, FANOTIFY_FILESYSTEM,
> > > > > > > +               FILE_PATH, FANOTIFY_EVICTABLE,
> > > > > > > +               0,
> > > > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > > +       },
> > > > > > > +       {
> > > > > > > +               "don't ignore mount events created inside a parent with evicted ignore mark",
> > > > > > > +               MOUNT_PATH, FANOTIFY_MOUNT,
> > > > > > > +               DIR_PATH, FANOTIFY_EVICTABLE,
> > > > > > > +               FAN_EVENT_ON_CHILD,
> > > > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > > +       },
> > > > > > > +       {
> > > > > > > +               "don't ignore fs events created inside a parent with evicted ignore mark",
> > > > > > > +               MOUNT_PATH, FANOTIFY_FILESYSTEM,
> > > > > > > +               DIR_PATH, FANOTIFY_EVICTABLE,
> > > > > > > +               FAN_EVENT_ON_CHILD,
> > > > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > > +       },
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > we are seeing some sporadic failures from this last testcase, with
> > > > > > recent upstream kernels (v5.19-rc4-14-g941e3e791269).
> > > > > > Has anyone also ran into it and knows if it's bug on test side or kernel?
> > >
> > > Hi Jan,
> > >
> > > I am traveling so I cannot work on solving the problem for the next week,
> > > but I can explain the problem and offer suggestions for short term solutions
> > > and maybe a longer term solution.
> > >
> > > > > >
> > > > > > fanotify10.c:496: TINFO: Test #27: don't ignore fs events created
> > > > > > inside a parent with evicted ignore mark
> > > > > > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > > > > > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > > > > > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > > > > > fanotify10.c:340: TFAIL: Unexpected inode mark (mflags=240,
> > > > > > mask=8000020 ignored_mask=20)
> > > > >
> > > > > It is a test bug.
> > > > > The problem is that we want to evict an inode, but there is no
> > > > > reliable mechanism to do that.
> > > > >
> > > > > This is the reason for this workaround in fanotify23:
> > > > >
> > > > >         /* Shrinkers on other fs do not work reliably enough to
> > > > > guarantee mark eviction on drop_caches */
> > > > >         .dev_fs_type = "ext2",
> > > > >
> > > > > I did not encounter the problem with fanotify10 myself, but it should
> > > > > be the same.
> > > > > fanotify10 is not filesystem dependent, so if you can apply the same workaround
> > > > > from fanotify23 and it works on your systems we can do that.
> > > >
> > > > Test is using default fs type, which should already be ext2.
> > >
> > > I see. You can try xfs on your system to see if it behaves better,
> > > because xfs has a specialized inode shriker.
> > >
> > > > Here's a more complete log from failed test:
> > > > https://s3.us-east-1.amazonaws.com/arr-cki-prod-datawarehouse-public/datawarehouse-public/2022/06/30/redhat:576928171/build_ppc64le_redhat:576928171_ppc64le/tests/1/results_0001/job.01/recipes/12221009/tasks/8/logs/syscalls.fail.log
> > > >
> > >
> > > Are the failures only in the first test iteration?
> >
> > I think so. It runs in automated environment, where we see failures
> > about once a week. I haven't managed to reproduce it by hand yet.
> >
> > >
> > > As a long shot, I would try to remove mount_cycle() from setup().
> > > I see that fanotify23 does not have it and it is not really needed, so
> > > it may help
> > > fanotify10 pass the first iteration.
> > >
> > > The problem is that the drop_caches knob is not a reliable way to evict inodes,
> > > so when a test like fanotify10 needs to evict an inode, random factors are mixed
> > > into the test run.
> > >
> > > As a quick band aid, I suggest to replace the TFAIL, "Unexpected inode mark"
> > > in show_fanotify_marks() with TCONF, because the test failed to setup an
> > > "evictable ignored mark that gets evicted", propagate a failure return
> > > value from
> > > create_fanotify_groups() => show_fanotify_marks() and skip the test case
> > > instead of failing it.
> >
> > Thanks for suggestion.
>
> Is this still an issue? I didn't see anything happening in the fanotify10
> test upstream. If the issue still happens, maybe something like the
> attached patch may improve the situation? Jan, do you have a chance to test
> it?
>

Jan,

Did you notice that the test already does mount_cycle()
after setup and that there is no modification of any file during the test?
I don't see how syncfs() would make a difference.

Maybe it's an atime update of the executed file??
I think at some point in the discussion it was said that the issue
only happens on the first run of the test loop.

If it's atime update then syncfs() might help and adding
noatime mount flags to the test could also fix it.

Thanks,
Amir.
Jan Kara Aug. 25, 2022, 9:33 a.m. UTC | #9
On Wed 24-08-22 21:13:59, Amir Goldstein wrote:
> On Wed, Aug 24, 2022 at 6:24 PM Jan Kara <jack@suse.cz> wrote:
> >
> > On Tue 12-07-22 10:19:19, Jan Stancek wrote:
> > > On Sat, Jul 9, 2022 at 12:09 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > > >
> > > > On Thu, Jul 7, 2022 at 3:49 PM Jan Stancek <jstancek@redhat.com> wrote:
> > > > >
> > > > > On Thu, Jun 30, 2022 at 10:21 AM Amir Goldstein <amir73il@gmail.com> wrote:
> > > > > >
> > > > > > On Thu, Jun 30, 2022 at 9:27 AM Jan Stancek <jstancek@redhat.com> wrote:
> > > > > > >
> > > > > > > On Mon, Jun 13, 2022 at 4:39 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > > > > > > >
> > > > > > > > Test multiple groups with evictable mark with ignore mask
> > > > > > > >
> > > > > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > > > > > > ---
> > > > > > > >  .../kernel/syscalls/fanotify/fanotify10.c     | 78 +++++++++++++++++++
> > > > > > > >  1 file changed, 78 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > > > > > > index b9a50672d..52277d0b7 100644
> > > > > > > > --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > > > > > > +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > > > > > > @@ -71,6 +71,7 @@ static char event_buf[EVENT_BUF_LEN];
> > > > > > > >  static int exec_events_unsupported;
> > > > > > > >  static int fan_report_dfid_unsupported;
> > > > > > > >  static int filesystem_mark_unsupported;
> > > > > > > > +static int evictable_mark_unsupported;
> > > > > > > >
> > > > > > > >  #define MOUNT_PATH "fs_mnt"
> > > > > > > >  #define MNT2_PATH "mntpoint"
> > > > > > > > @@ -90,6 +91,10 @@ static int filesystem_mark_unsupported;
> > > > > > > >  #define FILE_EXEC_PATH2 MNT2_PATH"/"TEST_APP
> > > > > > > >  #define FILE2_EXEC_PATH2 MNT2_PATH"/"TEST_APP2
> > > > > > > >
> > > > > > > > +#define DROP_CACHES_FILE "/proc/sys/vm/drop_caches"
> > > > > > > > +#define CACHE_PRESSURE_FILE "/proc/sys/vm/vfs_cache_pressure"
> > > > > > > > +
> > > > > > > > +static int old_cache_pressure;
> > > > > > > >  static pid_t child_pid;
> > > > > > > >  static int bind_mount_created;
> > > > > > > >  static unsigned int num_classes = NUM_CLASSES;
> > > > > > > > @@ -98,12 +103,14 @@ enum {
> > > > > > > >         FANOTIFY_INODE,
> > > > > > > >         FANOTIFY_MOUNT,
> > > > > > > >         FANOTIFY_FILESYSTEM,
> > > > > > > > +       FANOTIFY_EVICTABLE,
> > > > > > > >  };
> > > > > > > >
> > > > > > > >  static struct fanotify_mark_type fanotify_mark_types[] = {
> > > > > > > >         INIT_FANOTIFY_MARK_TYPE(INODE),
> > > > > > > >         INIT_FANOTIFY_MARK_TYPE(MOUNT),
> > > > > > > >         INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
> > > > > > > > +       INIT_FANOTIFY_MARK_TYPE(EVICTABLE),
> > > > > > > >  };
> > > > > > > >
> > > > > > > >  static struct tcase {
> > > > > > > > @@ -289,14 +296,59 @@ static struct tcase {
> > > > > > > >                 0,
> > > > > > > >                 FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > > >         },
> > > > > > > > +       /* Evictable ignore mark test cases */
> > > > > > > > +       {
> > > > > > > > +               "don't ignore mount events created on file with evicted ignore mark",
> > > > > > > > +               MOUNT_PATH, FANOTIFY_MOUNT,
> > > > > > > > +               FILE_PATH, FANOTIFY_EVICTABLE,
> > > > > > > > +               0,
> > > > > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > > > +       },
> > > > > > > > +       {
> > > > > > > > +               "don't ignore fs events created on a file with evicted ignore mark",
> > > > > > > > +               MOUNT_PATH, FANOTIFY_FILESYSTEM,
> > > > > > > > +               FILE_PATH, FANOTIFY_EVICTABLE,
> > > > > > > > +               0,
> > > > > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > > > +       },
> > > > > > > > +       {
> > > > > > > > +               "don't ignore mount events created inside a parent with evicted ignore mark",
> > > > > > > > +               MOUNT_PATH, FANOTIFY_MOUNT,
> > > > > > > > +               DIR_PATH, FANOTIFY_EVICTABLE,
> > > > > > > > +               FAN_EVENT_ON_CHILD,
> > > > > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > > > +       },
> > > > > > > > +       {
> > > > > > > > +               "don't ignore fs events created inside a parent with evicted ignore mark",
> > > > > > > > +               MOUNT_PATH, FANOTIFY_FILESYSTEM,
> > > > > > > > +               DIR_PATH, FANOTIFY_EVICTABLE,
> > > > > > > > +               FAN_EVENT_ON_CHILD,
> > > > > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > > > +       },
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > we are seeing some sporadic failures from this last testcase, with
> > > > > > > recent upstream kernels (v5.19-rc4-14-g941e3e791269).
> > > > > > > Has anyone also ran into it and knows if it's bug on test side or kernel?
> > > >
> > > > Hi Jan,
> > > >
> > > > I am traveling so I cannot work on solving the problem for the next week,
> > > > but I can explain the problem and offer suggestions for short term solutions
> > > > and maybe a longer term solution.
> > > >
> > > > > > >
> > > > > > > fanotify10.c:496: TINFO: Test #27: don't ignore fs events created
> > > > > > > inside a parent with evicted ignore mark
> > > > > > > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > > > > > > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > > > > > > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > > > > > > fanotify10.c:340: TFAIL: Unexpected inode mark (mflags=240,
> > > > > > > mask=8000020 ignored_mask=20)
> > > > > >
> > > > > > It is a test bug.
> > > > > > The problem is that we want to evict an inode, but there is no
> > > > > > reliable mechanism to do that.
> > > > > >
> > > > > > This is the reason for this workaround in fanotify23:
> > > > > >
> > > > > >         /* Shrinkers on other fs do not work reliably enough to
> > > > > > guarantee mark eviction on drop_caches */
> > > > > >         .dev_fs_type = "ext2",
> > > > > >
> > > > > > I did not encounter the problem with fanotify10 myself, but it should
> > > > > > be the same.
> > > > > > fanotify10 is not filesystem dependent, so if you can apply the same workaround
> > > > > > from fanotify23 and it works on your systems we can do that.
> > > > >
> > > > > Test is using default fs type, which should already be ext2.
> > > >
> > > > I see. You can try xfs on your system to see if it behaves better,
> > > > because xfs has a specialized inode shriker.
> > > >
> > > > > Here's a more complete log from failed test:
> > > > > https://s3.us-east-1.amazonaws.com/arr-cki-prod-datawarehouse-public/datawarehouse-public/2022/06/30/redhat:576928171/build_ppc64le_redhat:576928171_ppc64le/tests/1/results_0001/job.01/recipes/12221009/tasks/8/logs/syscalls.fail.log
> > > > >
> > > >
> > > > Are the failures only in the first test iteration?
> > >
> > > I think so. It runs in automated environment, where we see failures
> > > about once a week. I haven't managed to reproduce it by hand yet.
> > >
> > > >
> > > > As a long shot, I would try to remove mount_cycle() from setup().
> > > > I see that fanotify23 does not have it and it is not really needed, so
> > > > it may help
> > > > fanotify10 pass the first iteration.
> > > >
> > > > The problem is that the drop_caches knob is not a reliable way to evict inodes,
> > > > so when a test like fanotify10 needs to evict an inode, random factors are mixed
> > > > into the test run.
> > > >
> > > > As a quick band aid, I suggest to replace the TFAIL, "Unexpected inode mark"
> > > > in show_fanotify_marks() with TCONF, because the test failed to setup an
> > > > "evictable ignored mark that gets evicted", propagate a failure return
> > > > value from
> > > > create_fanotify_groups() => show_fanotify_marks() and skip the test case
> > > > instead of failing it.
> > >
> > > Thanks for suggestion.
> >
> > Is this still an issue? I didn't see anything happening in the fanotify10
> > test upstream. If the issue still happens, maybe something like the
> > attached patch may improve the situation? Jan, do you have a chance to test
> > it?
> >
> 
> Jan,
> 
> Did you notice that the test already does mount_cycle()
> after setup and that there is no modification of any file during the test?
> I don't see how syncfs() would make a difference.
> 
> Maybe it's an atime update of the executed file??
> I think at some point in the discussion it was said that the issue
> only happens on the first run of the test loop.
> 
> If it's atime update then syncfs() might help and adding
> noatime mount flags to the test could also fix it.

Yes, I was suspecting atime update because by default LTP mounts
filesystems with full atime support enabled. And I agree noatime should
solve that as well but syncfs() just seemed as more robust and
future-proof to me...

								Honza
Jan Stancek Aug. 25, 2022, 12:53 p.m. UTC | #10
On Wed, Aug 24, 2022 at 5:24 PM Jan Kara <jack@suse.cz> wrote:
>
> On Tue 12-07-22 10:19:19, Jan Stancek wrote:
> > On Sat, Jul 9, 2022 at 12:09 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > On Thu, Jul 7, 2022 at 3:49 PM Jan Stancek <jstancek@redhat.com> wrote:
> > > >
> > > > On Thu, Jun 30, 2022 at 10:21 AM Amir Goldstein <amir73il@gmail.com> wrote:
> > > > >
> > > > > On Thu, Jun 30, 2022 at 9:27 AM Jan Stancek <jstancek@redhat.com> wrote:
> > > > > >
> > > > > > On Mon, Jun 13, 2022 at 4:39 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > > > > > >
> > > > > > > Test multiple groups with evictable mark with ignore mask
> > > > > > >
> > > > > > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > > > > > > ---
> > > > > > >  .../kernel/syscalls/fanotify/fanotify10.c     | 78 +++++++++++++++++++
> > > > > > >  1 file changed, 78 insertions(+)
> > > > > > >
> > > > > > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > > > > > index b9a50672d..52277d0b7 100644
> > > > > > > --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > > > > > +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > > > > > > @@ -71,6 +71,7 @@ static char event_buf[EVENT_BUF_LEN];
> > > > > > >  static int exec_events_unsupported;
> > > > > > >  static int fan_report_dfid_unsupported;
> > > > > > >  static int filesystem_mark_unsupported;
> > > > > > > +static int evictable_mark_unsupported;
> > > > > > >
> > > > > > >  #define MOUNT_PATH "fs_mnt"
> > > > > > >  #define MNT2_PATH "mntpoint"
> > > > > > > @@ -90,6 +91,10 @@ static int filesystem_mark_unsupported;
> > > > > > >  #define FILE_EXEC_PATH2 MNT2_PATH"/"TEST_APP
> > > > > > >  #define FILE2_EXEC_PATH2 MNT2_PATH"/"TEST_APP2
> > > > > > >
> > > > > > > +#define DROP_CACHES_FILE "/proc/sys/vm/drop_caches"
> > > > > > > +#define CACHE_PRESSURE_FILE "/proc/sys/vm/vfs_cache_pressure"
> > > > > > > +
> > > > > > > +static int old_cache_pressure;
> > > > > > >  static pid_t child_pid;
> > > > > > >  static int bind_mount_created;
> > > > > > >  static unsigned int num_classes = NUM_CLASSES;
> > > > > > > @@ -98,12 +103,14 @@ enum {
> > > > > > >         FANOTIFY_INODE,
> > > > > > >         FANOTIFY_MOUNT,
> > > > > > >         FANOTIFY_FILESYSTEM,
> > > > > > > +       FANOTIFY_EVICTABLE,
> > > > > > >  };
> > > > > > >
> > > > > > >  static struct fanotify_mark_type fanotify_mark_types[] = {
> > > > > > >         INIT_FANOTIFY_MARK_TYPE(INODE),
> > > > > > >         INIT_FANOTIFY_MARK_TYPE(MOUNT),
> > > > > > >         INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
> > > > > > > +       INIT_FANOTIFY_MARK_TYPE(EVICTABLE),
> > > > > > >  };
> > > > > > >
> > > > > > >  static struct tcase {
> > > > > > > @@ -289,14 +296,59 @@ static struct tcase {
> > > > > > >                 0,
> > > > > > >                 FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > >         },
> > > > > > > +       /* Evictable ignore mark test cases */
> > > > > > > +       {
> > > > > > > +               "don't ignore mount events created on file with evicted ignore mark",
> > > > > > > +               MOUNT_PATH, FANOTIFY_MOUNT,
> > > > > > > +               FILE_PATH, FANOTIFY_EVICTABLE,
> > > > > > > +               0,
> > > > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > > +       },
> > > > > > > +       {
> > > > > > > +               "don't ignore fs events created on a file with evicted ignore mark",
> > > > > > > +               MOUNT_PATH, FANOTIFY_FILESYSTEM,
> > > > > > > +               FILE_PATH, FANOTIFY_EVICTABLE,
> > > > > > > +               0,
> > > > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > > +       },
> > > > > > > +       {
> > > > > > > +               "don't ignore mount events created inside a parent with evicted ignore mark",
> > > > > > > +               MOUNT_PATH, FANOTIFY_MOUNT,
> > > > > > > +               DIR_PATH, FANOTIFY_EVICTABLE,
> > > > > > > +               FAN_EVENT_ON_CHILD,
> > > > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > > +       },
> > > > > > > +       {
> > > > > > > +               "don't ignore fs events created inside a parent with evicted ignore mark",
> > > > > > > +               MOUNT_PATH, FANOTIFY_FILESYSTEM,
> > > > > > > +               DIR_PATH, FANOTIFY_EVICTABLE,
> > > > > > > +               FAN_EVENT_ON_CHILD,
> > > > > > > +               FILE_PATH, FAN_OPEN, FAN_OPEN
> > > > > > > +       },
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > we are seeing some sporadic failures from this last testcase, with
> > > > > > recent upstream kernels (v5.19-rc4-14-g941e3e791269).
> > > > > > Has anyone also ran into it and knows if it's bug on test side or kernel?
> > >
> > > Hi Jan,
> > >
> > > I am traveling so I cannot work on solving the problem for the next week,
> > > but I can explain the problem and offer suggestions for short term solutions
> > > and maybe a longer term solution.
> > >
> > > > > >
> > > > > > fanotify10.c:496: TINFO: Test #27: don't ignore fs events created
> > > > > > inside a parent with evicted ignore mark
> > > > > > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > > > > > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > > > > > fanotify10.c:338: TPASS: No fanotify inode marks as expected
> > > > > > fanotify10.c:340: TFAIL: Unexpected inode mark (mflags=240,
> > > > > > mask=8000020 ignored_mask=20)
> > > > >
> > > > > It is a test bug.
> > > > > The problem is that we want to evict an inode, but there is no
> > > > > reliable mechanism to do that.
> > > > >
> > > > > This is the reason for this workaround in fanotify23:
> > > > >
> > > > >         /* Shrinkers on other fs do not work reliably enough to
> > > > > guarantee mark eviction on drop_caches */
> > > > >         .dev_fs_type = "ext2",
> > > > >
> > > > > I did not encounter the problem with fanotify10 myself, but it should
> > > > > be the same.
> > > > > fanotify10 is not filesystem dependent, so if you can apply the same workaround
> > > > > from fanotify23 and it works on your systems we can do that.
> > > >
> > > > Test is using default fs type, which should already be ext2.
> > >
> > > I see. You can try xfs on your system to see if it behaves better,
> > > because xfs has a specialized inode shriker.
> > >
> > > > Here's a more complete log from failed test:
> > > > https://s3.us-east-1.amazonaws.com/arr-cki-prod-datawarehouse-public/datawarehouse-public/2022/06/30/redhat:576928171/build_ppc64le_redhat:576928171_ppc64le/tests/1/results_0001/job.01/recipes/12221009/tasks/8/logs/syscalls.fail.log
> > > >
> > >
> > > Are the failures only in the first test iteration?
> >
> > I think so. It runs in automated environment, where we see failures
> > about once a week. I haven't managed to reproduce it by hand yet.
> >
> > >
> > > As a long shot, I would try to remove mount_cycle() from setup().
> > > I see that fanotify23 does not have it and it is not really needed, so
> > > it may help
> > > fanotify10 pass the first iteration.
> > >
> > > The problem is that the drop_caches knob is not a reliable way to evict inodes,
> > > so when a test like fanotify10 needs to evict an inode, random factors are mixed
> > > into the test run.
> > >
> > > As a quick band aid, I suggest to replace the TFAIL, "Unexpected inode mark"
> > > in show_fanotify_marks() with TCONF, because the test failed to setup an
> > > "evictable ignored mark that gets evicted", propagate a failure return
> > > value from
> > > create_fanotify_groups() => show_fanotify_marks() and skip the test case
> > > instead of failing it.
> >
> > Thanks for suggestion.
>
> Is this still an issue?

It still rarely fails - two instances in last 3 weeks:
6.0.0-0.rc1.13.test.eln
https://s3.us-east-1.amazonaws.com/arr-cki-prod-datawarehouse-public/datawarehouse-public/2022/08/15/redhat:613661619/build_x86_64_redhat:613661619_x86_64/tests/3/results_0001/job.01/recipes/12445146/tasks/10/logs/syscalls.fail.log

5.20.0-0.rc0.aea23e7c464b
https://s3.us-east-1.amazonaws.com/arr-cki-prod-datawarehouse-public/datawarehouse-public/2022/08/14/redhat:613063904/build_x86_64_redhat:613063904_x86_64_debug/tests/2/results_0001/job.01/recipes/12440847/tasks/9/logs/syscalls.fail.log

> I didn't see anything happening in the fanotify10
> test upstream. If the issue still happens, maybe something like the
> attached patch may improve the situation? Jan, do you have a chance to test
> it?

I can't reproduce failure on demand. If the patch doesn't have any
side-effects, then let's apply it, and we'll see in couple weeks.

Regards,
Jan

>
>                                                                 Honza
> --
> Jan Kara <jack@suse.com>
> SUSE Labs, CR
Amir Goldstein Aug. 25, 2022, 1:47 p.m. UTC | #11
> > Is this still an issue?
>
> It still rarely fails - two instances in last 3 weeks:
> 6.0.0-0.rc1.13.test.eln
> https://s3.us-east-1.amazonaws.com/arr-cki-prod-datawarehouse-public/datawarehouse-public/2022/08/15/redhat:613661619/build_x86_64_redhat:613661619_x86_64/tests/3/results_0001/job.01/recipes/12445146/tasks/10/logs/syscalls.fail.log
>
> 5.20.0-0.rc0.aea23e7c464b
> https://s3.us-east-1.amazonaws.com/arr-cki-prod-datawarehouse-public/datawarehouse-public/2022/08/14/redhat:613063904/build_x86_64_redhat:613063904_x86_64_debug/tests/2/results_0001/job.01/recipes/12440847/tasks/9/logs/syscalls.fail.log
>
> > I didn't see anything happening in the fanotify10
> > test upstream. If the issue still happens, maybe something like the
> > attached patch may improve the situation? Jan, do you have a chance to test
> > it?
>
> I can't reproduce failure on demand. If the patch doesn't have any
> side-effects, then let's apply it, and we'll see in couple weeks.
>

Yes, let's do that please.

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

Thanks,
Amir.
Jan Kara Aug. 25, 2022, 2:03 p.m. UTC | #12
On Thu 25-08-22 16:47:53, Amir Goldstein wrote:
> > > Is this still an issue?
> >
> > It still rarely fails - two instances in last 3 weeks:
> > 6.0.0-0.rc1.13.test.eln
> > https://s3.us-east-1.amazonaws.com/arr-cki-prod-datawarehouse-public/datawarehouse-public/2022/08/15/redhat:613661619/build_x86_64_redhat:613661619_x86_64/tests/3/results_0001/job.01/recipes/12445146/tasks/10/logs/syscalls.fail.log
> >
> > 5.20.0-0.rc0.aea23e7c464b
> > https://s3.us-east-1.amazonaws.com/arr-cki-prod-datawarehouse-public/datawarehouse-public/2022/08/14/redhat:613063904/build_x86_64_redhat:613063904_x86_64_debug/tests/2/results_0001/job.01/recipes/12440847/tasks/9/logs/syscalls.fail.log
> >
> > > I didn't see anything happening in the fanotify10
> > > test upstream. If the issue still happens, maybe something like the
> > > attached patch may improve the situation? Jan, do you have a chance to test
> > > it?
> >
> > I can't reproduce failure on demand. If the patch doesn't have any
> > side-effects, then let's apply it, and we'll see in couple weeks.
> >
> 
> Yes, let's do that please.
> 
> Acked-by: Amir Goldstein <amir73il@gmail.com>

Agreed. Done official submission.

								Honza
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
index b9a50672d..52277d0b7 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify10.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
@@ -71,6 +71,7 @@  static char event_buf[EVENT_BUF_LEN];
 static int exec_events_unsupported;
 static int fan_report_dfid_unsupported;
 static int filesystem_mark_unsupported;
+static int evictable_mark_unsupported;
 
 #define MOUNT_PATH "fs_mnt"
 #define MNT2_PATH "mntpoint"
@@ -90,6 +91,10 @@  static int filesystem_mark_unsupported;
 #define FILE_EXEC_PATH2 MNT2_PATH"/"TEST_APP
 #define FILE2_EXEC_PATH2 MNT2_PATH"/"TEST_APP2
 
+#define DROP_CACHES_FILE "/proc/sys/vm/drop_caches"
+#define CACHE_PRESSURE_FILE "/proc/sys/vm/vfs_cache_pressure"
+
+static int old_cache_pressure;
 static pid_t child_pid;
 static int bind_mount_created;
 static unsigned int num_classes = NUM_CLASSES;
@@ -98,12 +103,14 @@  enum {
 	FANOTIFY_INODE,
 	FANOTIFY_MOUNT,
 	FANOTIFY_FILESYSTEM,
+	FANOTIFY_EVICTABLE,
 };
 
 static struct fanotify_mark_type fanotify_mark_types[] = {
 	INIT_FANOTIFY_MARK_TYPE(INODE),
 	INIT_FANOTIFY_MARK_TYPE(MOUNT),
 	INIT_FANOTIFY_MARK_TYPE(FILESYSTEM),
+	INIT_FANOTIFY_MARK_TYPE(EVICTABLE),
 };
 
 static struct tcase {
@@ -289,14 +296,59 @@  static struct tcase {
 		0,
 		FILE_PATH, FAN_OPEN, FAN_OPEN
 	},
+	/* Evictable ignore mark test cases */
+	{
+		"don't ignore mount events created on file with evicted ignore mark",
+		MOUNT_PATH, FANOTIFY_MOUNT,
+		FILE_PATH, FANOTIFY_EVICTABLE,
+		0,
+		FILE_PATH, FAN_OPEN, FAN_OPEN
+	},
+	{
+		"don't ignore fs events created on a file with evicted ignore mark",
+		MOUNT_PATH, FANOTIFY_FILESYSTEM,
+		FILE_PATH, FANOTIFY_EVICTABLE,
+		0,
+		FILE_PATH, FAN_OPEN, FAN_OPEN
+	},
+	{
+		"don't ignore mount events created inside a parent with evicted ignore mark",
+		MOUNT_PATH, FANOTIFY_MOUNT,
+		DIR_PATH, FANOTIFY_EVICTABLE,
+		FAN_EVENT_ON_CHILD,
+		FILE_PATH, FAN_OPEN, FAN_OPEN
+	},
+	{
+		"don't ignore fs events created inside a parent with evicted ignore mark",
+		MOUNT_PATH, FANOTIFY_FILESYSTEM,
+		DIR_PATH, FANOTIFY_EVICTABLE,
+		FAN_EVENT_ON_CHILD,
+		FILE_PATH, FAN_OPEN, FAN_OPEN
+	},
 };
 
+static void show_fanotify_marks(int fd)
+{
+	unsigned int mflags, mask, ignored_mask;
+	char procfdinfo[100];
+
+	sprintf(procfdinfo, "/proc/%d/fdinfo/%d", (int)getpid(), fd);
+	if (FILE_LINES_SCANF(procfdinfo, "fanotify ino:%*x sdev:%*x mflags: %x mask:%x ignored_mask:%x",
+				&mflags, &mask, &ignored_mask)) {
+		tst_res(TPASS, "No fanotify inode marks as expected");
+	} else {
+		tst_res(TFAIL, "Unexpected inode mark (mflags=%x, mask=%x ignored_mask=%x)",
+				mflags, mask, ignored_mask);
+	}
+}
+
 static int create_fanotify_groups(unsigned int n)
 {
 	struct tcase *tc = &tcases[n];
 	struct fanotify_mark_type *mark, *ignore_mark;
 	unsigned int mark_ignored, mask;
 	unsigned int p, i;
+	int evictable_ignored = (tc->ignore_mark_type == FANOTIFY_EVICTABLE);
 
 	mark = &fanotify_mark_types[tc->mark_type];
 	ignore_mark = &fanotify_mark_types[tc->ignore_mark_type];
@@ -345,6 +397,20 @@  add_mark:
 			}
 		}
 	}
+
+	/*
+	 * drop_caches should evict inode from cache and remove evictable marks
+	 */
+	if (evictable_ignored) {
+		SAFE_FILE_PRINTF(DROP_CACHES_FILE, "3");
+		for (p = 0; p < num_classes; p++) {
+			for (i = 0; i < GROUPS_PER_PRIO; i++) {
+				if (fd_notify[p][i] > 0)
+					show_fanotify_marks(fd_notify[p][i]);
+			}
+		}
+	}
+
 	return 0;
 }
 
@@ -439,6 +505,11 @@  static void test_fanotify(unsigned int n)
 		return;
 	}
 
+	if (evictable_mark_unsupported && tc->ignore_mark_type == FANOTIFY_EVICTABLE) {
+		tst_res(TCONF, "FAN_MARK_EVICTABLE not supported in kernel?");
+		return;
+	}
+
 	if (tc->ignored_onchild && tst_kvercmp(5, 9, 0) < 0) {
 		tst_res(TCONF, "ignored mask in combination with flag FAN_EVENT_ON_CHILD"
 				" has undefined behavior on kernel < 5.9");
@@ -527,6 +598,7 @@  static void setup(void)
 	exec_events_unsupported = fanotify_events_supported_by_kernel(FAN_OPEN_EXEC,
 								      FAN_CLASS_CONTENT, 0);
 	filesystem_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_FILESYSTEM);
+	evictable_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_EVICTABLE);
 	fan_report_dfid_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_DFID_NAME,
 									  MOUNT_PATH);
 	if (fan_report_dfid_unsupported) {
@@ -545,6 +617,10 @@  static void setup(void)
 	/* Create another bind mount at another path for generating events */
 	SAFE_MKDIR(MNT2_PATH, 0755);
 	mount_cycle();
+
+	SAFE_FILE_SCANF(CACHE_PRESSURE_FILE, "%d", &old_cache_pressure);
+	/* Set high priority for evicting inodes */
+	SAFE_FILE_PRINTF(CACHE_PRESSURE_FILE, "500");
 }
 
 static void cleanup(void)
@@ -553,6 +629,8 @@  static void cleanup(void)
 
 	if (bind_mount_created)
 		SAFE_UMOUNT(MNT2_PATH);
+
+	SAFE_FILE_PRINTF(CACHE_PRESSURE_FILE, "%d", old_cache_pressure);
 }
 
 static const char *const resource_files[] = {