| Message ID | 20220905154239.2652169-2-amir73il@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Fanotify tests for FAN_MARK_IGNORE | expand |
Hi Amir, Reviewed-by: Petr Vorel <pvorel@suse.cz> ... > diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c > index 0eb83e2f8..e40916c08 100644 > --- a/testcases/kernel/syscalls/fanotify/fanotify09.c > +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c > @@ -73,12 +73,14 @@ static char event_buf[EVENT_BUF_LEN]; > static int mount_created; > static int fan_report_dfid_unsupported; > +static int ignore_mark_unsupported; > static struct tcase { > const char *tname; > struct fanotify_mark_type mark; > unsigned int ondir; > unsigned int ignore; > + unsigned int ignore_flags; > unsigned int report_name; > const char *close_nowrite; > int nevents; > @@ -88,7 +90,7 @@ static struct tcase { > "Events on non-dir child with both parent and mount marks", > INIT_FANOTIFY_MARK_TYPE(MOUNT), > 0, > - 0, > + 0, 0, > 0, > DIR_NAME, > 1, 0, nit: as number of struct grow, it'd help readability to omit members with 0: .tname = "Events on non-dir child with both parent and mount marks", .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), .close_nowrite = DIR_NAME, .nevents = 1, This is obviously non-blocker of this patch, if you agree it can be done any time later. Also I fixed make check warnings some time ago (i.e. checkpatch.pl), but they got back. I might find a time to fix them (we appreciate you look after the test, thus don't want to bother you with these tiny details). Kind regards, Petr
On Mon, Sep 5, 2022 at 7:04 PM Petr Vorel <pvorel@suse.cz> wrote: > > Hi Amir, > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > ... > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c > > index 0eb83e2f8..e40916c08 100644 > > --- a/testcases/kernel/syscalls/fanotify/fanotify09.c > > +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c > > @@ -73,12 +73,14 @@ static char event_buf[EVENT_BUF_LEN]; > > static int mount_created; > > > static int fan_report_dfid_unsupported; > > +static int ignore_mark_unsupported; > > > static struct tcase { > > const char *tname; > > struct fanotify_mark_type mark; > > unsigned int ondir; > > unsigned int ignore; > > + unsigned int ignore_flags; > > unsigned int report_name; > > const char *close_nowrite; > > int nevents; > > @@ -88,7 +90,7 @@ static struct tcase { > > "Events on non-dir child with both parent and mount marks", > > INIT_FANOTIFY_MARK_TYPE(MOUNT), > > 0, > > - 0, > > + 0, 0, > > 0, > > DIR_NAME, > > 1, 0, > > nit: as number of struct grow, it'd help readability to omit members with 0: > .tname = "Events on non-dir child with both parent and mount marks", > .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), > .close_nowrite = DIR_NAME, > .nevents = 1, > You are absolutely right. The readability of use cases is quite tough at this point. > This is obviously non-blocker of this patch, if you agree it can be done any > time later. Certainly. I think that growing the struct is a good place to stop and do this cleanup, we have got plenty of time until v6.0 and I expect to get more comments on this series, so I will probably do this cleanup already for v2. > > Also I fixed make check warnings some time ago (i.e. checkpatch.pl), but they Cool. I didn't know. > got back. I might find a time to fix them (we appreciate you look after the > test, thus don't want to bother you with these tiny details). > This is the only new check warning I see: fanotify10.c:378: ERROR: Bad function definition - void drop_caches() should probably be void drop_caches(void) make: [../../../../include/mk/rules.mk:56: check-fanotify10] Error 1 (ignored) fanotify10.c:378:25: warning: non-ANSI function declaration of function 'drop_caches' I will fix it. Thanks, Amir.
> On Mon, Sep 5, 2022 at 7:04 PM Petr Vorel <pvorel@suse.cz> wrote: > > Hi Amir, > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > ... > > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c > > > index 0eb83e2f8..e40916c08 100644 > > > --- a/testcases/kernel/syscalls/fanotify/fanotify09.c > > > +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c > > > @@ -73,12 +73,14 @@ static char event_buf[EVENT_BUF_LEN]; > > > static int mount_created; > > > static int fan_report_dfid_unsupported; > > > +static int ignore_mark_unsupported; > > > static struct tcase { > > > const char *tname; > > > struct fanotify_mark_type mark; > > > unsigned int ondir; > > > unsigned int ignore; > > > + unsigned int ignore_flags; > > > unsigned int report_name; > > > const char *close_nowrite; > > > int nevents; > > > @@ -88,7 +90,7 @@ static struct tcase { > > > "Events on non-dir child with both parent and mount marks", > > > INIT_FANOTIFY_MARK_TYPE(MOUNT), > > > 0, > > > - 0, > > > + 0, 0, > > > 0, > > > DIR_NAME, > > > 1, 0, > > nit: as number of struct grow, it'd help readability to omit members with 0: > > .tname = "Events on non-dir child with both parent and mount marks", > > .mark = INIT_FANOTIFY_MARK_TYPE(MOUNT), > > .close_nowrite = DIR_NAME, > > .nevents = 1, > You are absolutely right. > The readability of use cases is quite tough at this point. > > This is obviously non-blocker of this patch, if you agree it can be done any > > time later. > Certainly. > I think that growing the struct is a good place to stop and > do this cleanup, we have got plenty of time until v6.0 and I expect to > get more comments on this series, so I will probably do this cleanup > already for v2. Thank you! FYI we have LTP releases regularly 4x a year [1], the next one will be around the end of September, with git freeze ~ 2 weeks before, i.e. very soon. Of course this patchset should get in the release (I don't expect any issues any major problems reported by Jan or Matthew, checkpatch.pl issues will not block it). > > Also I fixed make check warnings some time ago (i.e. checkpatch.pl), but they > Cool. I didn't know. > > got back. I might find a time to fix them (we appreciate you look after the > > test, thus don't want to bother you with these tiny details). > This is the only new check warning I see: > fanotify10.c:378: ERROR: Bad function definition - void drop_caches() > should probably be void drop_caches(void) > make: [../../../../include/mk/rules.mk:56: check-fanotify10] Error 1 (ignored) > fanotify10.c:378:25: warning: non-ANSI function declaration of > function 'drop_caches' My bad - I fixed it before 0cb281815 ("tools: Check headers with checkpatch.pl") which included the report also for headers. And yes, the rest of (mostly) formatting reports are in fanotify.h. Obviously we haven't found all checks which aren't relevant for userspace, therefore I posted patch ignore some of them in LTP [2]. > I will fix it. Thank you. Kind regards, Petr [1] https://github.com/linux-test-project/ltp/releases [2] https://lore.kernel.org/ltp/20220906054612.9790-1-pvorel@suse.cz/
diff --git a/testcases/kernel/syscalls/fanotify/fanotify.h b/testcases/kernel/syscalls/fanotify/fanotify.h index a118fbd9e..d67c079af 100644 --- a/testcases/kernel/syscalls/fanotify/fanotify.h +++ b/testcases/kernel/syscalls/fanotify/fanotify.h @@ -106,6 +106,12 @@ static inline int safe_fanotify_mark(const char *file, const int lineno, #ifndef FAN_MARK_EVICTABLE #define FAN_MARK_EVICTABLE 0x00000200 #endif +#ifndef FAN_MARK_IGNORE +#define FAN_MARK_IGNORE 0x00000400 +#endif +#ifndef FAN_MARK_IGNORE_SURV +#define FAN_MARK_IGNORE_SURV (FAN_MARK_IGNORE | FAN_MARK_IGNORED_SURV_MODIFY) +#endif /* New dirent event masks */ #ifndef FAN_ATTRIB diff --git a/testcases/kernel/syscalls/fanotify/fanotify09.c b/testcases/kernel/syscalls/fanotify/fanotify09.c index 0eb83e2f8..e40916c08 100644 --- a/testcases/kernel/syscalls/fanotify/fanotify09.c +++ b/testcases/kernel/syscalls/fanotify/fanotify09.c @@ -73,12 +73,14 @@ static char event_buf[EVENT_BUF_LEN]; static int mount_created; static int fan_report_dfid_unsupported; +static int ignore_mark_unsupported; static struct tcase { const char *tname; struct fanotify_mark_type mark; unsigned int ondir; unsigned int ignore; + unsigned int ignore_flags; unsigned int report_name; const char *close_nowrite; int nevents; @@ -88,7 +90,7 @@ static struct tcase { "Events on non-dir child with both parent and mount marks", INIT_FANOTIFY_MARK_TYPE(MOUNT), 0, - 0, + 0, 0, 0, DIR_NAME, 1, 0, @@ -97,7 +99,7 @@ static struct tcase { "Events on non-dir child and subdir with both parent and mount marks", INIT_FANOTIFY_MARK_TYPE(MOUNT), FAN_ONDIR, - 0, + 0, 0, 0, DIR_NAME, 2, 0, @@ -106,7 +108,7 @@ static struct tcase { "Events on non-dir child and parent with both parent and mount marks", INIT_FANOTIFY_MARK_TYPE(MOUNT), FAN_ONDIR, - 0, + 0, 0, 0, ".", 2, 0 @@ -115,7 +117,7 @@ static struct tcase { "Events on non-dir child and subdir with both parent and subdir marks", INIT_FANOTIFY_MARK_TYPE(INODE), FAN_ONDIR, - 0, + 0, 0, 0, DIR_NAME, 2, 0, @@ -124,7 +126,7 @@ static struct tcase { "Events on non-dir children with both parent and mount marks", INIT_FANOTIFY_MARK_TYPE(MOUNT), 0, - 0, + 0, 0, 0, FILE2_NAME, 2, FAN_CLOSE_NOWRITE, @@ -133,7 +135,7 @@ static struct tcase { "Events on non-dir child with both parent and mount marks and filename info", INIT_FANOTIFY_MARK_TYPE(MOUNT), 0, - 0, + 0, 0, FAN_REPORT_DFID_NAME, FILE2_NAME, 2, FAN_CLOSE_NOWRITE, @@ -142,7 +144,7 @@ static struct tcase { "Events on non-dir child with ignore mask on parent", INIT_FANOTIFY_MARK_TYPE(MOUNT), 0, - FAN_MARK_IGNORED_MASK, + FAN_MARK_IGNORED_MASK, 0, 0, DIR_NAME, 1, 0, @@ -151,11 +153,75 @@ static struct tcase { "Events on non-dir children with surviving ignore mask on parent", INIT_FANOTIFY_MARK_TYPE(MOUNT), 0, - FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY, + FAN_MARK_IGNORED_MASK | FAN_MARK_IGNORED_SURV_MODIFY, 0, + 0, + FILE2_NAME, + 2, FAN_CLOSE_NOWRITE, + }, + /* FAN_MARK_IGNORE test cases: */ + { + "Events on dir with ignore mask that does not apply to dirs", + INIT_FANOTIFY_MARK_TYPE(MOUNT), + FAN_ONDIR, + FAN_MARK_IGNORE_SURV, 0, + 0, + ".", + 2, FAN_CLOSE_NOWRITE, + }, + { + "Events on dir with ignore mask that does apply to dirs", + INIT_FANOTIFY_MARK_TYPE(MOUNT), + FAN_ONDIR, + FAN_MARK_IGNORE_SURV, FAN_ONDIR, + 0, + ".", + 2, 0, + }, + { + "Events on child with ignore mask on parent that does not apply to children", + INIT_FANOTIFY_MARK_TYPE(MOUNT), + 0, + FAN_MARK_IGNORE_SURV, 0, + 0, + FILE2_NAME, + 2, FAN_CLOSE_NOWRITE, + }, + { + "Events on child with ignore mask on parent that does apply to children", + INIT_FANOTIFY_MARK_TYPE(MOUNT), + 0, + FAN_MARK_IGNORE_SURV, FAN_EVENT_ON_CHILD, 0, FILE2_NAME, + 2, 0, + }, + { + "Events on subdir with ignore mask on parent that does not apply to children", + INIT_FANOTIFY_MARK_TYPE(MOUNT), + FAN_ONDIR, + FAN_MARK_IGNORE_SURV, FAN_ONDIR, + 0, + DIR_NAME, 2, FAN_CLOSE_NOWRITE, }, + { + "Events on subdir with ignore mask on parent that does not apply to dirs", + INIT_FANOTIFY_MARK_TYPE(MOUNT), + FAN_ONDIR, + FAN_MARK_IGNORE_SURV, FAN_EVENT_ON_CHILD, + 0, + DIR_NAME, + 2, FAN_CLOSE_NOWRITE, + }, + { + "Events on subdir with ignore mask on parent that does apply to subdirs", + INIT_FANOTIFY_MARK_TYPE(MOUNT), + FAN_ONDIR, + FAN_MARK_IGNORE_SURV, FAN_EVENT_ON_CHILD | FAN_ONDIR, + 0, + DIR_NAME, + 2, 0, + }, }; static void create_fanotify_groups(struct tcase *tc) @@ -170,16 +236,19 @@ static void create_fanotify_groups(struct tcase *tc) */ unsigned int report_name = tc->report_name; unsigned int mask_flags = tc->ondir | FAN_EVENT_ON_CHILD; - unsigned int parent_mask, ignore = 0; + unsigned int parent_mask, ignore_mask, ignore = 0; /* - * The non-first groups do not request events on children and - * subdirs and may set an ignore mask on parent dir. + * The non-first groups may request events on children and + * subdirs only when setting an ignore mask on parent dir. + * The parent ignore mask may request to ignore events on + * children or subdirs. */ if (i > 0) { ignore = tc->ignore; report_name = 0; - mask_flags = 0; + if (!ignore) + mask_flags = 0; } fd_notify[i] = SAFE_FANOTIFY_INIT(FAN_CLASS_NOTIF | report_name | @@ -206,8 +275,9 @@ static void create_fanotify_groups(struct tcase *tc) * the close event on dir. */ parent_mask = FAN_MODIFY | tc->ondir | mask_flags; + ignore_mask = FAN_CLOSE_NOWRITE | tc->ignore_flags; SAFE_FANOTIFY_MARK(fd_notify[i], FAN_MARK_ADD | ignore, - ignore ? FAN_CLOSE_NOWRITE : parent_mask, + ignore ? ignore_mask : parent_mask, AT_FDCWD, "."); } } @@ -331,6 +401,11 @@ static void test_fanotify(unsigned int n) return; } + if (ignore_mark_unsupported && tc->ignore & FAN_MARK_IGNORE) { + tst_res(TCONF, "FAN_MARK_IGNORE not supported in kernel?"); + return; + } + create_fanotify_groups(tc); /* @@ -422,6 +497,7 @@ static void setup(void) { fan_report_dfid_unsupported = fanotify_init_flags_supported_on_fs(FAN_REPORT_DFID_NAME, MOUNT_PATH); + ignore_mark_unsupported = fanotify_mark_supported_by_kernel(FAN_MARK_IGNORE_SURV); SAFE_MKDIR(MOUNT_NAME, 0755); SAFE_MOUNT(MOUNT_PATH, MOUNT_NAME, "none", MS_BIND, NULL);
Verify the semantics of the new FAN_MARK_IGNORE flag: 1. Verify that ignore mask ignores events on dir only when FAN_ONDIR flag is set in ignore mask 2. Verify that ignore mask ignores events on child only when FAN_EVENT_ON_CHILD flag is set in ignore mask 3. Verify that ignore mask ignores events on subdir only when both FAN_EVENT_ON_CHILD and FAN_ONDIR flags are set in ignore mask Signed-off-by: Amir Goldstein <amir73il@gmail.com> --- testcases/kernel/syscalls/fanotify/fanotify.h | 6 ++ .../kernel/syscalls/fanotify/fanotify09.c | 102 +++++++++++++++--- 2 files changed, 95 insertions(+), 13 deletions(-)