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

[Uladzislau Rezki <urezki@gmail.com>]

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