* [LTP] [PATCH 0/3] Make fanotify10 test yet more reliable
@ 2022-11-15 12:47 Jan Kara
2022-11-15 12:47 ` [LTP] [PATCH 1/3] fanotify10: Use named initializers Jan Kara
` (2 more replies)
0 siblings, 3 replies; 24+ messages in thread
From: Jan Kara @ 2022-11-15 12:47 UTC (permalink / raw)
To: ltp; +Cc: Jan Kara
Hello!
I was debugging with Pengfei Xu why fanotify10 testcase still occasionally
fails in his test setup. After a lot of back and forth we have identified two
causes. One lies within the kernel slab reclaim itself (fix submitted), the
other one is the inherent problem that slab reclaim needs to first reclaim
dentries (which means going through round of LRU aging before dentry is
reclaimed) and then inodes have to go through LRU aging before they are
reclaimed. As a result code dropping slab caches can decide there's not enough
forward progress and stop before the inodes we are interested in are evicted.
This patch modifies fanotify10 testcase to create multiple files / dirs with
ignore marks and return success if at least half of ignore marks got reclaimed.
This both gives slab reclaim code better feel of forward progress as well as
provides some robustness against some inode not being reclaimed for some random
reason.
With the kernel fix and this modification to fanotify10 testcase, Xu cannot
trigger the failure anymore.
Honza
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* [LTP] [PATCH 1/3] fanotify10: Use named initializers
2022-11-15 12:47 [LTP] [PATCH 0/3] Make fanotify10 test yet more reliable Jan Kara
@ 2022-11-15 12:47 ` Jan Kara
2022-11-15 12:47 ` [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files Jan Kara
2022-11-15 12:47 ` [LTP] [PATCH 3/3] fanotify10: Make evictable marks tests more reliable Jan Kara
2 siblings, 0 replies; 24+ messages in thread
From: Jan Kara @ 2022-11-15 12:47 UTC (permalink / raw)
To: ltp; +Cc: Jan Kara
Convert the testcase to use named initializers instead of anonymous
ones.
Signed-off-by: Jan Kara <jack@suse.cz>
---
.../kernel/syscalls/fanotify/fanotify10.c | 408 +++++++++++-------
1 file changed, 249 insertions(+), 159 deletions(-)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
index ca0d8a9e4a4a..6f21094bed2c 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify10.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
@@ -133,227 +133,317 @@ static struct tcase {
unsigned long long expected_mask_without_ignore;
} tcases[] = {
{
- "ignore mount events created on a specific file",
- MOUNT_PATH, FANOTIFY_MOUNT,
- FILE_MNT2, FANOTIFY_INODE,
- 0,
- FILE_PATH, 0, FAN_OPEN
+ .tname = "ignore mount events created on a specific file",
+ .mark_path = MOUNT_PATH,
+ .mark_type = FANOTIFY_MOUNT,
+ .ignore_path = FILE_MNT2,
+ .ignore_mark_type = FANOTIFY_INODE,
+ .event_path = FILE_PATH,
+ .expected_mask_without_ignore = FAN_OPEN
},
{
- "ignore exec mount events created on a specific file",
- MOUNT_PATH, FANOTIFY_MOUNT,
- FILE_EXEC_PATH2, FANOTIFY_INODE,
- 0,
- FILE_EXEC_PATH, FAN_OPEN_EXEC, FAN_OPEN | FAN_OPEN_EXEC
+ .tname = "ignore exec mount events created on a specific file",
+ .mark_path = MOUNT_PATH,
+ .mark_type = FANOTIFY_MOUNT,
+ .ignore_path = FILE_EXEC_PATH2,
+ .ignore_mark_type = FANOTIFY_INODE,
+ .event_path = FILE_EXEC_PATH,
+ .expected_mask_with_ignore = FAN_OPEN_EXEC,
+ .expected_mask_without_ignore = FAN_OPEN | FAN_OPEN_EXEC
},
{
- "don't ignore mount events created on another file",
- MOUNT_PATH, FANOTIFY_MOUNT,
- FILE_PATH, FANOTIFY_INODE,
- 0,
- FILE2_PATH, FAN_OPEN, FAN_OPEN
+ .tname = "don't ignore mount events created on another file",
+ .mark_path = MOUNT_PATH,
+ .mark_type = FANOTIFY_MOUNT,
+ .ignore_path = FILE_PATH,
+ .ignore_mark_type = FANOTIFY_INODE,
+ .event_path = FILE2_PATH,
+ .expected_mask_with_ignore = FAN_OPEN,
+ .expected_mask_without_ignore = FAN_OPEN
},
{
- "don't ignore exec mount events created on another file",
- MOUNT_PATH, FANOTIFY_MOUNT,
- FILE_EXEC_PATH, FANOTIFY_INODE,
- 0,
- FILE2_EXEC_PATH, FAN_OPEN | FAN_OPEN_EXEC,
- FAN_OPEN | FAN_OPEN_EXEC
+ .tname = "don't ignore exec mount events created on another file",
+ .mark_path = MOUNT_PATH,
+ .mark_type = FANOTIFY_MOUNT,
+ .ignore_path = FILE_EXEC_PATH,
+ .ignore_mark_type = FANOTIFY_INODE,
+ .event_path = FILE2_EXEC_PATH,
+ .expected_mask_with_ignore = FAN_OPEN | FAN_OPEN_EXEC,
+ .expected_mask_without_ignore = FAN_OPEN | FAN_OPEN_EXEC
},
{
- "ignore inode events created on a specific mount point",
- FILE_PATH, FANOTIFY_INODE,
- MNT2_PATH, FANOTIFY_MOUNT,
- 0,
- FILE_MNT2, 0, FAN_OPEN
+ .tname = "ignore inode events created on a specific mount point",
+ .mark_path = FILE_PATH,
+ .mark_type = FANOTIFY_INODE,
+ .ignore_path = MNT2_PATH,
+ .ignore_mark_type = FANOTIFY_MOUNT,
+ .event_path = FILE_MNT2,
+ .expected_mask_without_ignore = FAN_OPEN
},
{
- "ignore exec inode events created on a specific mount point",
- FILE_EXEC_PATH, FANOTIFY_INODE,
- MNT2_PATH, FANOTIFY_MOUNT,
- 0,
- FILE_EXEC_PATH2, FAN_OPEN_EXEC, FAN_OPEN | FAN_OPEN_EXEC
+ .tname = "ignore exec inode events created on a specific mount point",
+ .mark_path = FILE_EXEC_PATH,
+ .mark_type = FANOTIFY_INODE,
+ .ignore_path = MNT2_PATH,
+ .ignore_mark_type = FANOTIFY_MOUNT,
+ .event_path = FILE_EXEC_PATH2,
+ .expected_mask_with_ignore = FAN_OPEN_EXEC,
+ .expected_mask_without_ignore = FAN_OPEN | FAN_OPEN_EXEC
},
{
- "don't ignore inode events created on another mount point",
- FILE_MNT2, FANOTIFY_INODE,
- MNT2_PATH, FANOTIFY_MOUNT,
- 0,
- FILE_PATH, FAN_OPEN, FAN_OPEN
+ .tname = "don't ignore inode events created on another mount point",
+ .mark_path = FILE_MNT2,
+ .mark_type = FANOTIFY_INODE,
+ .ignore_path = MNT2_PATH,
+ .ignore_mark_type = FANOTIFY_MOUNT,
+ .event_path = FILE_PATH,
+ .expected_mask_with_ignore = FAN_OPEN,
+ .expected_mask_without_ignore = FAN_OPEN
},
{
- "don't ignore exec inode events created on another mount point",
- FILE_EXEC_PATH2, FANOTIFY_INODE,
- MNT2_PATH, FANOTIFY_MOUNT,
- 0,
- FILE_EXEC_PATH, FAN_OPEN | FAN_OPEN_EXEC,
- FAN_OPEN | FAN_OPEN_EXEC
+ .tname = "don't ignore exec inode events created on another mount point",
+ .mark_path = FILE_EXEC_PATH2,
+ .mark_type = FANOTIFY_INODE,
+ .ignore_path = MNT2_PATH,
+ .ignore_mark_type = FANOTIFY_MOUNT,
+ .event_path = FILE_EXEC_PATH,
+ .expected_mask_with_ignore = FAN_OPEN | FAN_OPEN_EXEC,
+ .expected_mask_without_ignore = FAN_OPEN | FAN_OPEN_EXEC
},
{
- "ignore fs events created on a specific file",
- MOUNT_PATH, FANOTIFY_FILESYSTEM,
- FILE_PATH, FANOTIFY_INODE,
- 0,
- FILE_PATH, 0, FAN_OPEN
+ .tname = "ignore fs events created on a specific file",
+ .mark_path = MOUNT_PATH,
+ .mark_type = FANOTIFY_FILESYSTEM,
+ .ignore_path = FILE_PATH,
+ .ignore_mark_type = FANOTIFY_INODE,
+ .event_path = FILE_PATH,
+ .expected_mask_without_ignore = FAN_OPEN
},
{
- "ignore exec fs events created on a specific file",
- MOUNT_PATH, FANOTIFY_FILESYSTEM,
- FILE_EXEC_PATH, FANOTIFY_INODE,
- 0,
- FILE_EXEC_PATH, FAN_OPEN_EXEC, FAN_OPEN | FAN_OPEN_EXEC
+ .tname = "ignore exec fs events created on a specific file",
+ .mark_path = MOUNT_PATH,
+ .mark_type = FANOTIFY_FILESYSTEM,
+ .ignore_path = FILE_EXEC_PATH,
+ .ignore_mark_type = FANOTIFY_INODE,
+ .event_path = FILE_EXEC_PATH,
+ .expected_mask_with_ignore = FAN_OPEN_EXEC,
+ .expected_mask_without_ignore = FAN_OPEN | FAN_OPEN_EXEC
},
{
- "don't ignore mount events created on another file",
- MOUNT_PATH, FANOTIFY_FILESYSTEM,
- FILE_PATH, FANOTIFY_INODE,
- 0,
- FILE2_PATH, FAN_OPEN, FAN_OPEN
+ .tname = "don't ignore mount events created on another file",
+ .mark_path = MOUNT_PATH,
+ .mark_type = FANOTIFY_FILESYSTEM,
+ .ignore_path = FILE_PATH,
+ .ignore_mark_type = FANOTIFY_INODE,
+ .event_path = FILE2_PATH,
+ .expected_mask_with_ignore = FAN_OPEN,
+ .expected_mask_without_ignore = FAN_OPEN
},
{
- "don't ignore exec mount events created on another file",
- MOUNT_PATH, FANOTIFY_FILESYSTEM,
- FILE_EXEC_PATH, FANOTIFY_INODE,
- 0,
- FILE2_EXEC_PATH, FAN_OPEN | FAN_OPEN_EXEC,
- FAN_OPEN | FAN_OPEN_EXEC
+ .tname = "don't ignore exec mount events created on another file",
+ .mark_path = MOUNT_PATH,
+ .mark_type = FANOTIFY_FILESYSTEM,
+ .ignore_path = FILE_EXEC_PATH,
+ .ignore_mark_type = FANOTIFY_INODE,
+ .event_path = FILE2_EXEC_PATH,
+ .expected_mask_with_ignore = FAN_OPEN | FAN_OPEN_EXEC,
+ .expected_mask_without_ignore = FAN_OPEN | FAN_OPEN_EXEC
},
{
- "ignore fs events created on a specific mount point",
- MOUNT_PATH, FANOTIFY_FILESYSTEM,
- MNT2_PATH, FANOTIFY_MOUNT,
- 0,
- FILE_MNT2, 0, FAN_OPEN
+ .tname = "ignore fs events created on a specific mount point",
+ .mark_path = MOUNT_PATH,
+ .mark_type = FANOTIFY_FILESYSTEM,
+ .ignore_path = MNT2_PATH,
+ .ignore_mark_type = FANOTIFY_MOUNT,
+ .event_path = FILE_MNT2,
+ .expected_mask_without_ignore = FAN_OPEN
},
{
- "ignore exec fs events created on a specific mount point",
- MOUNT_PATH, FANOTIFY_FILESYSTEM,
- MNT2_PATH, FANOTIFY_MOUNT,
- 0,
- FILE_EXEC_PATH2, FAN_OPEN_EXEC, FAN_OPEN | FAN_OPEN_EXEC
+ .tname = "ignore exec fs events created on a specific mount point",
+ .mark_path = MOUNT_PATH,
+ .mark_type = FANOTIFY_FILESYSTEM,
+ .ignore_path = MNT2_PATH,
+ .ignore_mark_type = FANOTIFY_MOUNT,
+ .event_path = FILE_EXEC_PATH2,
+ .expected_mask_with_ignore = FAN_OPEN_EXEC,
+ .expected_mask_without_ignore = FAN_OPEN | FAN_OPEN_EXEC
},
{
- "don't ignore fs events created on another mount point",
- MOUNT_PATH, FANOTIFY_FILESYSTEM,
- MNT2_PATH, FANOTIFY_MOUNT,
- 0,
- FILE_PATH, FAN_OPEN, FAN_OPEN
+ .tname = "don't ignore fs events created on another mount point",
+ .mark_path = MOUNT_PATH,
+ .mark_type = FANOTIFY_FILESYSTEM,
+ .ignore_path = MNT2_PATH,
+ .ignore_mark_type = FANOTIFY_MOUNT,
+ .event_path = FILE_PATH,
+ .expected_mask_with_ignore = FAN_OPEN,
+ .expected_mask_without_ignore = FAN_OPEN
},
{
- "don't ignore exec fs events created on another mount point",
- MOUNT_PATH, FANOTIFY_FILESYSTEM,
- MNT2_PATH, FANOTIFY_MOUNT,
- 0,
- FILE_EXEC_PATH, FAN_OPEN | FAN_OPEN_EXEC,
- FAN_OPEN | FAN_OPEN_EXEC
+ .tname = "don't ignore exec fs events created on another mount point",
+ .mark_path = MOUNT_PATH,
+ .mark_type = FANOTIFY_FILESYSTEM,
+ .ignore_path = MNT2_PATH,
+ .ignore_mark_type = FANOTIFY_MOUNT,
+ .event_path = FILE_EXEC_PATH,
+ .expected_mask_with_ignore = FAN_OPEN | FAN_OPEN_EXEC,
+ .expected_mask_without_ignore = FAN_OPEN | FAN_OPEN_EXEC
},
{
- "ignore child exec events created on a specific mount point",
- MOUNT_PATH, FANOTIFY_PARENT,
- MOUNT_PATH, FANOTIFY_MOUNT,
- 0,
- FILE_EXEC_PATH, FAN_OPEN_EXEC, FAN_OPEN | FAN_OPEN_EXEC
+ .tname = "ignore child exec events created on a specific mount point",
+ .mark_path = MOUNT_PATH,
+ .mark_type = FANOTIFY_PARENT,
+ .ignore_path = MOUNT_PATH,
+ .ignore_mark_type = FANOTIFY_MOUNT,
+ .event_path = FILE_EXEC_PATH,
+ .expected_mask_with_ignore = FAN_OPEN_EXEC,
+ .expected_mask_without_ignore = FAN_OPEN | FAN_OPEN_EXEC
},
{
- "ignore events on children of directory created on a specific file",
- DIR_PATH, FANOTIFY_PARENT,
- DIR_PATH, FANOTIFY_PARENT,
- FAN_EVENT_ON_CHILD,
- FILE_PATH, 0, FAN_OPEN
+ .tname = "ignore events on children of directory created on a specific file",
+ .mark_path = DIR_PATH,
+ .mark_type = FANOTIFY_PARENT,
+ .ignore_path = DIR_PATH,
+ .ignore_mark_type = FANOTIFY_PARENT,
+ .ignored_flags = FAN_EVENT_ON_CHILD,
+ .event_path = FILE_PATH,
+ .expected_mask_without_ignore = FAN_OPEN
},
{
- "ignore events on file created inside a parent watching children",
- FILE_PATH, FANOTIFY_INODE,
- DIR_PATH, FANOTIFY_PARENT,
- FAN_EVENT_ON_CHILD,
- FILE_PATH, 0, FAN_OPEN
+ .tname = "ignore events on file created inside a parent watching children",
+ .mark_path = FILE_PATH,
+ .mark_type = FANOTIFY_INODE,
+ .ignore_path = DIR_PATH,
+ .ignore_mark_type = FANOTIFY_PARENT,
+ .ignored_flags = FAN_EVENT_ON_CHILD,
+ .event_path = FILE_PATH,
+ .expected_mask_without_ignore = FAN_OPEN
},
{
- "don't ignore events on file created inside a parent not watching children",
- FILE_PATH, FANOTIFY_INODE,
- DIR_PATH, FANOTIFY_PARENT,
- 0,
- FILE_PATH, FAN_OPEN, FAN_OPEN
+ .tname = "don't ignore events on file created inside a parent not watching children",
+ .mark_path = FILE_PATH,
+ .mark_type = FANOTIFY_INODE,
+ .ignore_path = DIR_PATH,
+ .ignore_mark_type = FANOTIFY_PARENT,
+ .event_path = FILE_PATH,
+ .expected_mask_with_ignore = FAN_OPEN,
+ .expected_mask_without_ignore = FAN_OPEN
},
{
- "ignore mount events created inside a parent watching children",
- FILE_PATH, FANOTIFY_MOUNT,
- DIR_PATH, FANOTIFY_PARENT,
- FAN_EVENT_ON_CHILD,
- FILE_PATH, 0, FAN_OPEN
+ .tname = "ignore mount events created inside a parent watching children",
+ .mark_path = FILE_PATH,
+ .mark_type = FANOTIFY_MOUNT,
+ .ignore_path = DIR_PATH,
+ .ignore_mark_type = FANOTIFY_PARENT,
+ .ignored_flags = FAN_EVENT_ON_CHILD,
+ .event_path = FILE_PATH,
+ .expected_mask_without_ignore = FAN_OPEN
},
{
- "don't ignore mount events created inside a parent not watching children",
- FILE_PATH, FANOTIFY_MOUNT,
- DIR_PATH, FANOTIFY_PARENT,
- 0,
- FILE_PATH, FAN_OPEN, FAN_OPEN
+ .tname = "don't ignore mount events created inside a parent not watching children",
+ .mark_path = FILE_PATH,
+ .mark_type = FANOTIFY_MOUNT,
+ .ignore_path = DIR_PATH,
+ .ignore_mark_type = FANOTIFY_PARENT,
+ .event_path = FILE_PATH,
+ .expected_mask_with_ignore = FAN_OPEN,
+ .expected_mask_without_ignore = FAN_OPEN
},
{
- "ignore fs events created inside a parent watching children",
- FILE_PATH, FANOTIFY_FILESYSTEM,
- DIR_PATH, FANOTIFY_PARENT,
- FAN_EVENT_ON_CHILD,
- FILE_PATH, 0, FAN_OPEN
+ .tname = "ignore fs events created inside a parent watching children",
+ .mark_path = FILE_PATH,
+ .mark_type = FANOTIFY_FILESYSTEM,
+ .ignore_path = DIR_PATH,
+ .ignore_mark_type = FANOTIFY_PARENT,
+ .ignored_flags = FAN_EVENT_ON_CHILD,
+ .event_path = FILE_PATH,
+ .expected_mask_without_ignore = FAN_OPEN
},
{
- "don't ignore fs events created inside a parent not watching children",
- FILE_PATH, FANOTIFY_FILESYSTEM,
- DIR_PATH, FANOTIFY_PARENT,
- 0,
- FILE_PATH, FAN_OPEN, FAN_OPEN
+ .tname = "don't ignore fs events created inside a parent not watching children",
+ .mark_path = FILE_PATH,
+ .mark_type = FANOTIFY_FILESYSTEM,
+ .ignore_path = DIR_PATH,
+ .ignore_mark_type = FANOTIFY_PARENT,
+ .event_path = FILE_PATH,
+ .expected_mask_with_ignore = FAN_OPEN,
+ .expected_mask_without_ignore = 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
+ .tname = "don't ignore mount events created on file with evicted ignore mark",
+ .mark_path = MOUNT_PATH,
+ .mark_type = FANOTIFY_MOUNT,
+ .ignore_path = FILE_PATH,
+ .ignore_mark_type = FANOTIFY_EVICTABLE,
+ .event_path = FILE_PATH,
+ .expected_mask_with_ignore = FAN_OPEN,
+ .expected_mask_without_ignore = 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
+ .tname = "don't ignore fs events created on a file with evicted ignore mark",
+ .mark_path = MOUNT_PATH,
+ .mark_type = FANOTIFY_FILESYSTEM,
+ .ignore_path = FILE_PATH,
+ .ignore_mark_type = FANOTIFY_EVICTABLE,
+ .event_path = FILE_PATH,
+ .expected_mask_with_ignore = FAN_OPEN,
+ .expected_mask_without_ignore = 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
+ .tname = "don't ignore mount events created inside a parent with evicted ignore mark",
+ .mark_path = MOUNT_PATH,
+ .mark_type = FANOTIFY_MOUNT,
+ .ignore_path = DIR_PATH,
+ .ignore_mark_type = FANOTIFY_EVICTABLE,
+ .ignored_flags = FAN_EVENT_ON_CHILD,
+ .event_path = FILE_PATH,
+ .expected_mask_with_ignore = FAN_OPEN,
+ .expected_mask_without_ignore = 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
+ .tname = "don't ignore fs events created inside a parent with evicted ignore mark",
+ .mark_path = MOUNT_PATH,
+ .mark_type = FANOTIFY_FILESYSTEM,
+ .ignore_path = DIR_PATH,
+ .ignore_mark_type = FANOTIFY_EVICTABLE,
+ .ignored_flags = FAN_EVENT_ON_CHILD,
+ .event_path = FILE_PATH,
+ .expected_mask_with_ignore = FAN_OPEN,
+ .expected_mask_without_ignore = FAN_OPEN
},
/* FAN_MARK_IGNORE specific test cases */
{
- "ignore events on subdir inside a parent watching subdirs",
- SUBDIR_PATH, FANOTIFY_SUBDIR,
- DIR_PATH, FANOTIFY_PARENT,
- FAN_EVENT_ON_CHILD | FAN_ONDIR,
- SUBDIR_PATH, 0, FAN_OPEN | FAN_ONDIR
+ .tname = "ignore events on subdir inside a parent watching subdirs",
+ .mark_path = SUBDIR_PATH,
+ .mark_type = FANOTIFY_SUBDIR,
+ .ignore_path = DIR_PATH,
+ .ignore_mark_type = FANOTIFY_PARENT,
+ .ignored_flags = FAN_EVENT_ON_CHILD | FAN_ONDIR,
+ .event_path = SUBDIR_PATH,
+ .expected_mask_with_ignore = 0,
+ .expected_mask_without_ignore = FAN_OPEN | FAN_ONDIR
},
{
- "don't ignore events on subdir inside a parent not watching children",
- SUBDIR_PATH, FANOTIFY_SUBDIR,
- DIR_PATH, FANOTIFY_PARENT,
- FAN_ONDIR,
- SUBDIR_PATH, FAN_OPEN | FAN_ONDIR, FAN_OPEN | FAN_ONDIR
+ .tname = "don't ignore events on subdir inside a parent not watching children",
+ .mark_path = SUBDIR_PATH,
+ .mark_type = FANOTIFY_SUBDIR,
+ .ignore_path = DIR_PATH,
+ .ignore_mark_type = FANOTIFY_PARENT,
+ .ignored_flags = FAN_ONDIR,
+ .event_path = SUBDIR_PATH,
+ .expected_mask_with_ignore = FAN_OPEN | FAN_ONDIR,
+ .expected_mask_without_ignore = FAN_OPEN | FAN_ONDIR
},
{
- "don't ignore events on subdir inside a parent watching non-dir children",
- SUBDIR_PATH, FANOTIFY_SUBDIR,
- DIR_PATH, FANOTIFY_PARENT,
- FAN_EVENT_ON_CHILD,
- SUBDIR_PATH, FAN_OPEN | FAN_ONDIR, FAN_OPEN | FAN_ONDIR
+ .tname = "don't ignore events on subdir inside a parent watching non-dir children",
+ .mark_path = SUBDIR_PATH,
+ .mark_type = FANOTIFY_SUBDIR,
+ .ignore_path = DIR_PATH,
+ .ignore_mark_type = FANOTIFY_PARENT,
+ .ignored_flags = FAN_EVENT_ON_CHILD,
+ .event_path = SUBDIR_PATH,
+ .expected_mask_with_ignore = FAN_OPEN | FAN_ONDIR,
+ .expected_mask_without_ignore = FAN_OPEN | FAN_ONDIR
},
};
--
2.35.3
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files
2022-11-15 12:47 [LTP] [PATCH 0/3] Make fanotify10 test yet more reliable Jan Kara
2022-11-15 12:47 ` [LTP] [PATCH 1/3] fanotify10: Use named initializers Jan Kara
@ 2022-11-15 12:47 ` Jan Kara
2022-11-17 15:58 ` Petr Vorel
2022-11-21 15:04 ` Cyril Hrubis
2022-11-15 12:47 ` [LTP] [PATCH 3/3] fanotify10: Make evictable marks tests more reliable Jan Kara
2 siblings, 2 replies; 24+ messages in thread
From: Jan Kara @ 2022-11-15 12:47 UTC (permalink / raw)
To: ltp; +Cc: Jan Kara
Add support for marking and generating events on multiple files. We'll
use this for more reliable checking of evictable marks.
Signed-off-by: Jan Kara <jack@suse.cz>
---
.../kernel/syscalls/fanotify/fanotify10.c | 417 ++++++++++--------
1 file changed, 245 insertions(+), 172 deletions(-)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
index 6f21094bed2c..e19bd907470f 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify10.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
@@ -43,6 +43,7 @@
#include <sys/mount.h>
#include <sys/syscall.h>
#include "tst_test.h"
+#include "tst_safe_stdio.h"
#ifdef HAVE_SYS_FANOTIFY_H
#include "fanotify.h"
@@ -69,6 +70,7 @@ static int fd_notify[NUM_CLASSES][GROUPS_PER_PRIO];
static int fd_syncfs;
static char event_buf[EVENT_BUF_LEN];
+static int event_buf_pos, event_buf_len;
static int exec_events_unsupported;
static int fan_report_dfid_unsupported;
static int filesystem_mark_unsupported;
@@ -123,346 +125,377 @@ static struct fanotify_mark_type fanotify_mark_types[] = {
static struct tcase {
const char *tname;
- const char *mark_path;
+ int mark_path_cnt;
+ const char *mark_path_fmt;
int mark_type;
- const char *ignore_path;
+ int ignore_path_cnt;
+ const char *ignore_path_fmt;
int ignore_mark_type;
unsigned int ignored_flags;
- const char *event_path;
+ int event_path_cnt;
+ const char *event_path_fmt;
unsigned long long expected_mask_with_ignore;
unsigned long long expected_mask_without_ignore;
} tcases[] = {
{
.tname = "ignore mount events created on a specific file",
- .mark_path = MOUNT_PATH,
+ .mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_MOUNT,
- .ignore_path = FILE_MNT2,
+ .ignore_path_fmt = FILE_MNT2,
.ignore_mark_type = FANOTIFY_INODE,
- .event_path = FILE_PATH,
+ .event_path_fmt = FILE_PATH,
.expected_mask_without_ignore = FAN_OPEN
},
{
.tname = "ignore exec mount events created on a specific file",
- .mark_path = MOUNT_PATH,
+ .mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_MOUNT,
- .ignore_path = FILE_EXEC_PATH2,
+ .ignore_path_fmt = FILE_EXEC_PATH2,
.ignore_mark_type = FANOTIFY_INODE,
- .event_path = FILE_EXEC_PATH,
+ .event_path_fmt = FILE_EXEC_PATH,
.expected_mask_with_ignore = FAN_OPEN_EXEC,
.expected_mask_without_ignore = FAN_OPEN | FAN_OPEN_EXEC
},
{
.tname = "don't ignore mount events created on another file",
- .mark_path = MOUNT_PATH,
+ .mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_MOUNT,
- .ignore_path = FILE_PATH,
+ .ignore_path_fmt = FILE_PATH,
.ignore_mark_type = FANOTIFY_INODE,
- .event_path = FILE2_PATH,
+ .event_path_fmt = FILE2_PATH,
.expected_mask_with_ignore = FAN_OPEN,
.expected_mask_without_ignore = FAN_OPEN
},
{
.tname = "don't ignore exec mount events created on another file",
- .mark_path = MOUNT_PATH,
+ .mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_MOUNT,
- .ignore_path = FILE_EXEC_PATH,
+ .ignore_path_fmt = FILE_EXEC_PATH,
.ignore_mark_type = FANOTIFY_INODE,
- .event_path = FILE2_EXEC_PATH,
+ .event_path_fmt = FILE2_EXEC_PATH,
.expected_mask_with_ignore = FAN_OPEN | FAN_OPEN_EXEC,
.expected_mask_without_ignore = FAN_OPEN | FAN_OPEN_EXEC
},
{
.tname = "ignore inode events created on a specific mount point",
- .mark_path = FILE_PATH,
+ .mark_path_fmt = FILE_PATH,
.mark_type = FANOTIFY_INODE,
- .ignore_path = MNT2_PATH,
+ .ignore_path_fmt = MNT2_PATH,
.ignore_mark_type = FANOTIFY_MOUNT,
- .event_path = FILE_MNT2,
+ .event_path_fmt = FILE_MNT2,
.expected_mask_without_ignore = FAN_OPEN
},
{
.tname = "ignore exec inode events created on a specific mount point",
- .mark_path = FILE_EXEC_PATH,
+ .mark_path_fmt = FILE_EXEC_PATH,
.mark_type = FANOTIFY_INODE,
- .ignore_path = MNT2_PATH,
+ .ignore_path_fmt = MNT2_PATH,
.ignore_mark_type = FANOTIFY_MOUNT,
- .event_path = FILE_EXEC_PATH2,
+ .event_path_fmt = FILE_EXEC_PATH2,
.expected_mask_with_ignore = FAN_OPEN_EXEC,
.expected_mask_without_ignore = FAN_OPEN | FAN_OPEN_EXEC
},
{
.tname = "don't ignore inode events created on another mount point",
- .mark_path = FILE_MNT2,
+ .mark_path_fmt = FILE_MNT2,
.mark_type = FANOTIFY_INODE,
- .ignore_path = MNT2_PATH,
+ .ignore_path_fmt = MNT2_PATH,
.ignore_mark_type = FANOTIFY_MOUNT,
- .event_path = FILE_PATH,
+ .event_path_fmt = FILE_PATH,
.expected_mask_with_ignore = FAN_OPEN,
.expected_mask_without_ignore = FAN_OPEN
},
{
.tname = "don't ignore exec inode events created on another mount point",
- .mark_path = FILE_EXEC_PATH2,
+ .mark_path_fmt = FILE_EXEC_PATH2,
.mark_type = FANOTIFY_INODE,
- .ignore_path = MNT2_PATH,
+ .ignore_path_fmt = MNT2_PATH,
.ignore_mark_type = FANOTIFY_MOUNT,
- .event_path = FILE_EXEC_PATH,
+ .event_path_fmt = FILE_EXEC_PATH,
.expected_mask_with_ignore = FAN_OPEN | FAN_OPEN_EXEC,
.expected_mask_without_ignore = FAN_OPEN | FAN_OPEN_EXEC
},
{
.tname = "ignore fs events created on a specific file",
- .mark_path = MOUNT_PATH,
+ .mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_FILESYSTEM,
- .ignore_path = FILE_PATH,
+ .ignore_path_fmt = FILE_PATH,
.ignore_mark_type = FANOTIFY_INODE,
- .event_path = FILE_PATH,
+ .event_path_fmt = FILE_PATH,
.expected_mask_without_ignore = FAN_OPEN
},
{
.tname = "ignore exec fs events created on a specific file",
- .mark_path = MOUNT_PATH,
+ .mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_FILESYSTEM,
- .ignore_path = FILE_EXEC_PATH,
+ .ignore_path_fmt = FILE_EXEC_PATH,
.ignore_mark_type = FANOTIFY_INODE,
- .event_path = FILE_EXEC_PATH,
+ .event_path_fmt = FILE_EXEC_PATH,
.expected_mask_with_ignore = FAN_OPEN_EXEC,
.expected_mask_without_ignore = FAN_OPEN | FAN_OPEN_EXEC
},
{
.tname = "don't ignore mount events created on another file",
- .mark_path = MOUNT_PATH,
+ .mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_FILESYSTEM,
- .ignore_path = FILE_PATH,
+ .ignore_path_fmt = FILE_PATH,
.ignore_mark_type = FANOTIFY_INODE,
- .event_path = FILE2_PATH,
+ .event_path_fmt = FILE2_PATH,
.expected_mask_with_ignore = FAN_OPEN,
.expected_mask_without_ignore = FAN_OPEN
},
{
.tname = "don't ignore exec mount events created on another file",
- .mark_path = MOUNT_PATH,
+ .mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_FILESYSTEM,
- .ignore_path = FILE_EXEC_PATH,
+ .ignore_path_fmt = FILE_EXEC_PATH,
.ignore_mark_type = FANOTIFY_INODE,
- .event_path = FILE2_EXEC_PATH,
+ .event_path_fmt = FILE2_EXEC_PATH,
.expected_mask_with_ignore = FAN_OPEN | FAN_OPEN_EXEC,
.expected_mask_without_ignore = FAN_OPEN | FAN_OPEN_EXEC
},
{
.tname = "ignore fs events created on a specific mount point",
- .mark_path = MOUNT_PATH,
+ .mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_FILESYSTEM,
- .ignore_path = MNT2_PATH,
+ .ignore_path_fmt = MNT2_PATH,
.ignore_mark_type = FANOTIFY_MOUNT,
- .event_path = FILE_MNT2,
+ .event_path_fmt = FILE_MNT2,
.expected_mask_without_ignore = FAN_OPEN
},
{
.tname = "ignore exec fs events created on a specific mount point",
- .mark_path = MOUNT_PATH,
+ .mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_FILESYSTEM,
- .ignore_path = MNT2_PATH,
+ .ignore_path_fmt = MNT2_PATH,
.ignore_mark_type = FANOTIFY_MOUNT,
- .event_path = FILE_EXEC_PATH2,
+ .event_path_fmt = FILE_EXEC_PATH2,
.expected_mask_with_ignore = FAN_OPEN_EXEC,
.expected_mask_without_ignore = FAN_OPEN | FAN_OPEN_EXEC
},
{
.tname = "don't ignore fs events created on another mount point",
- .mark_path = MOUNT_PATH,
+ .mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_FILESYSTEM,
- .ignore_path = MNT2_PATH,
+ .ignore_path_fmt = MNT2_PATH,
.ignore_mark_type = FANOTIFY_MOUNT,
- .event_path = FILE_PATH,
+ .event_path_fmt = FILE_PATH,
.expected_mask_with_ignore = FAN_OPEN,
.expected_mask_without_ignore = FAN_OPEN
},
{
.tname = "don't ignore exec fs events created on another mount point",
- .mark_path = MOUNT_PATH,
+ .mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_FILESYSTEM,
- .ignore_path = MNT2_PATH,
+ .ignore_path_fmt = MNT2_PATH,
.ignore_mark_type = FANOTIFY_MOUNT,
- .event_path = FILE_EXEC_PATH,
+ .event_path_fmt = FILE_EXEC_PATH,
.expected_mask_with_ignore = FAN_OPEN | FAN_OPEN_EXEC,
.expected_mask_without_ignore = FAN_OPEN | FAN_OPEN_EXEC
},
{
.tname = "ignore child exec events created on a specific mount point",
- .mark_path = MOUNT_PATH,
+ .mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_PARENT,
- .ignore_path = MOUNT_PATH,
+ .ignore_path_fmt = MOUNT_PATH,
.ignore_mark_type = FANOTIFY_MOUNT,
- .event_path = FILE_EXEC_PATH,
+ .event_path_fmt = FILE_EXEC_PATH,
.expected_mask_with_ignore = FAN_OPEN_EXEC,
.expected_mask_without_ignore = FAN_OPEN | FAN_OPEN_EXEC
},
{
.tname = "ignore events on children of directory created on a specific file",
- .mark_path = DIR_PATH,
+ .mark_path_fmt = DIR_PATH,
.mark_type = FANOTIFY_PARENT,
- .ignore_path = DIR_PATH,
+ .ignore_path_fmt = DIR_PATH,
.ignore_mark_type = FANOTIFY_PARENT,
.ignored_flags = FAN_EVENT_ON_CHILD,
- .event_path = FILE_PATH,
+ .event_path_fmt = FILE_PATH,
.expected_mask_without_ignore = FAN_OPEN
},
{
.tname = "ignore events on file created inside a parent watching children",
- .mark_path = FILE_PATH,
+ .mark_path_fmt = FILE_PATH,
.mark_type = FANOTIFY_INODE,
- .ignore_path = DIR_PATH,
+ .ignore_path_fmt = DIR_PATH,
.ignore_mark_type = FANOTIFY_PARENT,
.ignored_flags = FAN_EVENT_ON_CHILD,
- .event_path = FILE_PATH,
+ .event_path_fmt = FILE_PATH,
.expected_mask_without_ignore = FAN_OPEN
},
{
.tname = "don't ignore events on file created inside a parent not watching children",
- .mark_path = FILE_PATH,
+ .mark_path_fmt = FILE_PATH,
.mark_type = FANOTIFY_INODE,
- .ignore_path = DIR_PATH,
+ .ignore_path_fmt = DIR_PATH,
.ignore_mark_type = FANOTIFY_PARENT,
- .event_path = FILE_PATH,
+ .event_path_fmt = FILE_PATH,
.expected_mask_with_ignore = FAN_OPEN,
.expected_mask_without_ignore = FAN_OPEN
},
{
.tname = "ignore mount events created inside a parent watching children",
- .mark_path = FILE_PATH,
+ .mark_path_fmt = FILE_PATH,
.mark_type = FANOTIFY_MOUNT,
- .ignore_path = DIR_PATH,
+ .ignore_path_fmt = DIR_PATH,
.ignore_mark_type = FANOTIFY_PARENT,
.ignored_flags = FAN_EVENT_ON_CHILD,
- .event_path = FILE_PATH,
+ .event_path_fmt = FILE_PATH,
.expected_mask_without_ignore = FAN_OPEN
},
{
.tname = "don't ignore mount events created inside a parent not watching children",
- .mark_path = FILE_PATH,
+ .mark_path_fmt = FILE_PATH,
.mark_type = FANOTIFY_MOUNT,
- .ignore_path = DIR_PATH,
+ .ignore_path_fmt = DIR_PATH,
.ignore_mark_type = FANOTIFY_PARENT,
- .event_path = FILE_PATH,
+ .event_path_fmt = FILE_PATH,
.expected_mask_with_ignore = FAN_OPEN,
.expected_mask_without_ignore = FAN_OPEN
},
{
.tname = "ignore fs events created inside a parent watching children",
- .mark_path = FILE_PATH,
+ .mark_path_fmt = FILE_PATH,
.mark_type = FANOTIFY_FILESYSTEM,
- .ignore_path = DIR_PATH,
+ .ignore_path_fmt = DIR_PATH,
.ignore_mark_type = FANOTIFY_PARENT,
.ignored_flags = FAN_EVENT_ON_CHILD,
- .event_path = FILE_PATH,
+ .event_path_fmt = FILE_PATH,
.expected_mask_without_ignore = FAN_OPEN
},
{
.tname = "don't ignore fs events created inside a parent not watching children",
- .mark_path = FILE_PATH,
+ .mark_path_fmt = FILE_PATH,
.mark_type = FANOTIFY_FILESYSTEM,
- .ignore_path = DIR_PATH,
+ .ignore_path_fmt = DIR_PATH,
.ignore_mark_type = FANOTIFY_PARENT,
- .event_path = FILE_PATH,
+ .event_path_fmt = FILE_PATH,
.expected_mask_with_ignore = FAN_OPEN,
.expected_mask_without_ignore = FAN_OPEN
},
/* Evictable ignore mark test cases */
{
.tname = "don't ignore mount events created on file with evicted ignore mark",
- .mark_path = MOUNT_PATH,
+ .mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_MOUNT,
- .ignore_path = FILE_PATH,
+ .ignore_path_fmt = FILE_PATH,
.ignore_mark_type = FANOTIFY_EVICTABLE,
- .event_path = FILE_PATH,
+ .event_path_fmt = FILE_PATH,
.expected_mask_with_ignore = FAN_OPEN,
.expected_mask_without_ignore = FAN_OPEN
},
{
.tname = "don't ignore fs events created on a file with evicted ignore mark",
- .mark_path = MOUNT_PATH,
+ .mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_FILESYSTEM,
- .ignore_path = FILE_PATH,
+ .ignore_path_fmt = FILE_PATH,
.ignore_mark_type = FANOTIFY_EVICTABLE,
- .event_path = FILE_PATH,
+ .event_path_fmt = FILE_PATH,
.expected_mask_with_ignore = FAN_OPEN,
.expected_mask_without_ignore = FAN_OPEN
},
{
.tname = "don't ignore mount events created inside a parent with evicted ignore mark",
- .mark_path = MOUNT_PATH,
+ .mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_MOUNT,
- .ignore_path = DIR_PATH,
+ .ignore_path_fmt = DIR_PATH,
.ignore_mark_type = FANOTIFY_EVICTABLE,
.ignored_flags = FAN_EVENT_ON_CHILD,
- .event_path = FILE_PATH,
+ .event_path_fmt = FILE_PATH,
.expected_mask_with_ignore = FAN_OPEN,
.expected_mask_without_ignore = FAN_OPEN
},
{
.tname = "don't ignore fs events created inside a parent with evicted ignore mark",
- .mark_path = MOUNT_PATH,
+ .mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_FILESYSTEM,
- .ignore_path = DIR_PATH,
+ .ignore_path_fmt = DIR_PATH,
.ignore_mark_type = FANOTIFY_EVICTABLE,
.ignored_flags = FAN_EVENT_ON_CHILD,
- .event_path = FILE_PATH,
+ .event_path_fmt = FILE_PATH,
.expected_mask_with_ignore = FAN_OPEN,
.expected_mask_without_ignore = FAN_OPEN
},
/* FAN_MARK_IGNORE specific test cases */
{
.tname = "ignore events on subdir inside a parent watching subdirs",
- .mark_path = SUBDIR_PATH,
+ .mark_path_fmt = SUBDIR_PATH,
.mark_type = FANOTIFY_SUBDIR,
- .ignore_path = DIR_PATH,
+ .ignore_path_fmt = DIR_PATH,
.ignore_mark_type = FANOTIFY_PARENT,
.ignored_flags = FAN_EVENT_ON_CHILD | FAN_ONDIR,
- .event_path = SUBDIR_PATH,
+ .event_path_fmt = SUBDIR_PATH,
.expected_mask_with_ignore = 0,
.expected_mask_without_ignore = FAN_OPEN | FAN_ONDIR
},
{
.tname = "don't ignore events on subdir inside a parent not watching children",
- .mark_path = SUBDIR_PATH,
+ .mark_path_fmt = SUBDIR_PATH,
.mark_type = FANOTIFY_SUBDIR,
- .ignore_path = DIR_PATH,
+ .ignore_path_fmt = DIR_PATH,
.ignore_mark_type = FANOTIFY_PARENT,
.ignored_flags = FAN_ONDIR,
- .event_path = SUBDIR_PATH,
+ .event_path_fmt = SUBDIR_PATH,
.expected_mask_with_ignore = FAN_OPEN | FAN_ONDIR,
.expected_mask_without_ignore = FAN_OPEN | FAN_ONDIR
},
{
.tname = "don't ignore events on subdir inside a parent watching non-dir children",
- .mark_path = SUBDIR_PATH,
+ .mark_path_fmt = SUBDIR_PATH,
.mark_type = FANOTIFY_SUBDIR,
- .ignore_path = DIR_PATH,
+ .ignore_path_fmt = DIR_PATH,
.ignore_mark_type = FANOTIFY_PARENT,
.ignored_flags = FAN_EVENT_ON_CHILD,
- .event_path = SUBDIR_PATH,
+ .event_path_fmt = SUBDIR_PATH,
.expected_mask_with_ignore = FAN_OPEN | FAN_ONDIR,
.expected_mask_without_ignore = FAN_OPEN | FAN_ONDIR
},
};
-static void show_fanotify_ignore_marks(int fd, int expected)
+static int format_path_check(char *buf, const char *fmt, int count, int i)
+{
+ int limit = count ? : 1;
+
+ if (i >= limit)
+ return 0;
+
+ if (count)
+ sprintf(buf, fmt, i);
+ else
+ strcpy(buf, fmt);
+ return 1;
+}
+
+#define foreach_path(tc, buf, pname) \
+ for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
+ (tc)->pname##_cnt, piter); piter++)
+
+static void show_fanotify_ignore_marks(int fd, int min, int max)
{
unsigned int mflags, mask, ignored_mask;
char procfdinfo[100];
+ char line[BUFSIZ];
+ int marks = 0;
+ FILE *f;
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) || !ignored_mask) {
- tst_res(!expected ? TPASS : TFAIL,
- "No fanotify inode ignore marks %sexpected",
- !expected ? "as " : "is un");
- } else {
- tst_res(expected ? TPASS : TFAIL,
- "Found %sexpected inode ignore mark (mflags=%x, mask=%x ignored_mask=%x)",
- expected ? "" : "un", mflags, mask, ignored_mask);
+ f = SAFE_FOPEN(procfdinfo, "r");
+ while (fgets(line, BUFSIZ, f)) {
+ if (sscanf(line, "fanotify ino:%*x sdev:%*x mflags: %x mask:%x ignored_mask:%x",
+ &mflags, &mask, &ignored_mask) == 3) {
+ if (ignored_mask != 0)
+ marks++;
+ }
}
+ if (marks < min) {
+ tst_res(TFAIL, "Found %d ignore marks but at least %d expected", marks, min);
+ return;
+ }
+ if (marks > max) {
+ tst_res(TFAIL, "Found %d ignore marks but at most %d expected", marks, max);
+ return;
+ }
+ tst_res(TPASS, "Found %d ignore marks which is in expected range %d-%d", marks, min, max);
}
static void drop_caches(void)
@@ -482,6 +515,7 @@ static int create_fanotify_groups(unsigned int n)
int evictable_ignored = (tc->ignore_mark_type == FANOTIFY_EVICTABLE);
int ignore_mark_type;
int ignored_onchild = tc->ignored_flags & FAN_EVENT_ON_CHILD;
+ char path[PATH_MAX];
mark = &fanotify_mark_types[tc->mark_type];
ignore_mark = &fanotify_mark_types[tc->ignore_mark_type];
@@ -501,11 +535,12 @@ static int create_fanotify_groups(unsigned int n)
* FAN_EVENT_ON_CHILD has no effect on filesystem/mount
* or inode mark on non-directory.
*/
- SAFE_FANOTIFY_MARK(fd_notify[p][i],
+ foreach_path(tc, path, mark_path)
+ SAFE_FANOTIFY_MARK(fd_notify[p][i],
FAN_MARK_ADD | mark->flag,
tc->expected_mask_without_ignore |
FAN_EVENT_ON_CHILD | FAN_ONDIR,
- AT_FDCWD, tc->mark_path);
+ AT_FDCWD, path);
/* Do not add ignore mark for first priority groups */
if (p == 0)
@@ -519,9 +554,10 @@ static int create_fanotify_groups(unsigned int n)
mark_ignored = tst_variant ? FAN_MARK_IGNORE_SURV : FAN_MARK_IGNORED_SURV;
mask = FAN_OPEN | tc->ignored_flags;
add_mark:
- SAFE_FANOTIFY_MARK(fd_notify[p][i],
+ foreach_path(tc, path, ignore_path)
+ SAFE_FANOTIFY_MARK(fd_notify[p][i],
FAN_MARK_ADD | ignore_mark->flag | mark_ignored,
- mask, AT_FDCWD, tc->ignore_path);
+ mask, AT_FDCWD, path);
/*
* FAN_MARK_IGNORE respects FAN_EVENT_ON_CHILD flag, but legacy
@@ -578,9 +614,24 @@ add_mark:
if (ignore_mark_type == FAN_MARK_INODE) {
for (p = 0; p < num_classes; p++) {
for (i = 0; i < GROUPS_PER_PRIO; i++) {
- if (fd_notify[p][i] > 0)
+ if (fd_notify[p][i] > 0) {
+ int minexp, maxexp;
+
+ if (p == 0) {
+ minexp = maxexp = 0;
+ } else if (evictable_ignored) {
+ minexp = 0;
+ /*
+ * Check at least half the
+ * marks get evicted by reclaim
+ */
+ maxexp = tc->ignore_path_cnt / 2;
+ } else {
+ minexp = maxexp = tc->ignore_path_cnt ? : 1;
+ }
show_fanotify_ignore_marks(fd_notify[p][i],
- p > 0 && !evictable_ignored);
+ minexp, maxexp);
+ }
}
}
}
@@ -613,7 +664,7 @@ static void mount_cycle(void)
bind_mount_created = 1;
}
-static void verify_event(int p, int group, struct fanotify_event_metadata *event,
+static int verify_event(int p, int group, struct fanotify_event_metadata *event,
unsigned long long expected_mask)
{
/* Only FAN_REPORT_FID reports the FAN_ONDIR flag in events on dirs */
@@ -626,33 +677,35 @@ static void verify_event(int p, int group, struct fanotify_event_metadata *event
(unsigned long long) event->mask,
(unsigned long long) expected_mask,
(unsigned int)event->pid, event->fd);
+ return 0;
} else if (event->pid != child_pid) {
tst_res(TFAIL, "group %d (%x) got event: mask %llx pid=%u "
"(expected %u) fd=%u", group, fanotify_class[p],
(unsigned long long)event->mask, (unsigned int)event->pid,
(unsigned int)child_pid, event->fd);
- } else {
- tst_res(TPASS, "group %d (%x) got event: mask %llx pid=%u fd=%u",
- group, fanotify_class[p], (unsigned long long)event->mask,
- (unsigned int)event->pid, event->fd);
+ return 0;
}
+ return 1;
}
-static int generate_event(const char *event_path,
- unsigned long long expected_mask)
+static int generate_event(struct tcase *tc, unsigned long long expected_mask)
{
int fd, status;
child_pid = SAFE_FORK();
if (child_pid == 0) {
- if (expected_mask & FAN_OPEN_EXEC) {
- SAFE_EXECL(event_path, event_path, NULL);
- } else {
- fd = SAFE_OPEN(event_path, O_RDONLY);
+ char path[PATH_MAX];
+
+ foreach_path(tc, path, event_path) {
+ if (expected_mask & FAN_OPEN_EXEC) {
+ SAFE_EXECL(path, path, NULL);
+ } else {
+ fd = SAFE_OPEN(path, O_RDONLY);
- if (fd > 0)
- SAFE_CLOSE(fd);
+ if (fd > 0)
+ SAFE_CLOSE(fd);
+ }
}
exit(0);
@@ -665,6 +718,37 @@ static int generate_event(const char *event_path,
return 0;
}
+struct fanotify_event_metadata *fetch_event(int fd, int *retp)
+{
+ int ret;
+ struct fanotify_event_metadata *event;
+
+ *retp = 0;
+ if (event_buf_pos >= event_buf_len) {
+ event_buf_pos = 0;
+ ret = read(fd, event_buf, EVENT_BUF_LEN);
+ if (ret < 0) {
+ if (errno == EAGAIN)
+ return NULL;
+ tst_brk(TBROK | TERRNO, "reading fanotify events failed");
+ *retp = -1;
+ return NULL;
+ }
+ event_buf_len = ret;
+ }
+ if (event_buf_len - event_buf_pos < (int)FAN_EVENT_METADATA_LEN) {
+ tst_brk(TBROK,
+ "too short event when reading fanotify events (%d < %d)",
+ event_buf_len - event_buf_pos,
+ (int)FAN_EVENT_METADATA_LEN);
+ *retp = -1;
+ return NULL;
+ }
+ event = (struct fanotify_event_metadata *)(event_buf + event_buf_pos);
+ event_buf_pos += event->event_len;
+ return event;
+}
+
static void test_fanotify(unsigned int n)
{
struct tcase *tc = &tcases[n];
@@ -672,6 +756,7 @@ static void test_fanotify(unsigned int n)
int ret;
unsigned int p, i;
struct fanotify_event_metadata *event;
+ int event_count;
tst_res(TINFO, "Test #%d: %s", n, tc->tname);
@@ -715,7 +800,7 @@ static void test_fanotify(unsigned int n)
ignore_mark = &fanotify_mark_types[tc->ignore_mark_type];
/* Generate event in child process */
- if (!generate_event(tc->event_path, tc->expected_mask_with_ignore))
+ if (!generate_event(tc, tc->expected_mask_with_ignore))
tst_brk(TBROK, "Child process terminated incorrectly");
/* First verify all groups without matching ignore mask got the event */
@@ -724,64 +809,52 @@ static void test_fanotify(unsigned int n)
break;
for (i = 0; i < GROUPS_PER_PRIO; i++) {
- ret = read(fd_notify[p][i], event_buf, EVENT_BUF_LEN);
- if (ret < 0) {
- if (errno == EAGAIN) {
- tst_res(TFAIL, "group %d (%x) "
- "with %s did not get event",
- i, fanotify_class[p], mark->name);
- continue;
- }
- tst_brk(TBROK | TERRNO,
- "reading fanotify events failed");
- }
- if (ret < (int)FAN_EVENT_METADATA_LEN) {
- tst_brk(TBROK,
- "short read when reading fanotify "
- "events (%d < %d)", ret,
- (int)EVENT_BUF_LEN);
+ event_count = 0;
+ event_buf_pos = event_buf_len = 0;
+ while ((event = fetch_event(fd_notify[p][i], &ret))) {
+ event_count++;
+ if (!verify_event(p, i, event, p == 0 ?
+ tc->expected_mask_without_ignore :
+ tc->expected_mask_with_ignore))
+ break;
+ if (event->fd != FAN_NOFD)
+ SAFE_CLOSE(event->fd);
}
- event = (struct fanotify_event_metadata *)event_buf;
- if (ret > (int)event->event_len) {
+ if (ret < 0)
+ continue;
+ if (event_count != (tc->event_path_cnt ? : 1)) {
tst_res(TFAIL, "group %d (%x) with %s "
- "got more than one event (%d > %d)",
- i, fanotify_class[p], mark->name, ret,
- event->event_len);
+ "got unexpected number of events (%d != %d)",
+ i, fanotify_class[p], mark->name,
+ event_count, tc->event_path_cnt);
} else {
- verify_event(p, i, event, p == 0 ?
- tc->expected_mask_without_ignore :
- tc->expected_mask_with_ignore);
+ tst_res(TPASS, "group %d (%x) got %d events: mask %llx pid=%u",
+ i, fanotify_class[p], event_count,
+ (unsigned long long)(p == 0 ?
+ tc->expected_mask_without_ignore :
+ tc->expected_mask_with_ignore),
+ (unsigned int)child_pid);
}
- if (event->fd != FAN_NOFD)
- SAFE_CLOSE(event->fd);
}
}
/* Then verify all groups with matching ignore mask did got the event */
for (p = 1; p < num_classes && !tc->expected_mask_with_ignore; p++) {
for (i = 0; i < GROUPS_PER_PRIO; i++) {
- ret = read(fd_notify[p][i], event_buf, EVENT_BUF_LEN);
- if (ret >= 0 && ret < (int)FAN_EVENT_METADATA_LEN) {
- tst_brk(TBROK,
- "short read when reading fanotify "
- "events (%d < %d)", ret,
- (int)EVENT_BUF_LEN);
- }
- event = (struct fanotify_event_metadata *)event_buf;
- if (ret > 0) {
- tst_res(TFAIL, "group %d (%x) with %s and "
- "%s ignore mask got unexpected event (mask %llx)",
- i, fanotify_class[p], mark->name, ignore_mark->name,
- event->mask);
+ event_count = 0;
+ event_buf_pos = event_buf_len = 0;
+ while ((event = fetch_event(fd_notify[p][i], &ret))) {
+ event_count++;
if (event->fd != FAN_NOFD)
SAFE_CLOSE(event->fd);
- } else if (errno == EAGAIN) {
- tst_res(TPASS, "group %d (%x) with %s and "
- "%s ignore mask got no event",
- i, fanotify_class[p], mark->name, ignore_mark->name);
- } else {
- tst_brk(TBROK | TERRNO,
- "reading fanotify events failed");
}
+ if (ret < 0)
+ continue;
+ if (event_count > tc->event_path_cnt / 2)
+ tst_res(TFAIL, "group %d (%x) with %s and "
+ "%s ignore mask got unexpectedly many events (%d > %d)",
+ i, fanotify_class[p], mark->name,
+ ignore_mark->name, event_count,
+ tc->event_path_cnt / 2);
}
}
cleanup:
--
2.35.3
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [LTP] [PATCH 3/3] fanotify10: Make evictable marks tests more reliable
2022-11-15 12:47 [LTP] [PATCH 0/3] Make fanotify10 test yet more reliable Jan Kara
2022-11-15 12:47 ` [LTP] [PATCH 1/3] fanotify10: Use named initializers Jan Kara
2022-11-15 12:47 ` [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files Jan Kara
@ 2022-11-15 12:47 ` Jan Kara
2022-11-16 2:17 ` Pengfei Xu
2022-11-21 15:09 ` Cyril Hrubis
2 siblings, 2 replies; 24+ messages in thread
From: Jan Kara @ 2022-11-15 12:47 UTC (permalink / raw)
To: ltp; +Cc: Jan Kara
Tests verifying that evictable inode marks don't pin inodes in memory
are unreliable because slab shrinking (triggered by drop_caches) does
not reliably evict inodes - dentries have to take round through the LRU
list and only then get reclaimed and inodes get unpinned and then inodes
have to be rotated through their LRU list to get reclaimed. If there are
not enough freed entries while shrinking other slab caches, drop_caches
will abort attempts to reclaim slab before inodes get evicted.
Tweak evictable marks tests to use more files and marks in parallel and
just verify that some (at least half) of the marks got evicted. This
should be more tolerant to random fluctuation in slab reclaim
efficiency.
Signed-off-by: Jan Kara <jack@suse.cz>
---
.../kernel/syscalls/fanotify/fanotify10.c | 62 ++++++++++++++++---
1 file changed, 52 insertions(+), 10 deletions(-)
diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
index e19bd907470f..cfbf4c31dd08 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify10.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
@@ -86,7 +86,10 @@ static int ignore_mark_unsupported;
#define TEST_APP "fanotify_child"
#define TEST_APP2 "fanotify_child2"
#define DIR_PATH MOUNT_PATH"/"DIR_NAME
+#define DIR_PATH_MULTI DIR_PATH"%d"
#define FILE_PATH DIR_PATH"/"FILE_NAME
+#define FILE_PATH_MULTI FILE_PATH"%d"
+#define FILE_PATH_MULTIDIR DIR_PATH_MULTI"/"FILE_NAME
#define FILE2_PATH DIR_PATH"/"FILE2_NAME
#define SUBDIR_PATH DIR_PATH"/"SUBDIR_NAME
#define FILE_EXEC_PATH MOUNT_PATH"/"TEST_APP
@@ -104,6 +107,7 @@ static int old_cache_pressure;
static pid_t child_pid;
static int bind_mount_created;
static unsigned int num_classes = NUM_CLASSES;
+static int max_file_multi;
enum {
FANOTIFY_INODE,
@@ -378,9 +382,11 @@ static struct tcase {
.tname = "don't ignore mount events created on file with evicted ignore mark",
.mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_MOUNT,
- .ignore_path_fmt = FILE_PATH,
+ .ignore_path_cnt = 16,
+ .ignore_path_fmt = FILE_PATH_MULTI,
.ignore_mark_type = FANOTIFY_EVICTABLE,
- .event_path_fmt = FILE_PATH,
+ .event_path_cnt = 16,
+ .event_path_fmt = FILE_PATH_MULTI,
.expected_mask_with_ignore = FAN_OPEN,
.expected_mask_without_ignore = FAN_OPEN
},
@@ -388,9 +394,11 @@ static struct tcase {
.tname = "don't ignore fs events created on a file with evicted ignore mark",
.mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_FILESYSTEM,
- .ignore_path_fmt = FILE_PATH,
+ .ignore_path_cnt = 16,
+ .ignore_path_fmt = FILE_PATH_MULTI,
.ignore_mark_type = FANOTIFY_EVICTABLE,
- .event_path_fmt = FILE_PATH,
+ .event_path_cnt = 16,
+ .event_path_fmt = FILE_PATH_MULTI,
.expected_mask_with_ignore = FAN_OPEN,
.expected_mask_without_ignore = FAN_OPEN
},
@@ -398,10 +406,12 @@ static struct tcase {
.tname = "don't ignore mount events created inside a parent with evicted ignore mark",
.mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_MOUNT,
- .ignore_path_fmt = DIR_PATH,
+ .ignore_path_cnt = 16,
+ .ignore_path_fmt = DIR_PATH_MULTI,
.ignore_mark_type = FANOTIFY_EVICTABLE,
.ignored_flags = FAN_EVENT_ON_CHILD,
- .event_path_fmt = FILE_PATH,
+ .event_path_cnt = 16,
+ .event_path_fmt = FILE_PATH_MULTIDIR,
.expected_mask_with_ignore = FAN_OPEN,
.expected_mask_without_ignore = FAN_OPEN
},
@@ -409,10 +419,12 @@ static struct tcase {
.tname = "don't ignore fs events created inside a parent with evicted ignore mark",
.mark_path_fmt = MOUNT_PATH,
.mark_type = FANOTIFY_FILESYSTEM,
- .ignore_path_fmt = DIR_PATH,
+ .ignore_path_cnt = 16,
+ .ignore_path_fmt = DIR_PATH_MULTI,
.ignore_mark_type = FANOTIFY_EVICTABLE,
.ignored_flags = FAN_EVENT_ON_CHILD,
- .event_path_fmt = FILE_PATH,
+ .event_path_cnt = 16,
+ .event_path_fmt = FILE_PATH_MULTIDIR,
.expected_mask_with_ignore = FAN_OPEN,
.expected_mask_without_ignore = FAN_OPEN
},
@@ -864,6 +876,8 @@ cleanup:
static void setup(void)
{
+ int i;
+
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);
@@ -880,7 +894,24 @@ static void setup(void)
SAFE_MKDIR(DIR_PATH, 0755);
SAFE_MKDIR(SUBDIR_PATH, 0755);
SAFE_FILE_PRINTF(FILE_PATH, "1");
- SAFE_FILE_PRINTF(FILE2_PATH, "1");
+ for (i = 0; i < (int)(sizeof(tcases)/sizeof(tcases[0])); i++) {
+ if (tcases[i].mark_path_cnt > max_file_multi)
+ max_file_multi = tcases[i].mark_path_cnt;
+ if (tcases[i].ignore_path_cnt > max_file_multi)
+ max_file_multi = tcases[i].ignore_path_cnt;
+ if (tcases[i].event_path_cnt > max_file_multi)
+ max_file_multi = tcases[i].event_path_cnt;
+ }
+ for (i = 0; i < max_file_multi; i++) {
+ char path[PATH_MAX];
+
+ sprintf(path, FILE_PATH_MULTI, i);
+ SAFE_FILE_PRINTF(path, "1");
+ sprintf(path, DIR_PATH_MULTI, i);
+ SAFE_MKDIR(path, 0755);
+ sprintf(path, FILE_PATH_MULTIDIR, i);
+ SAFE_FILE_PRINTF(path, "1");
+ }
SAFE_CP(TEST_APP, FILE_EXEC_PATH);
SAFE_CP(TEST_APP, FILE2_EXEC_PATH);
@@ -896,6 +927,8 @@ static void setup(void)
static void cleanup(void)
{
+ int i;
+
cleanup_fanotify_groups();
if (bind_mount_created)
@@ -903,8 +936,17 @@ static void cleanup(void)
SAFE_FILE_PRINTF(CACHE_PRESSURE_FILE, "%d", old_cache_pressure);
+ for (i = 0; i < max_file_multi; i++) {
+ char path[PATH_MAX];
+
+ sprintf(path, FILE_PATH_MULTIDIR, i);
+ SAFE_UNLINK(path);
+ sprintf(path, DIR_PATH_MULTI, i);
+ SAFE_RMDIR(path);
+ sprintf(path, FILE_PATH_MULTI, i);
+ SAFE_UNLINK(path);
+ }
SAFE_UNLINK(FILE_PATH);
- SAFE_UNLINK(FILE2_PATH);
SAFE_RMDIR(SUBDIR_PATH);
SAFE_RMDIR(DIR_PATH);
SAFE_RMDIR(MNT2_PATH);
--
2.35.3
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 3/3] fanotify10: Make evictable marks tests more reliable
2022-11-15 12:47 ` [LTP] [PATCH 3/3] fanotify10: Make evictable marks tests more reliable Jan Kara
@ 2022-11-16 2:17 ` Pengfei Xu
2022-11-16 10:58 ` Jan Kara
2022-11-21 15:09 ` Cyril Hrubis
1 sibling, 1 reply; 24+ messages in thread
From: Pengfei Xu @ 2022-11-16 2:17 UTC (permalink / raw)
To: Jan Kara; +Cc: ltp
Hi Jan Kara,
On 2022-11-15 at 13:47:38 +0100, Jan Kara wrote:
> Tests verifying that evictable inode marks don't pin inodes in memory
> are unreliable because slab shrinking (triggered by drop_caches) does
> not reliably evict inodes - dentries have to take round through the LRU
> list and only then get reclaimed and inodes get unpinned and then inodes
> have to be rotated through their LRU list to get reclaimed. If there are
> not enough freed entries while shrinking other slab caches, drop_caches
> will abort attempts to reclaim slab before inodes get evicted.
>
> Tweak evictable marks tests to use more files and marks in parallel and
> just verify that some (at least half) of the marks got evicted. This
> should be more tolerant to random fluctuation in slab reclaim
> efficiency.
>
If possible, could you add the Tested-by tag:
Tested-by: Pengfei Xu <pengfei.xu@intel.com>
Thanks!
BR.
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> .../kernel/syscalls/fanotify/fanotify10.c | 62 ++++++++++++++++---
> 1 file changed, 52 insertions(+), 10 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> index e19bd907470f..cfbf4c31dd08 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> @@ -86,7 +86,10 @@ static int ignore_mark_unsupported;
> #define TEST_APP "fanotify_child"
> #define TEST_APP2 "fanotify_child2"
> #define DIR_PATH MOUNT_PATH"/"DIR_NAME
> +#define DIR_PATH_MULTI DIR_PATH"%d"
> #define FILE_PATH DIR_PATH"/"FILE_NAME
> +#define FILE_PATH_MULTI FILE_PATH"%d"
> +#define FILE_PATH_MULTIDIR DIR_PATH_MULTI"/"FILE_NAME
> #define FILE2_PATH DIR_PATH"/"FILE2_NAME
> #define SUBDIR_PATH DIR_PATH"/"SUBDIR_NAME
> #define FILE_EXEC_PATH MOUNT_PATH"/"TEST_APP
> @@ -104,6 +107,7 @@ static int old_cache_pressure;
> static pid_t child_pid;
> static int bind_mount_created;
> static unsigned int num_classes = NUM_CLASSES;
> +static int max_file_multi;
>
> enum {
> FANOTIFY_INODE,
> @@ -378,9 +382,11 @@ static struct tcase {
> .tname = "don't ignore mount events created on file with evicted ignore mark",
> .mark_path_fmt = MOUNT_PATH,
> .mark_type = FANOTIFY_MOUNT,
> - .ignore_path_fmt = FILE_PATH,
> + .ignore_path_cnt = 16,
> + .ignore_path_fmt = FILE_PATH_MULTI,
> .ignore_mark_type = FANOTIFY_EVICTABLE,
> - .event_path_fmt = FILE_PATH,
> + .event_path_cnt = 16,
> + .event_path_fmt = FILE_PATH_MULTI,
> .expected_mask_with_ignore = FAN_OPEN,
> .expected_mask_without_ignore = FAN_OPEN
> },
> @@ -388,9 +394,11 @@ static struct tcase {
> .tname = "don't ignore fs events created on a file with evicted ignore mark",
> .mark_path_fmt = MOUNT_PATH,
> .mark_type = FANOTIFY_FILESYSTEM,
> - .ignore_path_fmt = FILE_PATH,
> + .ignore_path_cnt = 16,
> + .ignore_path_fmt = FILE_PATH_MULTI,
> .ignore_mark_type = FANOTIFY_EVICTABLE,
> - .event_path_fmt = FILE_PATH,
> + .event_path_cnt = 16,
> + .event_path_fmt = FILE_PATH_MULTI,
> .expected_mask_with_ignore = FAN_OPEN,
> .expected_mask_without_ignore = FAN_OPEN
> },
> @@ -398,10 +406,12 @@ static struct tcase {
> .tname = "don't ignore mount events created inside a parent with evicted ignore mark",
> .mark_path_fmt = MOUNT_PATH,
> .mark_type = FANOTIFY_MOUNT,
> - .ignore_path_fmt = DIR_PATH,
> + .ignore_path_cnt = 16,
> + .ignore_path_fmt = DIR_PATH_MULTI,
> .ignore_mark_type = FANOTIFY_EVICTABLE,
> .ignored_flags = FAN_EVENT_ON_CHILD,
> - .event_path_fmt = FILE_PATH,
> + .event_path_cnt = 16,
> + .event_path_fmt = FILE_PATH_MULTIDIR,
> .expected_mask_with_ignore = FAN_OPEN,
> .expected_mask_without_ignore = FAN_OPEN
> },
> @@ -409,10 +419,12 @@ static struct tcase {
> .tname = "don't ignore fs events created inside a parent with evicted ignore mark",
> .mark_path_fmt = MOUNT_PATH,
> .mark_type = FANOTIFY_FILESYSTEM,
> - .ignore_path_fmt = DIR_PATH,
> + .ignore_path_cnt = 16,
> + .ignore_path_fmt = DIR_PATH_MULTI,
> .ignore_mark_type = FANOTIFY_EVICTABLE,
> .ignored_flags = FAN_EVENT_ON_CHILD,
> - .event_path_fmt = FILE_PATH,
> + .event_path_cnt = 16,
> + .event_path_fmt = FILE_PATH_MULTIDIR,
> .expected_mask_with_ignore = FAN_OPEN,
> .expected_mask_without_ignore = FAN_OPEN
> },
> @@ -864,6 +876,8 @@ cleanup:
>
> static void setup(void)
> {
> + int i;
> +
> 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);
> @@ -880,7 +894,24 @@ static void setup(void)
> SAFE_MKDIR(DIR_PATH, 0755);
> SAFE_MKDIR(SUBDIR_PATH, 0755);
> SAFE_FILE_PRINTF(FILE_PATH, "1");
> - SAFE_FILE_PRINTF(FILE2_PATH, "1");
> + for (i = 0; i < (int)(sizeof(tcases)/sizeof(tcases[0])); i++) {
> + if (tcases[i].mark_path_cnt > max_file_multi)
> + max_file_multi = tcases[i].mark_path_cnt;
> + if (tcases[i].ignore_path_cnt > max_file_multi)
> + max_file_multi = tcases[i].ignore_path_cnt;
> + if (tcases[i].event_path_cnt > max_file_multi)
> + max_file_multi = tcases[i].event_path_cnt;
> + }
> + for (i = 0; i < max_file_multi; i++) {
> + char path[PATH_MAX];
> +
> + sprintf(path, FILE_PATH_MULTI, i);
> + SAFE_FILE_PRINTF(path, "1");
> + sprintf(path, DIR_PATH_MULTI, i);
> + SAFE_MKDIR(path, 0755);
> + sprintf(path, FILE_PATH_MULTIDIR, i);
> + SAFE_FILE_PRINTF(path, "1");
> + }
>
> SAFE_CP(TEST_APP, FILE_EXEC_PATH);
> SAFE_CP(TEST_APP, FILE2_EXEC_PATH);
> @@ -896,6 +927,8 @@ static void setup(void)
>
> static void cleanup(void)
> {
> + int i;
> +
> cleanup_fanotify_groups();
>
> if (bind_mount_created)
> @@ -903,8 +936,17 @@ static void cleanup(void)
>
> SAFE_FILE_PRINTF(CACHE_PRESSURE_FILE, "%d", old_cache_pressure);
>
> + for (i = 0; i < max_file_multi; i++) {
> + char path[PATH_MAX];
> +
> + sprintf(path, FILE_PATH_MULTIDIR, i);
> + SAFE_UNLINK(path);
> + sprintf(path, DIR_PATH_MULTI, i);
> + SAFE_RMDIR(path);
> + sprintf(path, FILE_PATH_MULTI, i);
> + SAFE_UNLINK(path);
> + }
> SAFE_UNLINK(FILE_PATH);
> - SAFE_UNLINK(FILE2_PATH);
> SAFE_RMDIR(SUBDIR_PATH);
> SAFE_RMDIR(DIR_PATH);
> SAFE_RMDIR(MNT2_PATH);
> --
> 2.35.3
>
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 3/3] fanotify10: Make evictable marks tests more reliable
2022-11-16 2:17 ` Pengfei Xu
@ 2022-11-16 10:58 ` Jan Kara
2022-11-16 16:32 ` Amir Goldstein
0 siblings, 1 reply; 24+ messages in thread
From: Jan Kara @ 2022-11-16 10:58 UTC (permalink / raw)
To: Pengfei Xu; +Cc: Jan Kara, ltp
On Wed 16-11-22 10:17:14, Pengfei Xu wrote:
> Hi Jan Kara,
>
> On 2022-11-15 at 13:47:38 +0100, Jan Kara wrote:
> > Tests verifying that evictable inode marks don't pin inodes in memory
> > are unreliable because slab shrinking (triggered by drop_caches) does
> > not reliably evict inodes - dentries have to take round through the LRU
> > list and only then get reclaimed and inodes get unpinned and then inodes
> > have to be rotated through their LRU list to get reclaimed. If there are
> > not enough freed entries while shrinking other slab caches, drop_caches
> > will abort attempts to reclaim slab before inodes get evicted.
> >
> > Tweak evictable marks tests to use more files and marks in parallel and
> > just verify that some (at least half) of the marks got evicted. This
> > should be more tolerant to random fluctuation in slab reclaim
> > efficiency.
> >
> If possible, could you add the Tested-by tag:
> Tested-by: Pengfei Xu <pengfei.xu@intel.com>
Sure, will do. I'll just wait whether there will be some other review
comments.
Honza
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> > .../kernel/syscalls/fanotify/fanotify10.c | 62 ++++++++++++++++---
> > 1 file changed, 52 insertions(+), 10 deletions(-)
> >
> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > index e19bd907470f..cfbf4c31dd08 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> > @@ -86,7 +86,10 @@ static int ignore_mark_unsupported;
> > #define TEST_APP "fanotify_child"
> > #define TEST_APP2 "fanotify_child2"
> > #define DIR_PATH MOUNT_PATH"/"DIR_NAME
> > +#define DIR_PATH_MULTI DIR_PATH"%d"
> > #define FILE_PATH DIR_PATH"/"FILE_NAME
> > +#define FILE_PATH_MULTI FILE_PATH"%d"
> > +#define FILE_PATH_MULTIDIR DIR_PATH_MULTI"/"FILE_NAME
> > #define FILE2_PATH DIR_PATH"/"FILE2_NAME
> > #define SUBDIR_PATH DIR_PATH"/"SUBDIR_NAME
> > #define FILE_EXEC_PATH MOUNT_PATH"/"TEST_APP
> > @@ -104,6 +107,7 @@ static int old_cache_pressure;
> > static pid_t child_pid;
> > static int bind_mount_created;
> > static unsigned int num_classes = NUM_CLASSES;
> > +static int max_file_multi;
> >
> > enum {
> > FANOTIFY_INODE,
> > @@ -378,9 +382,11 @@ static struct tcase {
> > .tname = "don't ignore mount events created on file with evicted ignore mark",
> > .mark_path_fmt = MOUNT_PATH,
> > .mark_type = FANOTIFY_MOUNT,
> > - .ignore_path_fmt = FILE_PATH,
> > + .ignore_path_cnt = 16,
> > + .ignore_path_fmt = FILE_PATH_MULTI,
> > .ignore_mark_type = FANOTIFY_EVICTABLE,
> > - .event_path_fmt = FILE_PATH,
> > + .event_path_cnt = 16,
> > + .event_path_fmt = FILE_PATH_MULTI,
> > .expected_mask_with_ignore = FAN_OPEN,
> > .expected_mask_without_ignore = FAN_OPEN
> > },
> > @@ -388,9 +394,11 @@ static struct tcase {
> > .tname = "don't ignore fs events created on a file with evicted ignore mark",
> > .mark_path_fmt = MOUNT_PATH,
> > .mark_type = FANOTIFY_FILESYSTEM,
> > - .ignore_path_fmt = FILE_PATH,
> > + .ignore_path_cnt = 16,
> > + .ignore_path_fmt = FILE_PATH_MULTI,
> > .ignore_mark_type = FANOTIFY_EVICTABLE,
> > - .event_path_fmt = FILE_PATH,
> > + .event_path_cnt = 16,
> > + .event_path_fmt = FILE_PATH_MULTI,
> > .expected_mask_with_ignore = FAN_OPEN,
> > .expected_mask_without_ignore = FAN_OPEN
> > },
> > @@ -398,10 +406,12 @@ static struct tcase {
> > .tname = "don't ignore mount events created inside a parent with evicted ignore mark",
> > .mark_path_fmt = MOUNT_PATH,
> > .mark_type = FANOTIFY_MOUNT,
> > - .ignore_path_fmt = DIR_PATH,
> > + .ignore_path_cnt = 16,
> > + .ignore_path_fmt = DIR_PATH_MULTI,
> > .ignore_mark_type = FANOTIFY_EVICTABLE,
> > .ignored_flags = FAN_EVENT_ON_CHILD,
> > - .event_path_fmt = FILE_PATH,
> > + .event_path_cnt = 16,
> > + .event_path_fmt = FILE_PATH_MULTIDIR,
> > .expected_mask_with_ignore = FAN_OPEN,
> > .expected_mask_without_ignore = FAN_OPEN
> > },
> > @@ -409,10 +419,12 @@ static struct tcase {
> > .tname = "don't ignore fs events created inside a parent with evicted ignore mark",
> > .mark_path_fmt = MOUNT_PATH,
> > .mark_type = FANOTIFY_FILESYSTEM,
> > - .ignore_path_fmt = DIR_PATH,
> > + .ignore_path_cnt = 16,
> > + .ignore_path_fmt = DIR_PATH_MULTI,
> > .ignore_mark_type = FANOTIFY_EVICTABLE,
> > .ignored_flags = FAN_EVENT_ON_CHILD,
> > - .event_path_fmt = FILE_PATH,
> > + .event_path_cnt = 16,
> > + .event_path_fmt = FILE_PATH_MULTIDIR,
> > .expected_mask_with_ignore = FAN_OPEN,
> > .expected_mask_without_ignore = FAN_OPEN
> > },
> > @@ -864,6 +876,8 @@ cleanup:
> >
> > static void setup(void)
> > {
> > + int i;
> > +
> > 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);
> > @@ -880,7 +894,24 @@ static void setup(void)
> > SAFE_MKDIR(DIR_PATH, 0755);
> > SAFE_MKDIR(SUBDIR_PATH, 0755);
> > SAFE_FILE_PRINTF(FILE_PATH, "1");
> > - SAFE_FILE_PRINTF(FILE2_PATH, "1");
> > + for (i = 0; i < (int)(sizeof(tcases)/sizeof(tcases[0])); i++) {
> > + if (tcases[i].mark_path_cnt > max_file_multi)
> > + max_file_multi = tcases[i].mark_path_cnt;
> > + if (tcases[i].ignore_path_cnt > max_file_multi)
> > + max_file_multi = tcases[i].ignore_path_cnt;
> > + if (tcases[i].event_path_cnt > max_file_multi)
> > + max_file_multi = tcases[i].event_path_cnt;
> > + }
> > + for (i = 0; i < max_file_multi; i++) {
> > + char path[PATH_MAX];
> > +
> > + sprintf(path, FILE_PATH_MULTI, i);
> > + SAFE_FILE_PRINTF(path, "1");
> > + sprintf(path, DIR_PATH_MULTI, i);
> > + SAFE_MKDIR(path, 0755);
> > + sprintf(path, FILE_PATH_MULTIDIR, i);
> > + SAFE_FILE_PRINTF(path, "1");
> > + }
> >
> > SAFE_CP(TEST_APP, FILE_EXEC_PATH);
> > SAFE_CP(TEST_APP, FILE2_EXEC_PATH);
> > @@ -896,6 +927,8 @@ static void setup(void)
> >
> > static void cleanup(void)
> > {
> > + int i;
> > +
> > cleanup_fanotify_groups();
> >
> > if (bind_mount_created)
> > @@ -903,8 +936,17 @@ static void cleanup(void)
> >
> > SAFE_FILE_PRINTF(CACHE_PRESSURE_FILE, "%d", old_cache_pressure);
> >
> > + for (i = 0; i < max_file_multi; i++) {
> > + char path[PATH_MAX];
> > +
> > + sprintf(path, FILE_PATH_MULTIDIR, i);
> > + SAFE_UNLINK(path);
> > + sprintf(path, DIR_PATH_MULTI, i);
> > + SAFE_RMDIR(path);
> > + sprintf(path, FILE_PATH_MULTI, i);
> > + SAFE_UNLINK(path);
> > + }
> > SAFE_UNLINK(FILE_PATH);
> > - SAFE_UNLINK(FILE2_PATH);
> > SAFE_RMDIR(SUBDIR_PATH);
> > SAFE_RMDIR(DIR_PATH);
> > SAFE_RMDIR(MNT2_PATH);
> > --
> > 2.35.3
> >
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 3/3] fanotify10: Make evictable marks tests more reliable
2022-11-16 10:58 ` Jan Kara
@ 2022-11-16 16:32 ` Amir Goldstein
2022-11-17 15:50 ` Petr Vorel
0 siblings, 1 reply; 24+ messages in thread
From: Amir Goldstein @ 2022-11-16 16:32 UTC (permalink / raw)
To: Jan Kara, Petr Vorel; +Cc: ltp
On Wed, Nov 16, 2022 at 12:58 PM Jan Kara <jack@suse.cz> wrote:
>
> On Wed 16-11-22 10:17:14, Pengfei Xu wrote:
> > Hi Jan Kara,
> >
> > On 2022-11-15 at 13:47:38 +0100, Jan Kara wrote:
> > > Tests verifying that evictable inode marks don't pin inodes in memory
> > > are unreliable because slab shrinking (triggered by drop_caches) does
> > > not reliably evict inodes - dentries have to take round through the LRU
> > > list and only then get reclaimed and inodes get unpinned and then inodes
> > > have to be rotated through their LRU list to get reclaimed. If there are
> > > not enough freed entries while shrinking other slab caches, drop_caches
> > > will abort attempts to reclaim slab before inodes get evicted.
> > >
> > > Tweak evictable marks tests to use more files and marks in parallel and
> > > just verify that some (at least half) of the marks got evicted. This
> > > should be more tolerant to random fluctuation in slab reclaim
> > > efficiency.
> > >
> > If possible, could you add the Tested-by tag:
> > Tested-by: Pengfei Xu <pengfei.xu@intel.com>
>
> Sure, will do. I'll just wait whether there will be some other review
> comments.
I would want to say Reviewed-by, but I could only say Eyeballed-by.
I like the change and thanks for figuring this out, but the review
was very hard, so I did not have time to do it thoroughly.
Good luck, Petr ;-)
Thanks,
Amir.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 3/3] fanotify10: Make evictable marks tests more reliable
2022-11-16 16:32 ` Amir Goldstein
@ 2022-11-17 15:50 ` Petr Vorel
0 siblings, 0 replies; 24+ messages in thread
From: Petr Vorel @ 2022-11-17 15:50 UTC (permalink / raw)
To: Amir Goldstein; +Cc: Jan Kara, ltp
Hi Jan, Amir,
> On Wed, Nov 16, 2022 at 12:58 PM Jan Kara <jack@suse.cz> wrote:
> > On Wed 16-11-22 10:17:14, Pengfei Xu wrote:
> > > Hi Jan Kara,
> > > On 2022-11-15 at 13:47:38 +0100, Jan Kara wrote:
> > > > Tests verifying that evictable inode marks don't pin inodes in memory
> > > > are unreliable because slab shrinking (triggered by drop_caches) does
> > > > not reliably evict inodes - dentries have to take round through the LRU
> > > > list and only then get reclaimed and inodes get unpinned and then inodes
> > > > have to be rotated through their LRU list to get reclaimed. If there are
> > > > not enough freed entries while shrinking other slab caches, drop_caches
> > > > will abort attempts to reclaim slab before inodes get evicted.
> > > > Tweak evictable marks tests to use more files and marks in parallel and
> > > > just verify that some (at least half) of the marks got evicted. This
> > > > should be more tolerant to random fluctuation in slab reclaim
> > > > efficiency.
> > > If possible, could you add the Tested-by tag:
> > > Tested-by: Pengfei Xu <pengfei.xu@intel.com>
> > Sure, will do. I'll just wait whether there will be some other review
> > comments.
> I would want to say Reviewed-by, but I could only say Eyeballed-by.
> I like the change and thanks for figuring this out, but the review
> was very hard, so I did not have time to do it thoroughly.
> Good luck, Petr ;-)
Thanks :). I'm ill, hoping to be back working next week, I'll have look soon.
Kind regards,
Petr
> Thanks,
> Amir.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files
2022-11-15 12:47 ` [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files Jan Kara
@ 2022-11-17 15:58 ` Petr Vorel
2022-11-21 9:14 ` Jan Kara
2022-11-21 15:04 ` Cyril Hrubis
1 sibling, 1 reply; 24+ messages in thread
From: Petr Vorel @ 2022-11-17 15:58 UTC (permalink / raw)
To: Jan Kara; +Cc: Richard Palethorpe, ltp
Hi Jan, all,
> +#define foreach_path(tc, buf, pname) \
> + for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
Unfortunately we still support C99 due old compiler on CentOS 7,
therefore int piter needs to be defined outside of for loop.
fanotify10.c:470:2: error: ‘for’ loop initial declarations are only allowed in C99 mode
for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
^
fanotify10.c:470:11: error: redefinition of ‘piter’
for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
^
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files
2022-11-17 15:58 ` Petr Vorel
@ 2022-11-21 9:14 ` Jan Kara
2022-11-21 9:33 ` Petr Vorel
0 siblings, 1 reply; 24+ messages in thread
From: Jan Kara @ 2022-11-21 9:14 UTC (permalink / raw)
To: Petr Vorel; +Cc: Jan Kara, Richard Palethorpe, ltp
On Thu 17-11-22 16:58:50, Petr Vorel wrote:
> Hi Jan, all,
>
> > +#define foreach_path(tc, buf, pname) \
> > + for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
> Unfortunately we still support C99 due old compiler on CentOS 7,
> therefore int piter needs to be defined outside of for loop.
Hum, but variable declaration in the for loop is part of C99 standard (as
the error message also says). So did you want to say you are compiling
against C89 standard? And CentOS 7 ships with GCC 4.8.5 AFAICS which should
be fully C99 compliant BTW. So what's the situation here?
That being said I can workaround the problem in the macro, it will just be
somewhat uglier. So before doing that I'd like to understand whether
following C89 is really required...
Honza
> fanotify10.c:470:2: error: ‘for’ loop initial declarations are only allowed in C99 mode
> for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
> ^
>
> fanotify10.c:470:11: error: redefinition of ‘piter’
> for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
> ^
> Kind regards,
> Petr
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files
2022-11-21 9:14 ` Jan Kara
@ 2022-11-21 9:33 ` Petr Vorel
2022-11-21 9:39 ` Cyril Hrubis
2022-11-21 9:53 ` Jan Kara
0 siblings, 2 replies; 24+ messages in thread
From: Petr Vorel @ 2022-11-21 9:33 UTC (permalink / raw)
To: Jan Kara; +Cc: Richard Palethorpe, ltp
Hi Jan, all,
> On Thu 17-11-22 16:58:50, Petr Vorel wrote:
> > Hi Jan, all,
> > > +#define foreach_path(tc, buf, pname) \
> > > + for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
> > Unfortunately we still support C99 due old compiler on CentOS 7,
> > therefore int piter needs to be defined outside of for loop.
> Hum, but variable declaration in the for loop is part of C99 standard (as
> the error message also says). So did you want to say you are compiling
> against C89 standard? And CentOS 7 ships with GCC 4.8.5 AFAICS which should
> be fully C99 compliant BTW. So what's the situation here?
I'm sorry, I didn't express clearly myself. Yes, 4.8.5 supports C99,
but the default is C90 [1].
> That being said I can workaround the problem in the macro, it will just be
> somewhat uglier. So before doing that I'd like to understand whether
> following C89 is really required...
I'm don't remember why we have just not specified -std=... already, Cyril had
some objections, thus Cc him.
Cent0S EOL in 2024-06, we might reconsider to add -std=... to endup this agony
(errors like this often need to be fixed).
[1] https://gcc.gnu.org/onlinedocs/gcc-4.8.5/gcc/Standards.html
Kind regards,
Petr
> Honza
> > fanotify10.c:470:2: error: ‘for’ loop initial declarations are only allowed in C99 mode
> > for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
> > ^
> > fanotify10.c:470:11: error: redefinition of ‘piter’
> > for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
> > ^
> > Kind regards,
> > Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files
2022-11-21 9:33 ` Petr Vorel
@ 2022-11-21 9:39 ` Cyril Hrubis
2022-11-22 8:19 ` Petr Vorel
2022-11-21 9:53 ` Jan Kara
1 sibling, 1 reply; 24+ messages in thread
From: Cyril Hrubis @ 2022-11-21 9:39 UTC (permalink / raw)
To: Petr Vorel; +Cc: Jan Kara, Richard Palethorpe, ltp
Hi!
> > > > +#define foreach_path(tc, buf, pname) \
> > > > + for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
> > > Unfortunately we still support C99 due old compiler on CentOS 7,
> > > therefore int piter needs to be defined outside of for loop.
>
> > Hum, but variable declaration in the for loop is part of C99 standard (as
> > the error message also says). So did you want to say you are compiling
> > against C89 standard? And CentOS 7 ships with GCC 4.8.5 AFAICS which should
> > be fully C99 compliant BTW. So what's the situation here?
> I'm sorry, I didn't express clearly myself. Yes, 4.8.5 supports C99,
> but the default is C90 [1].
>
> > That being said I can workaround the problem in the macro, it will just be
> > somewhat uglier. So before doing that I'd like to understand whether
> > following C89 is really required...
>
> I'm don't remember why we have just not specified -std=... already, Cyril had
> some objections, thus Cc him.
I think that at the time there stil was even older compiler we had to
support. I guess that we can add -std=c99 now.
So I would propose adding -std=c99 into CFLAGS and see if anyone would
complain.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files
2022-11-21 9:33 ` Petr Vorel
2022-11-21 9:39 ` Cyril Hrubis
@ 2022-11-21 9:53 ` Jan Kara
2022-11-21 14:24 ` Richard Palethorpe
1 sibling, 1 reply; 24+ messages in thread
From: Jan Kara @ 2022-11-21 9:53 UTC (permalink / raw)
To: Petr Vorel; +Cc: Jan Kara, Richard Palethorpe, ltp
On Mon 21-11-22 10:33:13, Petr Vorel wrote:
> Hi Jan, all,
>
> > On Thu 17-11-22 16:58:50, Petr Vorel wrote:
> > > Hi Jan, all,
>
> > > > +#define foreach_path(tc, buf, pname) \
> > > > + for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
> > > Unfortunately we still support C99 due old compiler on CentOS 7,
> > > therefore int piter needs to be defined outside of for loop.
>
> > Hum, but variable declaration in the for loop is part of C99 standard (as
> > the error message also says). So did you want to say you are compiling
> > against C89 standard? And CentOS 7 ships with GCC 4.8.5 AFAICS which should
> > be fully C99 compliant BTW. So what's the situation here?
> I'm sorry, I didn't express clearly myself. Yes, 4.8.5 supports C99,
> but the default is C90 [1].
OK, thanks for explanation.
> > That being said I can workaround the problem in the macro, it will just be
> > somewhat uglier. So before doing that I'd like to understand whether
> > following C89 is really required...
>
> I'm don't remember why we have just not specified -std=... already, Cyril had
> some objections, thus Cc him.
>
> Cent0S EOL in 2024-06, we might reconsider to add -std=... to endup this agony
> (errors like this often need to be fixed).
>
> [1] https://gcc.gnu.org/onlinedocs/gcc-4.8.5/gcc/Standards.html
Given Cyril's reply, should I rework my patch or are we fine with using
C99?
Honza
> > > fanotify10.c:470:2: error: ‘for’ loop initial declarations are only allowed in C99 mode
> > > for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
> > > ^
>
> > > fanotify10.c:470:11: error: redefinition of ‘piter’
> > > for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
> > > ^
> > > Kind regards,
> > > Petr
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files
2022-11-21 9:53 ` Jan Kara
@ 2022-11-21 14:24 ` Richard Palethorpe
2022-11-22 8:17 ` Petr Vorel
0 siblings, 1 reply; 24+ messages in thread
From: Richard Palethorpe @ 2022-11-21 14:24 UTC (permalink / raw)
To: Jan Kara; +Cc: ltp
Hello,
Jan Kara <jack@suse.cz> writes:
> On Mon 21-11-22 10:33:13, Petr Vorel wrote:
>> Hi Jan, all,
>>
>> > On Thu 17-11-22 16:58:50, Petr Vorel wrote:
>> > > Hi Jan, all,
>>
>> > > > +#define foreach_path(tc, buf, pname) \
>> > > > + for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
>> > > Unfortunately we still support C99 due old compiler on CentOS 7,
>> > > therefore int piter needs to be defined outside of for loop.
>>
>> > Hum, but variable declaration in the for loop is part of C99 standard (as
>> > the error message also says). So did you want to say you are compiling
>> > against C89 standard? And CentOS 7 ships with GCC 4.8.5 AFAICS which should
>> > be fully C99 compliant BTW. So what's the situation here?
>> I'm sorry, I didn't express clearly myself. Yes, 4.8.5 supports C99,
>> but the default is C90 [1].
>
> OK, thanks for explanation.
>
>> > That being said I can workaround the problem in the macro, it will just be
>> > somewhat uglier. So before doing that I'd like to understand whether
>> > following C89 is really required...
>>
>> I'm don't remember why we have just not specified -std=... already, Cyril had
>> some objections, thus Cc him.
>>
>> Cent0S EOL in 2024-06, we might reconsider to add -std=... to endup this agony
>> (errors like this often need to be fixed).
>>
>> [1] https://gcc.gnu.org/onlinedocs/gcc-4.8.5/gcc/Standards.html
>
> Given Cyril's reply, should I rework my patch or are we fine with using
> C99?
Well -std=c99 doesn't work, but we can use -std=gnu99. If that doesn't
fix it then we should drop centos07 now IMO.
>
> Honza
>
>> > > fanotify10.c:470:2: error: ‘for’ loop initial declarations are only allowed in C99 mode
>> > > for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
>> > > ^
>>
>> > > fanotify10.c:470:11: error: redefinition of ‘piter’
>> > > for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
>> > > ^
>> > > Kind regards,
>> > > Petr
--
Thank you,
Richard.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files
2022-11-15 12:47 ` [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files Jan Kara
2022-11-17 15:58 ` Petr Vorel
@ 2022-11-21 15:04 ` Cyril Hrubis
2022-11-22 12:10 ` Richard Palethorpe
1 sibling, 1 reply; 24+ messages in thread
From: Cyril Hrubis @ 2022-11-21 15:04 UTC (permalink / raw)
To: Jan Kara; +Cc: ltp
Hi!
> static void drop_caches(void)
> @@ -482,6 +515,7 @@ static int create_fanotify_groups(unsigned int n)
> int evictable_ignored = (tc->ignore_mark_type == FANOTIFY_EVICTABLE);
> int ignore_mark_type;
> int ignored_onchild = tc->ignored_flags & FAN_EVENT_ON_CHILD;
> + char path[PATH_MAX];
>
> mark = &fanotify_mark_types[tc->mark_type];
> ignore_mark = &fanotify_mark_types[tc->ignore_mark_type];
> @@ -501,11 +535,12 @@ static int create_fanotify_groups(unsigned int n)
> * FAN_EVENT_ON_CHILD has no effect on filesystem/mount
> * or inode mark on non-directory.
> */
> - SAFE_FANOTIFY_MARK(fd_notify[p][i],
> + foreach_path(tc, path, mark_path)
> + SAFE_FANOTIFY_MARK(fd_notify[p][i],
> FAN_MARK_ADD | mark->flag,
> tc->expected_mask_without_ignore |
> FAN_EVENT_ON_CHILD | FAN_ONDIR,
> - AT_FDCWD, tc->mark_path);
> + AT_FDCWD, path);
This is minor, but I would have named the macro FOREACH_PATH() and added
curly braces around the block. And the same for the rest of the
invocations.
> /* Do not add ignore mark for first priority groups */
> if (p == 0)
> @@ -519,9 +554,10 @@ static int create_fanotify_groups(unsigned int n)
> mark_ignored = tst_variant ? FAN_MARK_IGNORE_SURV : FAN_MARK_IGNORED_SURV;
> mask = FAN_OPEN | tc->ignored_flags;
> add_mark:
> - SAFE_FANOTIFY_MARK(fd_notify[p][i],
> + foreach_path(tc, path, ignore_path)
> + SAFE_FANOTIFY_MARK(fd_notify[p][i],
> FAN_MARK_ADD | ignore_mark->flag | mark_ignored,
> - mask, AT_FDCWD, tc->ignore_path);
> + mask, AT_FDCWD, path);
>
> /*
> * FAN_MARK_IGNORE respects FAN_EVENT_ON_CHILD flag, but legacy
> @@ -578,9 +614,24 @@ add_mark:
> if (ignore_mark_type == FAN_MARK_INODE) {
> for (p = 0; p < num_classes; p++) {
> for (i = 0; i < GROUPS_PER_PRIO; i++) {
> - if (fd_notify[p][i] > 0)
> + if (fd_notify[p][i] > 0) {
> + int minexp, maxexp;
> +
> + if (p == 0) {
> + minexp = maxexp = 0;
> + } else if (evictable_ignored) {
> + minexp = 0;
> + /*
> + * Check at least half the
> + * marks get evicted by reclaim
> + */
> + maxexp = tc->ignore_path_cnt / 2;
> + } else {
> + minexp = maxexp = tc->ignore_path_cnt ? : 1;
> + }
> show_fanotify_ignore_marks(fd_notify[p][i],
> - p > 0 && !evictable_ignored);
> + minexp, maxexp);
> + }
> }
> }
> }
> @@ -613,7 +664,7 @@ static void mount_cycle(void)
> bind_mount_created = 1;
> }
>
> -static void verify_event(int p, int group, struct fanotify_event_metadata *event,
> +static int verify_event(int p, int group, struct fanotify_event_metadata *event,
> unsigned long long expected_mask)
> {
> /* Only FAN_REPORT_FID reports the FAN_ONDIR flag in events on dirs */
> @@ -626,33 +677,35 @@ static void verify_event(int p, int group, struct fanotify_event_metadata *event
> (unsigned long long) event->mask,
> (unsigned long long) expected_mask,
> (unsigned int)event->pid, event->fd);
> + return 0;
> } else if (event->pid != child_pid) {
> tst_res(TFAIL, "group %d (%x) got event: mask %llx pid=%u "
> "(expected %u) fd=%u", group, fanotify_class[p],
> (unsigned long long)event->mask, (unsigned int)event->pid,
> (unsigned int)child_pid, event->fd);
> - } else {
> - tst_res(TPASS, "group %d (%x) got event: mask %llx pid=%u fd=%u",
> - group, fanotify_class[p], (unsigned long long)event->mask,
> - (unsigned int)event->pid, event->fd);
> + return 0;
> }
> + return 1;
> }
>
> -static int generate_event(const char *event_path,
> - unsigned long long expected_mask)
> +static int generate_event(struct tcase *tc, unsigned long long expected_mask)
> {
> int fd, status;
>
> child_pid = SAFE_FORK();
>
> if (child_pid == 0) {
> - if (expected_mask & FAN_OPEN_EXEC) {
> - SAFE_EXECL(event_path, event_path, NULL);
> - } else {
> - fd = SAFE_OPEN(event_path, O_RDONLY);
> + char path[PATH_MAX];
> +
> + foreach_path(tc, path, event_path) {
> + if (expected_mask & FAN_OPEN_EXEC) {
> + SAFE_EXECL(path, path, NULL);
> + } else {
> + fd = SAFE_OPEN(path, O_RDONLY);
>
> - if (fd > 0)
> - SAFE_CLOSE(fd);
> + if (fd > 0)
> + SAFE_CLOSE(fd);
> + }
> }
>
> exit(0);
> @@ -665,6 +718,37 @@ static int generate_event(const char *event_path,
> return 0;
> }
>
> +struct fanotify_event_metadata *fetch_event(int fd, int *retp)
> +{
> + int ret;
> + struct fanotify_event_metadata *event;
> +
> + *retp = 0;
> + if (event_buf_pos >= event_buf_len) {
> + event_buf_pos = 0;
> + ret = read(fd, event_buf, EVENT_BUF_LEN);
> + if (ret < 0) {
> + if (errno == EAGAIN)
> + return NULL;
> + tst_brk(TBROK | TERRNO, "reading fanotify events failed");
> + *retp = -1;
If you call tst_brk(TBROK ...) the test exists since that is supposed to
signal unrecoverable error. There is no need to propagate the retp from
this function.
> + return NULL;
> + }
> + event_buf_len = ret;
> + }
> + if (event_buf_len - event_buf_pos < (int)FAN_EVENT_METADATA_LEN) {
> + tst_brk(TBROK,
> + "too short event when reading fanotify events (%d < %d)",
> + event_buf_len - event_buf_pos,
> + (int)FAN_EVENT_METADATA_LEN);
> + *retp = -1;
> + return NULL;
> + }
> + event = (struct fanotify_event_metadata *)(event_buf + event_buf_pos);
> + event_buf_pos += event->event_len;
> + return event;
> +}
> +
> static void test_fanotify(unsigned int n)
> {
> struct tcase *tc = &tcases[n];
> @@ -672,6 +756,7 @@ static void test_fanotify(unsigned int n)
> int ret;
> unsigned int p, i;
> struct fanotify_event_metadata *event;
> + int event_count;
>
> tst_res(TINFO, "Test #%d: %s", n, tc->tname);
>
> @@ -715,7 +800,7 @@ static void test_fanotify(unsigned int n)
> ignore_mark = &fanotify_mark_types[tc->ignore_mark_type];
>
> /* Generate event in child process */
> - if (!generate_event(tc->event_path, tc->expected_mask_with_ignore))
> + if (!generate_event(tc, tc->expected_mask_with_ignore))
> tst_brk(TBROK, "Child process terminated incorrectly");
>
> /* First verify all groups without matching ignore mask got the event */
> @@ -724,64 +809,52 @@ static void test_fanotify(unsigned int n)
> break;
>
> for (i = 0; i < GROUPS_PER_PRIO; i++) {
> - ret = read(fd_notify[p][i], event_buf, EVENT_BUF_LEN);
> - if (ret < 0) {
> - if (errno == EAGAIN) {
> - tst_res(TFAIL, "group %d (%x) "
> - "with %s did not get event",
> - i, fanotify_class[p], mark->name);
> - continue;
> - }
> - tst_brk(TBROK | TERRNO,
> - "reading fanotify events failed");
> - }
> - if (ret < (int)FAN_EVENT_METADATA_LEN) {
> - tst_brk(TBROK,
> - "short read when reading fanotify "
> - "events (%d < %d)", ret,
> - (int)EVENT_BUF_LEN);
> + event_count = 0;
> + event_buf_pos = event_buf_len = 0;
> + while ((event = fetch_event(fd_notify[p][i], &ret))) {
> + event_count++;
> + if (!verify_event(p, i, event, p == 0 ?
> + tc->expected_mask_without_ignore :
> + tc->expected_mask_with_ignore))
> + break;
> + if (event->fd != FAN_NOFD)
> + SAFE_CLOSE(event->fd);
> }
> - event = (struct fanotify_event_metadata *)event_buf;
> - if (ret > (int)event->event_len) {
> + if (ret < 0)
> + continue;
> + if (event_count != (tc->event_path_cnt ? : 1)) {
> tst_res(TFAIL, "group %d (%x) with %s "
> - "got more than one event (%d > %d)",
> - i, fanotify_class[p], mark->name, ret,
> - event->event_len);
> + "got unexpected number of events (%d != %d)",
> + i, fanotify_class[p], mark->name,
> + event_count, tc->event_path_cnt);
> } else {
> - verify_event(p, i, event, p == 0 ?
> - tc->expected_mask_without_ignore :
> - tc->expected_mask_with_ignore);
> + tst_res(TPASS, "group %d (%x) got %d events: mask %llx pid=%u",
> + i, fanotify_class[p], event_count,
> + (unsigned long long)(p == 0 ?
> + tc->expected_mask_without_ignore :
> + tc->expected_mask_with_ignore),
> + (unsigned int)child_pid);
> }
> - if (event->fd != FAN_NOFD)
> - SAFE_CLOSE(event->fd);
> }
> }
> /* Then verify all groups with matching ignore mask did got the event */
> for (p = 1; p < num_classes && !tc->expected_mask_with_ignore; p++) {
> for (i = 0; i < GROUPS_PER_PRIO; i++) {
> - ret = read(fd_notify[p][i], event_buf, EVENT_BUF_LEN);
> - if (ret >= 0 && ret < (int)FAN_EVENT_METADATA_LEN) {
> - tst_brk(TBROK,
> - "short read when reading fanotify "
> - "events (%d < %d)", ret,
> - (int)EVENT_BUF_LEN);
> - }
> - event = (struct fanotify_event_metadata *)event_buf;
> - if (ret > 0) {
> - tst_res(TFAIL, "group %d (%x) with %s and "
> - "%s ignore mask got unexpected event (mask %llx)",
> - i, fanotify_class[p], mark->name, ignore_mark->name,
> - event->mask);
> + event_count = 0;
> + event_buf_pos = event_buf_len = 0;
> + while ((event = fetch_event(fd_notify[p][i], &ret))) {
> + event_count++;
> if (event->fd != FAN_NOFD)
> SAFE_CLOSE(event->fd);
> - } else if (errno == EAGAIN) {
> - tst_res(TPASS, "group %d (%x) with %s and "
> - "%s ignore mask got no event",
> - i, fanotify_class[p], mark->name, ignore_mark->name);
> - } else {
> - tst_brk(TBROK | TERRNO,
> - "reading fanotify events failed");
> }
> + if (ret < 0)
> + continue;
> + if (event_count > tc->event_path_cnt / 2)
> + tst_res(TFAIL, "group %d (%x) with %s and "
> + "%s ignore mask got unexpectedly many events (%d > %d)",
> + i, fanotify_class[p], mark->name,
> + ignore_mark->name, event_count,
> + tc->event_path_cnt / 2);
Apart from the two minor issues I pointed out the rest looks good to me,
but honestly the code is getting way to complicated I would refrain from
adding anything else into the test without some refactoring.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 3/3] fanotify10: Make evictable marks tests more reliable
2022-11-15 12:47 ` [LTP] [PATCH 3/3] fanotify10: Make evictable marks tests more reliable Jan Kara
2022-11-16 2:17 ` Pengfei Xu
@ 2022-11-21 15:09 ` Cyril Hrubis
2022-11-22 10:30 ` Petr Vorel
1 sibling, 1 reply; 24+ messages in thread
From: Cyril Hrubis @ 2022-11-21 15:09 UTC (permalink / raw)
To: Jan Kara; +Cc: ltp
Hi!
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> .../kernel/syscalls/fanotify/fanotify10.c | 62 ++++++++++++++++---
> 1 file changed, 52 insertions(+), 10 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify10.c b/testcases/kernel/syscalls/fanotify/fanotify10.c
> index e19bd907470f..cfbf4c31dd08 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify10.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify10.c
> @@ -86,7 +86,10 @@ static int ignore_mark_unsupported;
> #define TEST_APP "fanotify_child"
> #define TEST_APP2 "fanotify_child2"
> #define DIR_PATH MOUNT_PATH"/"DIR_NAME
> +#define DIR_PATH_MULTI DIR_PATH"%d"
> #define FILE_PATH DIR_PATH"/"FILE_NAME
> +#define FILE_PATH_MULTI FILE_PATH"%d"
> +#define FILE_PATH_MULTIDIR DIR_PATH_MULTI"/"FILE_NAME
> #define FILE2_PATH DIR_PATH"/"FILE2_NAME
> #define SUBDIR_PATH DIR_PATH"/"SUBDIR_NAME
> #define FILE_EXEC_PATH MOUNT_PATH"/"TEST_APP
> @@ -104,6 +107,7 @@ static int old_cache_pressure;
> static pid_t child_pid;
> static int bind_mount_created;
> static unsigned int num_classes = NUM_CLASSES;
> +static int max_file_multi;
>
> enum {
> FANOTIFY_INODE,
> @@ -378,9 +382,11 @@ static struct tcase {
> .tname = "don't ignore mount events created on file with evicted ignore mark",
> .mark_path_fmt = MOUNT_PATH,
> .mark_type = FANOTIFY_MOUNT,
> - .ignore_path_fmt = FILE_PATH,
> + .ignore_path_cnt = 16,
> + .ignore_path_fmt = FILE_PATH_MULTI,
> .ignore_mark_type = FANOTIFY_EVICTABLE,
> - .event_path_fmt = FILE_PATH,
> + .event_path_cnt = 16,
> + .event_path_fmt = FILE_PATH_MULTI,
> .expected_mask_with_ignore = FAN_OPEN,
> .expected_mask_without_ignore = FAN_OPEN
> },
> @@ -388,9 +394,11 @@ static struct tcase {
> .tname = "don't ignore fs events created on a file with evicted ignore mark",
> .mark_path_fmt = MOUNT_PATH,
> .mark_type = FANOTIFY_FILESYSTEM,
> - .ignore_path_fmt = FILE_PATH,
> + .ignore_path_cnt = 16,
> + .ignore_path_fmt = FILE_PATH_MULTI,
> .ignore_mark_type = FANOTIFY_EVICTABLE,
> - .event_path_fmt = FILE_PATH,
> + .event_path_cnt = 16,
> + .event_path_fmt = FILE_PATH_MULTI,
> .expected_mask_with_ignore = FAN_OPEN,
> .expected_mask_without_ignore = FAN_OPEN
> },
> @@ -398,10 +406,12 @@ static struct tcase {
> .tname = "don't ignore mount events created inside a parent with evicted ignore mark",
> .mark_path_fmt = MOUNT_PATH,
> .mark_type = FANOTIFY_MOUNT,
> - .ignore_path_fmt = DIR_PATH,
> + .ignore_path_cnt = 16,
> + .ignore_path_fmt = DIR_PATH_MULTI,
> .ignore_mark_type = FANOTIFY_EVICTABLE,
> .ignored_flags = FAN_EVENT_ON_CHILD,
> - .event_path_fmt = FILE_PATH,
> + .event_path_cnt = 16,
> + .event_path_fmt = FILE_PATH_MULTIDIR,
> .expected_mask_with_ignore = FAN_OPEN,
> .expected_mask_without_ignore = FAN_OPEN
> },
> @@ -409,10 +419,12 @@ static struct tcase {
> .tname = "don't ignore fs events created inside a parent with evicted ignore mark",
> .mark_path_fmt = MOUNT_PATH,
> .mark_type = FANOTIFY_FILESYSTEM,
> - .ignore_path_fmt = DIR_PATH,
> + .ignore_path_cnt = 16,
> + .ignore_path_fmt = DIR_PATH_MULTI,
> .ignore_mark_type = FANOTIFY_EVICTABLE,
> .ignored_flags = FAN_EVENT_ON_CHILD,
> - .event_path_fmt = FILE_PATH,
> + .event_path_cnt = 16,
> + .event_path_fmt = FILE_PATH_MULTIDIR,
> .expected_mask_with_ignore = FAN_OPEN,
> .expected_mask_without_ignore = FAN_OPEN
> },
> @@ -864,6 +876,8 @@ cleanup:
>
> static void setup(void)
> {
> + int i;
> +
> 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);
> @@ -880,7 +894,24 @@ static void setup(void)
> SAFE_MKDIR(DIR_PATH, 0755);
> SAFE_MKDIR(SUBDIR_PATH, 0755);
> SAFE_FILE_PRINTF(FILE_PATH, "1");
> - SAFE_FILE_PRINTF(FILE2_PATH, "1");
> + for (i = 0; i < (int)(sizeof(tcases)/sizeof(tcases[0])); i++) {
^
We do have the standard ARRAY_SIZE()
macro defined in LTP
> + if (tcases[i].mark_path_cnt > max_file_multi)
> + max_file_multi = tcases[i].mark_path_cnt;
> + if (tcases[i].ignore_path_cnt > max_file_multi)
> + max_file_multi = tcases[i].ignore_path_cnt;
> + if (tcases[i].event_path_cnt > max_file_multi)
> + max_file_multi = tcases[i].event_path_cnt;
> + }
> + for (i = 0; i < max_file_multi; i++) {
> + char path[PATH_MAX];
> +
> + sprintf(path, FILE_PATH_MULTI, i);
> + SAFE_FILE_PRINTF(path, "1");
> + sprintf(path, DIR_PATH_MULTI, i);
> + SAFE_MKDIR(path, 0755);
> + sprintf(path, FILE_PATH_MULTIDIR, i);
> + SAFE_FILE_PRINTF(path, "1");
> + }
>
> SAFE_CP(TEST_APP, FILE_EXEC_PATH);
> SAFE_CP(TEST_APP, FILE2_EXEC_PATH);
> @@ -896,6 +927,8 @@ static void setup(void)
>
> static void cleanup(void)
> {
> + int i;
> +
> cleanup_fanotify_groups();
>
> if (bind_mount_created)
> @@ -903,8 +936,17 @@ static void cleanup(void)
>
> SAFE_FILE_PRINTF(CACHE_PRESSURE_FILE, "%d", old_cache_pressure);
>
> + for (i = 0; i < max_file_multi; i++) {
> + char path[PATH_MAX];
> +
> + sprintf(path, FILE_PATH_MULTIDIR, i);
> + SAFE_UNLINK(path);
> + sprintf(path, DIR_PATH_MULTI, i);
> + SAFE_RMDIR(path);
> + sprintf(path, FILE_PATH_MULTI, i);
> + SAFE_UNLINK(path);
> + }
> SAFE_UNLINK(FILE_PATH);
> - SAFE_UNLINK(FILE2_PATH);
> SAFE_RMDIR(SUBDIR_PATH);
> SAFE_RMDIR(DIR_PATH);
> SAFE_RMDIR(MNT2_PATH);
Do we have to unlink anything at all?
As far as I can tell we create these files on a device that is
reformatted after the test anyways.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files
2022-11-21 14:24 ` Richard Palethorpe
@ 2022-11-22 8:17 ` Petr Vorel
2022-11-22 8:57 ` Richard Palethorpe
0 siblings, 1 reply; 24+ messages in thread
From: Petr Vorel @ 2022-11-22 8:17 UTC (permalink / raw)
To: Richard Palethorpe; +Cc: Jan Kara, ltp
> Hello,
> Jan Kara <jack@suse.cz> writes:
> > On Mon 21-11-22 10:33:13, Petr Vorel wrote:
> >> Hi Jan, all,
> >> > On Thu 17-11-22 16:58:50, Petr Vorel wrote:
> >> > > Hi Jan, all,
> >> > > > +#define foreach_path(tc, buf, pname) \
> >> > > > + for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
> >> > > Unfortunately we still support C99 due old compiler on CentOS 7,
> >> > > therefore int piter needs to be defined outside of for loop.
> >> > Hum, but variable declaration in the for loop is part of C99 standard (as
> >> > the error message also says). So did you want to say you are compiling
> >> > against C89 standard? And CentOS 7 ships with GCC 4.8.5 AFAICS which should
> >> > be fully C99 compliant BTW. So what's the situation here?
> >> I'm sorry, I didn't express clearly myself. Yes, 4.8.5 supports C99,
> >> but the default is C90 [1].
> > OK, thanks for explanation.
> >> > That being said I can workaround the problem in the macro, it will just be
> >> > somewhat uglier. So before doing that I'd like to understand whether
> >> > following C89 is really required...
> >> I'm don't remember why we have just not specified -std=... already, Cyril had
> >> some objections, thus Cc him.
> >> Cent0S EOL in 2024-06, we might reconsider to add -std=... to endup this agony
> >> (errors like this often need to be fixed).
> >> [1] https://gcc.gnu.org/onlinedocs/gcc-4.8.5/gcc/Standards.html
> > Given Cyril's reply, should I rework my patch or are we fine with using
> > C99?
> Well -std=c99 doesn't work, but we can use -std=gnu99. If that doesn't
> fix it then we should drop centos07 now IMO.
I'd be ok to put fanotify10: CFLAGS += -std=gnu99 or even CFLAGS += -std=gnu99
(for all tests) into fanotify's Makefile, which fixes the problem.
Unless anybody objects, I can change it before merge.
@Richie: we need to keep Cent0S 7 working until its EOL in 2024-06.
I guess the reason not to specify it in top level Makefile was to have LTP code
being tested on newer standards. Unless there is a good reason for it, I'd vote
for putting -std=gnu99 into top level CFLAGS (and increase it after CentOS 7 EOL).
Kind regards,
Petr
> > Honza
> >> > > fanotify10.c:470:2: error: ‘for’ loop initial declarations are only allowed in C99 mode
> >> > > for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
> >> > > ^
> >> > > fanotify10.c:470:11: error: redefinition of ‘piter’
> >> > > for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
> >> > > ^
> >> > > Kind regards,
> >> > > Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files
2022-11-21 9:39 ` Cyril Hrubis
@ 2022-11-22 8:19 ` Petr Vorel
2022-11-22 10:10 ` Petr Vorel
0 siblings, 1 reply; 24+ messages in thread
From: Petr Vorel @ 2022-11-22 8:19 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: Jan Kara, Richard Palethorpe, ltp
> Hi!
> > > > > +#define foreach_path(tc, buf, pname) \
> > > > > + for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
> > > > Unfortunately we still support C99 due old compiler on CentOS 7,
> > > > therefore int piter needs to be defined outside of for loop.
> > > Hum, but variable declaration in the for loop is part of C99 standard (as
> > > the error message also says). So did you want to say you are compiling
> > > against C89 standard? And CentOS 7 ships with GCC 4.8.5 AFAICS which should
> > > be fully C99 compliant BTW. So what's the situation here?
> > I'm sorry, I didn't express clearly myself. Yes, 4.8.5 supports C99,
> > but the default is C90 [1].
> > > That being said I can workaround the problem in the macro, it will just be
> > > somewhat uglier. So before doing that I'd like to understand whether
> > > following C89 is really required...
> > I'm don't remember why we have just not specified -std=... already, Cyril had
> > some objections, thus Cc him.
> I think that at the time there stil was even older compiler we had to
> support. I guess that we can add -std=c99 now.
> So I would propose adding -std=c99 into CFLAGS and see if anyone would
> complain.
Ah, I overlooked this reply. Cyril, do you mean top level Makefile?
That's what I'd do (going to test it).
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files
2022-11-22 8:17 ` Petr Vorel
@ 2022-11-22 8:57 ` Richard Palethorpe
0 siblings, 0 replies; 24+ messages in thread
From: Richard Palethorpe @ 2022-11-22 8:57 UTC (permalink / raw)
To: Petr Vorel; +Cc: Jan Kara, ltp
Hello,
Petr Vorel <pvorel@suse.cz> writes:
>> Hello,
>
>> Jan Kara <jack@suse.cz> writes:
>
>> > On Mon 21-11-22 10:33:13, Petr Vorel wrote:
>> >> Hi Jan, all,
>
>> >> > On Thu 17-11-22 16:58:50, Petr Vorel wrote:
>> >> > > Hi Jan, all,
>
>> >> > > > +#define foreach_path(tc, buf, pname) \
>> >> > > > + for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
>> >> > > Unfortunately we still support C99 due old compiler on CentOS 7,
>> >> > > therefore int piter needs to be defined outside of for loop.
>
>> >> > Hum, but variable declaration in the for loop is part of C99 standard (as
>> >> > the error message also says). So did you want to say you are compiling
>> >> > against C89 standard? And CentOS 7 ships with GCC 4.8.5 AFAICS which should
>> >> > be fully C99 compliant BTW. So what's the situation here?
>> >> I'm sorry, I didn't express clearly myself. Yes, 4.8.5 supports C99,
>> >> but the default is C90 [1].
>
>> > OK, thanks for explanation.
>
>> >> > That being said I can workaround the problem in the macro, it will just be
>> >> > somewhat uglier. So before doing that I'd like to understand whether
>> >> > following C89 is really required...
>
>> >> I'm don't remember why we have just not specified -std=... already, Cyril had
>> >> some objections, thus Cc him.
>
>> >> Cent0S EOL in 2024-06, we might reconsider to add -std=... to endup this agony
>> >> (errors like this often need to be fixed).
>
>> >> [1] https://gcc.gnu.org/onlinedocs/gcc-4.8.5/gcc/Standards.html
>
>> > Given Cyril's reply, should I rework my patch or are we fine with using
>> > C99?
>
>> Well -std=c99 doesn't work, but we can use -std=gnu99. If that doesn't
>> fix it then we should drop centos07 now IMO.
> I'd be ok to put fanotify10: CFLAGS += -std=gnu99 or even CFLAGS += -std=gnu99
> (for all tests) into fanotify's Makefile, which fixes the problem.
> Unless anybody objects, I can change it before merge.
We definitely shouldn't do that. We've had this issue before and it will
come up again. Then we'll have a bunch of tests with this flag added.
>
> @Richie: we need to keep Cent0S 7 working until its EOL in 2024-06.
It appears though that they have not updated the GCC package since 2014.
It looks like glibc and the kernel receive updates. So it's feasible
someone is running LTP on Centos7. However for SLES versions this old we
(statically) compile on a newer release or install a newer GCC package
etc. Or in this case you could even just added '-std=gnu99' to the make
command on Centos7.
So I have to question whether we should support compilation on targets
this old to the extent that we do? I'd rather try to support more
distro's (e.g. buildroot, nixos) or new compilers (e.g. arocc) that
potentially will help with development.
I don't think we should make it impossible to use older distros, but we
should offload some work onto downstream. The expected result of
sticking with older releases is that backports and tweaks are required.
CentOS Stream 8 is on GCC 8 and Stream 9 has GCC 11, why not put that in
CI?
>
> I guess the reason not to specify it in top level Makefile was to have LTP code
> being tested on newer standards. Unless there is a good reason for it, I'd vote
> for putting -std=gnu99 into top level CFLAGS (and increase it after
> CentOS 7 EOL).
You could merge my patch with this one.
>
> Kind regards,
> Petr
>
>> > Honza
>
>> >> > > fanotify10.c:470:2: error: ‘for’ loop initial declarations are only allowed in C99 mode
>> >> > > for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
>> >> > > ^
>
>> >> > > fanotify10.c:470:11: error: redefinition of ‘piter’
>> >> > > for (int piter = 0; format_path_check((buf), (tc)->pname##_fmt, \
>> >> > > ^
>> >> > > Kind regards,
>> >> > > Petr
--
Thank you,
Richard.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files
2022-11-22 8:19 ` Petr Vorel
@ 2022-11-22 10:10 ` Petr Vorel
0 siblings, 0 replies; 24+ messages in thread
From: Petr Vorel @ 2022-11-22 10:10 UTC (permalink / raw)
To: Cyril Hrubis, Jan Kara, Richard Palethorpe, ltp
Hi all,
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 3/3] fanotify10: Make evictable marks tests more reliable
2022-11-21 15:09 ` Cyril Hrubis
@ 2022-11-22 10:30 ` Petr Vorel
2022-11-22 12:42 ` Cyril Hrubis
0 siblings, 1 reply; 24+ messages in thread
From: Petr Vorel @ 2022-11-22 10:30 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: Jan Kara, ltp
Hi all,
...
> > static void setup(void)
> > {
> > + int i;
> > +
> > 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);
> > @@ -880,7 +894,24 @@ static void setup(void)
> > SAFE_MKDIR(DIR_PATH, 0755);
> > SAFE_MKDIR(SUBDIR_PATH, 0755);
> > SAFE_FILE_PRINTF(FILE_PATH, "1");
> > - SAFE_FILE_PRINTF(FILE2_PATH, "1");
> > + for (i = 0; i < (int)(sizeof(tcases)/sizeof(tcases[0])); i++) {
> ^
> We do have the standard ARRAY_SIZE()
> macro defined in LTP
With this being fixed before merge
Reviewed-by: Petr Vorel <pvorel@suse.cz>
> > + if (tcases[i].mark_path_cnt > max_file_multi)
> > + max_file_multi = tcases[i].mark_path_cnt;
> > + if (tcases[i].ignore_path_cnt > max_file_multi)
> > + max_file_multi = tcases[i].ignore_path_cnt;
> > + if (tcases[i].event_path_cnt > max_file_multi)
> > + max_file_multi = tcases[i].event_path_cnt;
> > + }
> > + for (i = 0; i < max_file_multi; i++) {
> > + char path[PATH_MAX];
> > +
> > + sprintf(path, FILE_PATH_MULTI, i);
> > + SAFE_FILE_PRINTF(path, "1");
> > + sprintf(path, DIR_PATH_MULTI, i);
> > + SAFE_MKDIR(path, 0755);
> > + sprintf(path, FILE_PATH_MULTIDIR, i);
> > + SAFE_FILE_PRINTF(path, "1");
> > + }
> > SAFE_CP(TEST_APP, FILE_EXEC_PATH);
> > SAFE_CP(TEST_APP, FILE2_EXEC_PATH);
> > @@ -896,6 +927,8 @@ static void setup(void)
> > static void cleanup(void)
> > {
> > + int i;
> > +
> > cleanup_fanotify_groups();
> > if (bind_mount_created)
> > @@ -903,8 +936,17 @@ static void cleanup(void)
> > SAFE_FILE_PRINTF(CACHE_PRESSURE_FILE, "%d", old_cache_pressure);
> > + for (i = 0; i < max_file_multi; i++) {
> > + char path[PATH_MAX];
> > +
> > + sprintf(path, FILE_PATH_MULTIDIR, i);
> > + SAFE_UNLINK(path);
> > + sprintf(path, DIR_PATH_MULTI, i);
> > + SAFE_RMDIR(path);
> > + sprintf(path, FILE_PATH_MULTI, i);
> > + SAFE_UNLINK(path);
> > + }
> > SAFE_UNLINK(FILE_PATH);
> > - SAFE_UNLINK(FILE2_PATH);
> > SAFE_RMDIR(SUBDIR_PATH);
> > SAFE_RMDIR(DIR_PATH);
> > SAFE_RMDIR(MNT2_PATH);
> Do we have to unlink anything at all?
> As far as I can tell we create these files on a device that is
> reformatted after the test anyways.
It looks it's not cleared, because tmpdir is removed in do_cleanup(),
which is called just before the end of testing, not for each filesystem:
https://github.com/linux-test-project/ltp/tree/master/lib
Files created for testing in test's setup() are called for each filesystem
(do_test_setup), therefore files would remain created:
fanotify10.c:894: TBROK: mkdir(fs_mnt/testdir, 0755) failed: EEXIST (17)
Summary:
passed 492
failed 0
broken 1
skipped 3
warnings 0
And even if we disable just SAFE_UNLINK(FILE_PATH); we get errors
due our rmdir() implementation wanting directory being empty:
fanotify10.c:953: TWARN: rmdir(fs_mnt/testdir) failed: ENOTEMPTY (39)
fanotify10.c:894: TBROK: mkdir(fs_mnt/testdir, 0755) failed: EEXIST (17)
fanotify10.c:952: TWARN: rmdir(fs_mnt/testdir/testdir2) failed: ENOENT (2)
fanotify10.c:953: TWARN: rmdir(fs_mnt/testdir) failed: ENOTEMPTY (39)
fanotify10.c:954: TWARN: rmdir(mntpoint) failed: ENOENT (2)
IMHO it's safer to remove these files instead of creating them just for first
filesystem. Having unique path for each filesystem (e.g. use filesystem name in
the directory path would result would solve the need to remove files but
probably not worth of implementing.
Kind regards,
Petr
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files
2022-11-21 15:04 ` Cyril Hrubis
@ 2022-11-22 12:10 ` Richard Palethorpe
2022-11-22 12:56 ` Cyril Hrubis
0 siblings, 1 reply; 24+ messages in thread
From: Richard Palethorpe @ 2022-11-22 12:10 UTC (permalink / raw)
To: Cyril Hrubis; +Cc: Jan Kara, ltp
Hello,
Cyril Hrubis <chrubis@suse.cz> writes:
> Hi!
>> static void drop_caches(void)
>> @@ -482,6 +515,7 @@ static int create_fanotify_groups(unsigned int n)
>> int evictable_ignored = (tc->ignore_mark_type == FANOTIFY_EVICTABLE);
>> int ignore_mark_type;
>> int ignored_onchild = tc->ignored_flags & FAN_EVENT_ON_CHILD;
>> + char path[PATH_MAX];
>>
>> mark = &fanotify_mark_types[tc->mark_type];
>> ignore_mark = &fanotify_mark_types[tc->ignore_mark_type];
>> @@ -501,11 +535,12 @@ static int create_fanotify_groups(unsigned int n)
>> * FAN_EVENT_ON_CHILD has no effect on filesystem/mount
>> * or inode mark on non-directory.
>> */
>> - SAFE_FANOTIFY_MARK(fd_notify[p][i],
>> + foreach_path(tc, path, mark_path)
>> + SAFE_FANOTIFY_MARK(fd_notify[p][i],
>> FAN_MARK_ADD | mark->flag,
>> tc->expected_mask_without_ignore |
>> FAN_EVENT_ON_CHILD | FAN_ONDIR,
>> - AT_FDCWD, tc->mark_path);
>> + AT_FDCWD, path);
>
> This is minor, but I would have named the macro FOREACH_PATH() and
> added
This is actually not in line with the kernel style. At least IIRC
foreach macros are lower case in the instances I have seen. This is also
what we have in tst_cgroup.c for e.g.
> curly braces around the block. And the same for the rest of the
> invocations.
At some point checkpatch stopped complaining about this, so it's now the
authors discretion whether to use curly braces in these cases. Unless
there is something wrong with our checkpatch.
IMO there is no mistaking that it is a loop macro with one function call
in the body.
>
>> /* Do not add ignore mark for first priority groups */
>> if (p == 0)
>> @@ -519,9 +554,10 @@ static int create_fanotify_groups(unsigned int n)
>> mark_ignored = tst_variant ? FAN_MARK_IGNORE_SURV : FAN_MARK_IGNORED_SURV;
>> mask = FAN_OPEN | tc->ignored_flags;
>> add_mark:
>> - SAFE_FANOTIFY_MARK(fd_notify[p][i],
>> + foreach_path(tc, path, ignore_path)
>> + SAFE_FANOTIFY_MARK(fd_notify[p][i],
>> FAN_MARK_ADD | ignore_mark->flag | mark_ignored,
>> - mask, AT_FDCWD, tc->ignore_path);
>> + mask, AT_FDCWD, path);
>>
>> /*
>> * FAN_MARK_IGNORE respects FAN_EVENT_ON_CHILD flag, but legacy
>> @@ -578,9 +614,24 @@ add_mark:
>> if (ignore_mark_type == FAN_MARK_INODE) {
>> for (p = 0; p < num_classes; p++) {
>> for (i = 0; i < GROUPS_PER_PRIO; i++) {
>> - if (fd_notify[p][i] > 0)
>> + if (fd_notify[p][i] > 0) {
>> + int minexp, maxexp;
>> +
>> + if (p == 0) {
>> + minexp = maxexp = 0;
>> + } else if (evictable_ignored) {
>> + minexp = 0;
>> + /*
>> + * Check at least half the
>> + * marks get evicted by reclaim
>> + */
>> + maxexp = tc->ignore_path_cnt / 2;
>> + } else {
>> + minexp = maxexp = tc->ignore_path_cnt ? : 1;
>> + }
>> show_fanotify_ignore_marks(fd_notify[p][i],
>> - p > 0 && !evictable_ignored);
>> + minexp, maxexp);
>> + }
>> }
>> }
>> }
>> @@ -613,7 +664,7 @@ static void mount_cycle(void)
>> bind_mount_created = 1;
>> }
>>
>> -static void verify_event(int p, int group, struct fanotify_event_metadata *event,
>> +static int verify_event(int p, int group, struct fanotify_event_metadata *event,
>> unsigned long long expected_mask)
>> {
>> /* Only FAN_REPORT_FID reports the FAN_ONDIR flag in events on dirs */
>> @@ -626,33 +677,35 @@ static void verify_event(int p, int group, struct fanotify_event_metadata *event
>> (unsigned long long) event->mask,
>> (unsigned long long) expected_mask,
>> (unsigned int)event->pid, event->fd);
>> + return 0;
>> } else if (event->pid != child_pid) {
>> tst_res(TFAIL, "group %d (%x) got event: mask %llx pid=%u "
>> "(expected %u) fd=%u", group, fanotify_class[p],
>> (unsigned long long)event->mask, (unsigned int)event->pid,
>> (unsigned int)child_pid, event->fd);
>> - } else {
>> - tst_res(TPASS, "group %d (%x) got event: mask %llx pid=%u fd=%u",
>> - group, fanotify_class[p], (unsigned long long)event->mask,
>> - (unsigned int)event->pid, event->fd);
>> + return 0;
>> }
>> + return 1;
>> }
>>
>> -static int generate_event(const char *event_path,
>> - unsigned long long expected_mask)
>> +static int generate_event(struct tcase *tc, unsigned long long expected_mask)
>> {
>> int fd, status;
>>
>> child_pid = SAFE_FORK();
>>
>> if (child_pid == 0) {
>> - if (expected_mask & FAN_OPEN_EXEC) {
>> - SAFE_EXECL(event_path, event_path, NULL);
>> - } else {
>> - fd = SAFE_OPEN(event_path, O_RDONLY);
>> + char path[PATH_MAX];
>> +
>> + foreach_path(tc, path, event_path) {
>> + if (expected_mask & FAN_OPEN_EXEC) {
>> + SAFE_EXECL(path, path, NULL);
>> + } else {
>> + fd = SAFE_OPEN(path, O_RDONLY);
>>
>> - if (fd > 0)
>> - SAFE_CLOSE(fd);
>> + if (fd > 0)
>> + SAFE_CLOSE(fd);
>> + }
>> }
>>
>> exit(0);
>> @@ -665,6 +718,37 @@ static int generate_event(const char *event_path,
>> return 0;
>> }
>>
>> +struct fanotify_event_metadata *fetch_event(int fd, int *retp)
>> +{
>> + int ret;
>> + struct fanotify_event_metadata *event;
>> +
>> + *retp = 0;
>> + if (event_buf_pos >= event_buf_len) {
>> + event_buf_pos = 0;
>> + ret = read(fd, event_buf, EVENT_BUF_LEN);
>> + if (ret < 0) {
>> + if (errno == EAGAIN)
>> + return NULL;
>> + tst_brk(TBROK | TERRNO, "reading fanotify events failed");
>> + *retp = -1;
>
> If you call tst_brk(TBROK ...) the test exists since that is supposed to
> signal unrecoverable error. There is no need to propagate the retp from
> this function.
Yes although it's confusing for static analysis becuase tst_brk can
return in cleanup code. Also this function needs to be static. So I'll
do this before merge.
Will merge with gnu99 patch, after CI completes, thanks!
--
Thank you,
Richard.
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 3/3] fanotify10: Make evictable marks tests more reliable
2022-11-22 10:30 ` Petr Vorel
@ 2022-11-22 12:42 ` Cyril Hrubis
0 siblings, 0 replies; 24+ messages in thread
From: Cyril Hrubis @ 2022-11-22 12:42 UTC (permalink / raw)
To: Petr Vorel; +Cc: Jan Kara, ltp
Hi!@
> > > + for (i = 0; i < max_file_multi; i++) {
> > > + char path[PATH_MAX];
> > > +
> > > + sprintf(path, FILE_PATH_MULTIDIR, i);
> > > + SAFE_UNLINK(path);
> > > + sprintf(path, DIR_PATH_MULTI, i);
> > > + SAFE_RMDIR(path);
> > > + sprintf(path, FILE_PATH_MULTI, i);
> > > + SAFE_UNLINK(path);
> > > + }
> > > SAFE_UNLINK(FILE_PATH);
> > > - SAFE_UNLINK(FILE2_PATH);
> > > SAFE_RMDIR(SUBDIR_PATH);
> > > SAFE_RMDIR(DIR_PATH);
> > > SAFE_RMDIR(MNT2_PATH);
>
> > Do we have to unlink anything at all?
>
> > As far as I can tell we create these files on a device that is
> > reformatted after the test anyways.
>
> It looks it's not cleared, because tmpdir is removed in do_cleanup(),
> which is called just before the end of testing, not for each filesystem:
>
> https://github.com/linux-test-project/ltp/tree/master/lib
>
> Files created for testing in test's setup() are called for each filesystem
> (do_test_setup), therefore files would remain created:
>
> fanotify10.c:894: TBROK: mkdir(fs_mnt/testdir, 0755) failed: EEXIST (17)
>
> Summary:
> passed 492
> failed 0
> broken 1
> skipped 3
> warnings 0
>
> And even if we disable just SAFE_UNLINK(FILE_PATH); we get errors
> due our rmdir() implementation wanting directory being empty:
>
> fanotify10.c:953: TWARN: rmdir(fs_mnt/testdir) failed: ENOTEMPTY (39)
> fanotify10.c:894: TBROK: mkdir(fs_mnt/testdir, 0755) failed: EEXIST (17)
> fanotify10.c:952: TWARN: rmdir(fs_mnt/testdir/testdir2) failed: ENOENT (2)
> fanotify10.c:953: TWARN: rmdir(fs_mnt/testdir) failed: ENOTEMPTY (39)
> fanotify10.c:954: TWARN: rmdir(mntpoint) failed: ENOENT (2)
>
> IMHO it's safer to remove these files instead of creating them just for first
> filesystem. Having unique path for each filesystem (e.g. use filesystem name in
> the directory path would result would solve the need to remove files but
> probably not worth of implementing.
Hmm, I was commenting under an impression that we create all files on
the device and it looks like that is the case, but we do not run with
.all_filesystems but rather than that with .variants = 2 and of course
we call do_test_setup() for each test variant. So yes we run the test
twice with the same device and the cleanup is required.
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files
2022-11-22 12:10 ` Richard Palethorpe
@ 2022-11-22 12:56 ` Cyril Hrubis
0 siblings, 0 replies; 24+ messages in thread
From: Cyril Hrubis @ 2022-11-22 12:56 UTC (permalink / raw)
To: Richard Palethorpe; +Cc: Jan Kara, ltp
Hi!
> > This is minor, but I would have named the macro FOREACH_PATH() and
> > added
>
> This is actually not in line with the kernel style. At least IIRC
> foreach macros are lower case in the instances I have seen. This is also
> what we have in tst_cgroup.c for e.g.
>
> > curly braces around the block. And the same for the rest of the
> > invocations.
>
> At some point checkpatch stopped complaining about this, so it's now the
> authors discretion whether to use curly braces in these cases. Unless
> there is something wrong with our checkpatch.
I do not think that this was ever enforced by checkpatch.
> IMO there is no mistaking that it is a loop macro with one function call
> in the body.
I tend to put the curly braces everywhere where there is a multiline
macro/function call because I find that way easier on the eyes...
--
Cyril Hrubis
chrubis@suse.cz
--
Mailing list info: https://lists.linux.it/listinfo/ltp
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2022-11-22 12:55 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-15 12:47 [LTP] [PATCH 0/3] Make fanotify10 test yet more reliable Jan Kara
2022-11-15 12:47 ` [LTP] [PATCH 1/3] fanotify10: Use named initializers Jan Kara
2022-11-15 12:47 ` [LTP] [PATCH 2/3] fanotify10: Add support for multiple event files Jan Kara
2022-11-17 15:58 ` Petr Vorel
2022-11-21 9:14 ` Jan Kara
2022-11-21 9:33 ` Petr Vorel
2022-11-21 9:39 ` Cyril Hrubis
2022-11-22 8:19 ` Petr Vorel
2022-11-22 10:10 ` Petr Vorel
2022-11-21 9:53 ` Jan Kara
2022-11-21 14:24 ` Richard Palethorpe
2022-11-22 8:17 ` Petr Vorel
2022-11-22 8:57 ` Richard Palethorpe
2022-11-21 15:04 ` Cyril Hrubis
2022-11-22 12:10 ` Richard Palethorpe
2022-11-22 12:56 ` Cyril Hrubis
2022-11-15 12:47 ` [LTP] [PATCH 3/3] fanotify10: Make evictable marks tests more reliable Jan Kara
2022-11-16 2:17 ` Pengfei Xu
2022-11-16 10:58 ` Jan Kara
2022-11-16 16:32 ` Amir Goldstein
2022-11-17 15:50 ` Petr Vorel
2022-11-21 15:09 ` Cyril Hrubis
2022-11-22 10:30 ` Petr Vorel
2022-11-22 12:42 ` Cyril Hrubis
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.