| Message ID | 20221115124741.14400-3-jack@suse.cz |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Make fanotify10 test yet more reliable | expand |
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 >
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 > >
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.
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.
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.
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
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.
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);
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(-)