Re: [PATCH v2 7/7] kunit, slub: add test_kfree_rcu() and test_leak_destroy()

Uladzislau Rezki
2024-08-09T16:23:58Z

On Wed, Aug 07, 2024 at 12:31:20PM +0200, Vlastimil Babka wrote:
> Add a test that will create cache, allocate one object, kfree_rcu() it
> and attempt to destroy it. As long as the usage of kvfree_rcu_barrier()
> in kmem_cache_destroy() works correctly, there should be no warnings in
> dmesg and the test should pass.
>
> Additionally add a test_leak_destroy() test that leaks an object on
> purpose and verifies that kmem_cache_destroy() catches it.
>
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> ---
> lib/slub_kunit.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/lib/slub_kunit.c b/lib/slub_kunit.c
> index e6667a28c014..6e3a1e5a7142 100644
> --- a/lib/slub_kunit.c
> +++ b/lib/slub_kunit.c
> @@ -5,6 +5,7 @@
> #include <linux/slab.h>
> #include <linux/module.h>
> #include <linux/kernel.h>
> +#include <linux/rcupdate.h>
> #include "../mm/slab.h"
>
> static struct kunit_resource resource;
> @@ -157,6 +158,34 @@ static void test_kmalloc_redzone_access(struct kunit *test)
> kmem_cache_destroy(s);
> }
>
> +struct test_kfree_rcu_struct {
> + struct rcu_head rcu;
> +};
> +
> +static void test_kfree_rcu(struct kunit *test)
> +{
> + struct kmem_cache *s = test_kmem_cache_create("TestSlub_kfree_rcu",
> + sizeof(struct test_kfree_rcu_struct),
> + SLAB_NO_MERGE);
> + struct test_kfree_rcu_struct *p = kmem_cache_alloc(s, GFP_KERNEL);
> +
> + kfree_rcu(p, rcu);
> + kmem_cache_destroy(s);
> +
> + KUNIT_EXPECT_EQ(test, 0, slab_errors);
> +}
> +
>
Thank you for this test case!

I used this series to test _more_ the barrier and came to conclusion that it is
not enough, i.e. i had to extend it to something like below:

<snip>
+ snprintf(name, sizeof(name), "test-slub-%d", current->pid);
+
+ for (i = 0; i < test_loop_count; i++) {
+ s = test_kmem_cache_create(name, sizeof(struct test_kfree_rcu_struct),
+ SLAB_NO_MERGE);
+
+ if (!s)
+ BUG();
+
+ get_random_bytes(&nr_to_alloc, sizeof(nr_to_alloc));
+ nr_to_alloc = nr_to_alloc % 1000000;
+ INIT_LIST_HEAD(&local_head);
+
+ for (j = 0; j < nr_to_alloc; j++) {
+ p = kmem_cache_alloc(s, GFP_KERNEL);
+
+ if (p)
+ list_add(&p->list, &local_head);
+ }
+
+ list_for_each_entry_safe(p, n, &local_head, list)
+ kfree_rcu(p, rcu);
+
+ kmem_cache_destroy(s);
+ }
<snip>

by using this(~11 parallel jobs) i could trigger a warning that a freed
cache still has some objects and i have already figured out why. I will
send a v2 of barrier implementation with a fix.

Thanks!

--
Uladzislau Rezki