Message-ID: <e7d0ca75-82ce-4079-9426-e82e83e38621@suse.cz>
Date: Sat, 14 Sep 2024 20:39:38 +0200
From: Vlastimil Babka <vbabka@...e.cz>
To: Hyeonggon Yoo <42.hyeyoo@...il.com>
Cc: "Paul E. McKenney" <paulmck@...nel.org>,
 Joel Fernandes <joel@...lfernandes.org>,
 Josh Triplett <josh@...htriplett.org>, Boqun Feng <boqun.feng@...il.com>,
 Christoph Lameter <cl@...ux.com>, David Rientjes <rientjes@...gle.com>,
 Steven Rostedt <rostedt@...dmis.org>,
 Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 Lai Jiangshan <jiangshanlai@...il.com>, Zqiang <qiang.zhang1211@...il.com>,
 Julia Lawall <Julia.Lawall@...ia.fr>, Jakub Kicinski <kuba@...nel.org>,
 "Jason A. Donenfeld" <Jason@...c4.com>,
 "Uladzislau Rezki (Sony)" <urezki@...il.com>,
 Andrew Morton <akpm@...ux-foundation.org>,
 Roman Gushchin <roman.gushchin@...ux.dev>, linux-mm@...ck.org,
 linux-kernel@...r.kernel.org, rcu@...r.kernel.org,
 Alexander Potapenko <glider@...gle.com>, Marco Elver <elver@...gle.com>,
 Dmitry Vyukov <dvyukov@...gle.com>, kasan-dev@...glegroups.com,
 Jann Horn <jannh@...gle.com>, Mateusz Guzik <mjguzik@...il.com>
Subject: Re: [PATCH v2 7/7] kunit, slub: add test_kfree_rcu() and
 test_leak_destroy()

On 9/14/24 15:22, Hyeonggon Yoo wrote:
> On Wed, Aug 7, 2024 at 7:31 PM Vlastimil Babka <vbabka@...e.cz> 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@...e.cz>
>> ---
>>  lib/slub_kunit.c | 31 +++++++++++++++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>>
> 
> Hi Vlastimil,
> 
> I think we might need to suppress the WARN() due to the active objects
> in kmem_cache_destroy()
> when it's called from slub_kunit. With this change, the warning below
> will be printed every time
> slub_kunit is loaded, which made me wonder if there's a bug (for a while).
> 
> Actually, SLUB calls pr_err() is called by __kmem_cache_shutdown() if
> there are any active objects
> during destruction, and the error message is suppressed by slub_kunit.
> However, kmem_cache_destroy()
> still calls WARN() regardless if there is any error during shutdown.

Yeah, there was a LKP report about it already and I wanted to handle this
but forgot. It's not wrong to produce warnings during the tests, for example
the KASAN tests generate tons of them. But it's true that we suppress them
for slub and should continue so for consistency and not having to teach lkp
what can be ignored.

But I think it's fine if we add the suppressing during the rc stabilization
phase so will send the PR for merge window as it is, too late now.

Want to take a stab at the patch? :)

Vlastimil

> [  147.546531] Object 0x00000000c09342ca @offset=640
> [  147.546542] ------------[ cut here ]------------
> [  147.546544] kmem_cache_destroy TestSlub_kfree_rcu: Slab cache still
> has objects when called from test_leak_destroy+0x74/0x108 [slub_kunit]
> [  147.546579] WARNING: CPU: 5 PID: 39703 at mm/slab_common.c:507
> kmem_cache_destroy+0x174/0x188
> [  147.546587] Modules linked in: slub_kunit uinput snd_seq_dummy
> snd_hrtimer rfkill nf_conntrack_netbios_ns nf_conntrack_broadcast
> nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet
> nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct sunrpc nft_chain_nat
> nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 ip_set nf_tables
> nfnetlink qrtr binfmt_misc vfat fat snd_hda_codec_generic
> snd_hda_intel snd_intel_dspcfg snd_hda_codec uvcvideo snd_hda_core uvc
> snd_hwdep videobuf2_vmalloc snd_seq videobuf2_memops snd_seq_device
> videobuf2_v4l2 snd_pcm videobuf2_common snd_timer snd videodev mc
> soundcore virtio_balloon acpi_tad joydev loop zram virtio_gpu
> ahci_platform libahci_platform virtio_dma_buf crct10dif_ce polyval_ce
> polyval_generic ghash_ce sha3_ce virtio_net sha512_ce net_failover
> sha512_arm64 failover virtio_mmio ip6_tables ip_tables fuse
> [  147.546646] CPU: 5 UID: 0 PID: 39703 Comm: kunit_try_catch Tainted:
> G                 N 6.11.0-rc7-next-20240912 #2
> [  147.546649] Tainted: [N]=TEST
> [  147.546650] Hardware name: Parallels International GmbH. Parallels
> ARM Virtual Machine/Parallels ARM Virtual Platform, BIOS 20.0.0
> (55653) Thu, 05 Sep 202
> [  147.546652] pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> [  147.546655] pc : kmem_cache_destroy+0x174/0x188
> [  147.546657] lr : kmem_cache_destroy+0x174/0x188
> [  147.546659] sp : ffff80008aba3d60
> [  147.546660] x29: ffff80008aba3d60 x28: 0000000000000000 x27: 0000000000000000
> [  147.546662] x26: 0000000000000000 x25: 0000000000000000 x24: ffff800094a2b438
> [  147.546665] x23: ffff80008089b750 x22: 0000000000000001 x21: f9cc80007c1782f4
> [  147.546666] x20: ffff800082f9d088 x19: ffff0000c2308b00 x18: 00000000fffffffd
> [  147.546668] x17: 0000000046d4ed9c x16: 00000000ae1ad4db x15: ffff80008aba3430
> [  147.546670] x14: 0000000000000001 x13: ffff80008aba3657 x12: ffff800082f0f060
> [  147.546679] x11: 0000000000000001 x10: 0000000000000001 x9 : ffff8000801652c8
> [  147.546682] x8 : c0000000ffffdfff x7 : ffff800082e5ee68 x6 : 00000000000affa8
> [  147.546684] x5 : ffff00031fc70448 x4 : 0000000000000000 x3 : ffff80029d6b7000
> [  147.546686] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff00011f1c8000
> [  147.546688] Call trace:
> [  147.546689]  kmem_cache_destroy+0x174/0x188
> [  147.546692]  test_leak_destroy+0x74/0x108 [slub_kunit]
> [  147.546693]  kunit_try_run_case+0x74/0x170
> [  147.546697]  kunit_generic_run_threadfn_adapter+0x30/0x60
> [  147.546699]  kthread+0xf4/0x108
> [  147.546705]  ret_from_fork+0x10/0x20
> [  147.546710] ---[ end trace 0000000000000000 ]---

Powered by blists - more mailing lists