Re: [PATCH v5 8/8] slub_kunit: extend the test for kfree_rcu_nolock()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing synchronization for deferred RCU frees causes KUnit test suite contamination and false negatives.
--

--- Patch [8]: [PATCH v5 8/8] slub_kunit: extend the test for kfree_rcu_nolock() ---
commit 975207c355ab3b2db3420df72699cb12e4769b5e
Author: Harry Yoo (Oracle) <harry@xxxxxxxxxx>

slub_kunit: extend the test for kfree_rcu_nolock()
    
When slub_kunit is not built-in, call kfree_rcu() and kfree_rcu_nolock()
to test kfree_rcu_nolock() in slub_kunit.
    
Rename the test case as the test covers more _nolock() APIs.

> diff --git a/lib/tests/slub_kunit.c b/lib/tests/slub_kunit.c
> index 8c2b9911471eb..e3b63f0338d5f 100644
> --- a/lib/tests/slub_kunit.c
> +++ b/lib/tests/slub_kunit.c
[ ... ]
> @@ -397,14 +410,14 @@ static void disable_perf_events(struct test_nolock_context *ctx)
>  	perf_event_release_kernel(ctx->event);
>  }
>  
> -static void test_kmalloc_kfree_nolock_perf(struct kunit *test)
> +static void test_kmalloc_nolock_and_friends_perf(struct kunit *test)
>  {
>  	struct test_nolock_context ctx = { .test = test };
>  
>  	if (!enable_perf_events(&ctx))
>  		kunit_skip(test, "Failed to enable perf event, skipping");
>  
> -	test_kmalloc_kfree();
> +	test_kmalloc_and_friends();
>  
>  	disable_perf_events(&ctx);
>  	KUNIT_EXPECT_EQ(test, 0, slab_errors);

[Severity: Medium]
Will this evaluate slab_errors before all asynchronous frees complete?

Since test_kmalloc_and_friends() queues objects via kfree_rcu() and
kfree_rcu_nolock(), the callbacks could execute in softirq context later.
If they interrupt a subsequent KUnit test, slab_add_kunit_errors() could
attribute any SLUB errors from these deferred frees to the next test, causing
it to falsely fail while causing this test to falsely pass.

Should we add a synchronization call like kvfree_rcu_barrier() before
checking slab_errors?

[ ... ]
> @@ -438,14 +451,14 @@ static void unregister_slab_kprobes(struct test_nolock_context *ctx)
>  	unregister_kprobe(&ctx->kprobe);
>  }
>  
> -static void test_kmalloc_kfree_nolock_kprobe(struct kunit *test)
> +static void test_kmalloc_nolock_and_friends_kprobe(struct kunit *test)
>  {
>  	struct test_nolock_context ctx = { .test = test };
>  
>  	if (!register_slab_kprobes(&ctx))
>  		kunit_skip(test, "Failed to register kprobe, skipping");
>  
> -	test_kmalloc_kfree();
> +	test_kmalloc_and_friends();
>  
>  	unregister_slab_kprobes(&ctx);
>  	KUNIT_EXPECT_EQ(test, 0, slab_errors);

[Severity: Medium]
Does this test case need synchronization for the same reason?

Without a barrier before evaluating slab_errors, deferred RCU frees from
test_kmalloc_and_friends() might leak into the next KUnit test and cause
false test failures.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-kfree_rcu_nolock-v5-0-a28cdcda9673@xxxxxxxxxx?part=8