| Message ID | 20250228121356.336871-1-urezki@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/2] kunit, slub: Add test_kfree_rcu_wq_destroy use case | expand |
On 2/28/25 13:13, Uladzislau Rezki (Sony) wrote: > Add a test_kfree_rcu_wq_destroy test to verify a kmem_cache_destroy() > from a workqueue context. The problem is that, before destroying any > cache the kvfree_rcu_barrier() is invoked to guarantee that in-flight > freed objects are flushed. > > The _barrier() function queues and flushes its own internal workers > which might conflict with a workqueue type a kmem-cache gets destroyed > from. > > One example is when a WQ_MEM_RECLAIM workqueue is flushing !WQ_MEM_RECLAIM > events which leads to a kernel splat. See the check_flush_dependency() in > the workqueue.c file. > > If this test does not emits any kernel warning, it is passed. Well the workqueue warning doesn't seem to make the test fail. But someone will notice the warning, so that should be enough. We can't instrument warnings in other subsystem's code for slub kunit context anyway. It would have to be a generic kunit's hook for all warns. > Reviewed-by: Keith Busch <kbusch@kernel.org> > Co-developed-by: Vlastimil Babka <vbabka@suse.cz> > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> Pushed to slab/for-next, thanks.
On Fri, Feb 28, 2025 at 04:49:24PM +0100, Vlastimil Babka wrote: > On 2/28/25 13:13, Uladzislau Rezki (Sony) wrote: > > Add a test_kfree_rcu_wq_destroy test to verify a kmem_cache_destroy() > > from a workqueue context. The problem is that, before destroying any > > cache the kvfree_rcu_barrier() is invoked to guarantee that in-flight > > freed objects are flushed. > > > > The _barrier() function queues and flushes its own internal workers > > which might conflict with a workqueue type a kmem-cache gets destroyed > > from. > > > > One example is when a WQ_MEM_RECLAIM workqueue is flushing !WQ_MEM_RECLAIM > > events which leads to a kernel splat. See the check_flush_dependency() in > > the workqueue.c file. > > > > If this test does not emits any kernel warning, it is passed. > > Well the workqueue warning doesn't seem to make the test fail. But someone > will notice the warning, so that should be enough. We can't instrument > warnings in other subsystem's code for slub kunit context anyway. It would > have to be a generic kunit's hook for all warns. > I agree. > > Reviewed-by: Keith Busch <kbusch@kernel.org> > > Co-developed-by: Vlastimil Babka <vbabka@suse.cz> > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> > > Pushed to slab/for-next, thanks. > Thanks! -- Uladzislau Rezki
diff --git a/lib/slub_kunit.c b/lib/slub_kunit.c index f11691315c2f..d47c472b0520 100644 --- a/lib/slub_kunit.c +++ b/lib/slub_kunit.c @@ -6,6 +6,7 @@ #include <linux/module.h> #include <linux/kernel.h> #include <linux/rcupdate.h> +#include <linux/delay.h> #include "../mm/slab.h" static struct kunit_resource resource; @@ -181,6 +182,63 @@ static void test_kfree_rcu(struct kunit *test) KUNIT_EXPECT_EQ(test, 0, slab_errors); } +struct cache_destroy_work { + struct work_struct work; + struct kmem_cache *s; +}; + +static void cache_destroy_workfn(struct work_struct *w) +{ + struct cache_destroy_work *cdw; + + cdw = container_of(w, struct cache_destroy_work, work); + kmem_cache_destroy(cdw->s); +} + +#define KMEM_CACHE_DESTROY_NR 10 + +static void test_kfree_rcu_wq_destroy(struct kunit *test) +{ + struct test_kfree_rcu_struct *p; + struct cache_destroy_work cdw; + struct workqueue_struct *wq; + struct kmem_cache *s; + unsigned int delay; + int i; + + if (IS_BUILTIN(CONFIG_SLUB_KUNIT_TEST)) + kunit_skip(test, "can't do kfree_rcu() when test is built-in"); + + INIT_WORK_ONSTACK(&cdw.work, cache_destroy_workfn); + wq = alloc_workqueue("test_kfree_rcu_destroy_wq", + WQ_HIGHPRI | WQ_UNBOUND | WQ_MEM_RECLAIM, 0); + + if (!wq) + kunit_skip(test, "failed to alloc wq"); + + for (i = 0; i < KMEM_CACHE_DESTROY_NR; i++) { + s = test_kmem_cache_create("TestSlub_kfree_rcu_wq_destroy", + sizeof(struct test_kfree_rcu_struct), + SLAB_NO_MERGE); + + if (!s) + kunit_skip(test, "failed to create cache"); + + delay = get_random_u8(); + p = kmem_cache_alloc(s, GFP_KERNEL); + kfree_rcu(p, rcu); + + cdw.s = s; + + msleep(delay); + queue_work(wq, &cdw.work); + flush_work(&cdw.work); + } + + destroy_workqueue(wq); + KUNIT_EXPECT_EQ(test, 0, slab_errors); +} + static void test_leak_destroy(struct kunit *test) { struct kmem_cache *s = test_kmem_cache_create("TestSlub_leak_destroy", @@ -254,6 +312,7 @@ static struct kunit_case test_cases[] = { KUNIT_CASE(test_clobber_redzone_free), KUNIT_CASE(test_kmalloc_redzone_access), KUNIT_CASE(test_kfree_rcu), + KUNIT_CASE(test_kfree_rcu_wq_destroy), KUNIT_CASE(test_leak_destroy), KUNIT_CASE(test_krealloc_redzone_zeroing), {}