diff mbox series

[v2] fanotify23: Make evictable marks tests more reliable

Message ID 20230810175926.6205-1-jack@suse.cz
State Accepted
Headers show
Series [v2] fanotify23: Make evictable marks tests more reliable | expand

Commit Message

Jan Kara Aug. 10, 2023, 5:59 p.m. UTC
It has been observed that when fanotify23 test is run in a loop, it
eventually fails as:

fanotify23.c:112: TPASS: FAN_MARK_ADD failed with EEXIST as expected when trying to downgrade to evictable mark
fanotify23.c:75: TPASS: FAN_MARK_REMOVE failed with ENOENT as expected after empty mask
fanotify23.c:156: TPASS: Got no events as expected
fanotify23.c:81: TFAIL: FAN_MARK_REMOVE did not fail with ENOENT as expected after drop_caches: SUCCESS (0)

This is because repeated evictions of caches done by the test reclaim
all freeable slab objects in the system. So when the test calls drop
caches, only the dentry and inode of the test file are there to reclaim.
But for inode to be reclaimed, dentry (which holds inode reference) has
to be freed first and for dentry to be freed it has to first cycle
through the LRU which takes two slab reclaim calls.

Call drop caches twice to make sure dentry has chance to pass through
the LRU and be reclaimed.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 testcases/kernel/syscalls/fanotify/fanotify23.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Amir Goldstein Aug. 11, 2023, 12:01 p.m. UTC | #1
On Thu, Aug 10, 2023 at 8:59 PM Jan Kara <jack@suse.cz> wrote:
>
> It has been observed that when fanotify23 test is run in a loop, it
> eventually fails as:
>
> fanotify23.c:112: TPASS: FAN_MARK_ADD failed with EEXIST as expected when trying to downgrade to evictable mark
> fanotify23.c:75: TPASS: FAN_MARK_REMOVE failed with ENOENT as expected after empty mask
> fanotify23.c:156: TPASS: Got no events as expected
> fanotify23.c:81: TFAIL: FAN_MARK_REMOVE did not fail with ENOENT as expected after drop_caches: SUCCESS (0)
>
> This is because repeated evictions of caches done by the test reclaim
> all freeable slab objects in the system. So when the test calls drop
> caches, only the dentry and inode of the test file are there to reclaim.
> But for inode to be reclaimed, dentry (which holds inode reference) has
> to be freed first and for dentry to be freed it has to first cycle
> through the LRU which takes two slab reclaim calls.
>
> Call drop caches twice to make sure dentry has chance to pass through
> the LRU and be reclaimed.
>
> Signed-off-by: Jan Kara <jack@suse.cz>

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

> ---
>  testcases/kernel/syscalls/fanotify/fanotify23.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify23.c b/testcases/kernel/syscalls/fanotify/fanotify23.c
> index 89fd4f36a09b..fb812c51e34e 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify23.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify23.c
> @@ -160,10 +160,16 @@ static void test_fanotify(void)
>         }
>
>         /*
> -        * drop_caches should evict inode from cache and remove evictable mark
> +        * drop_caches should evict inode from cache and remove evictable mark.
> +        * We call drop_caches twice as once the dentries will just cycle
> +        * through the LRU without being reclaimed and if there are no other
> +        * objects to reclaim, the slab reclaim will just stop instead of
> +        * retrying. Note that this relies on how reclaim of fs objects work
> +        * for the filesystem but this test is restricted to ext2...
>          */
>         fsync_file(TEST_FILE);
>         SAFE_FILE_PRINTF(DROP_CACHES_FILE, "3");
> +       SAFE_FILE_PRINTF(DROP_CACHES_FILE, "3");
>
>         verify_mark_removed(TEST_FILE, "after drop_caches");
>
> --
> 2.35.3
>
Cyril Hrubis Aug. 11, 2023, 1:43 p.m. UTC | #2
Hi!
Applied, thanks.
diff mbox series

Patch

diff --git a/testcases/kernel/syscalls/fanotify/fanotify23.c b/testcases/kernel/syscalls/fanotify/fanotify23.c
index 89fd4f36a09b..fb812c51e34e 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify23.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify23.c
@@ -160,10 +160,16 @@  static void test_fanotify(void)
 	}
 
 	/*
-	 * drop_caches should evict inode from cache and remove evictable mark
+	 * drop_caches should evict inode from cache and remove evictable mark.
+	 * We call drop_caches twice as once the dentries will just cycle
+	 * through the LRU without being reclaimed and if there are no other
+	 * objects to reclaim, the slab reclaim will just stop instead of
+	 * retrying. Note that this relies on how reclaim of fs objects work
+	 * for the filesystem but this test is restricted to ext2...
 	 */
 	fsync_file(TEST_FILE);
 	SAFE_FILE_PRINTF(DROP_CACHES_FILE, "3");
+	SAFE_FILE_PRINTF(DROP_CACHES_FILE, "3");
 
 	verify_mark_removed(TEST_FILE, "after drop_caches");