On Thu, 15 Apr 2021 at 12:38, Vlastimil Babka vbabka@suse.cz wrote:
On 4/15/21 12:10 PM, Oliver Glitta wrote:
ut 13. 4. 2021 o 15:54 Marco Elver elver@google.com napísal(a):
On Tue, 13 Apr 2021 at 12:07, glittao@gmail.com wrote:
From: Oliver Glitta glittao@gmail.com
SLUB has resiliency_test() function which is hidden behind #ifdef SLUB_RESILIENCY_TEST that is not part of Kconfig, so nobody runs it. KUnit should be a proper replacement for it.
Try changing byte in redzone after allocation and changing pointer to next free node, first byte, 50th byte and redzone byte. Check if validation finds errors.
There are several differences from the original resiliency test: Tests create own caches with known state instead of corrupting shared kmalloc caches.
The corruption of freepointer uses correct offset, the original resiliency test got broken with freepointer changes.
Scratch changing random byte test, because it does not have meaning in this form where we need deterministic results.
Add new option CONFIG_SLUB_KUNIT_TEST in Kconfig. Because the test deliberatly modifies non-allocated objects, it depends on !KASAN which would have otherwise prevented that.
Hmm, did the test fail with KASAN? Is it possible to skip the tests and still run a subset of tests with KASAN? It'd be nice if we could run some of these tests with KASAN as well.
Use kunit_resource to count errors in cache and silence bug reports. Count error whenever slab_bug() or slab_fix() is called or when the count of pages is wrong.
Signed-off-by: Oliver Glitta glittao@gmail.com
Reviewed-by: Marco Elver elver@google.com
Thank you.
Thanks, this all looks good to me. But perhaps do test what works with KASAN, to see if you need the !KASAN constraint for all cases.
I tried to run tests with KASAN functionality disabled with function kasan_disable_current() and three of the tests failed with wrong errors counts. So I add the !KASAN constraint for all tests, because the merge window is coming, we want to know if this version is stable and without other mistakes. We will take a closer look at that in the follow-up patch.
Agreed. In this context, KASAN is essentially a different implementation of the same checks that SLUB_DEBUG offers (and also does other checks) and we excercise these SLUB_DEBUG checks by deliberately causing the corruption that they detect
- so instead, KASAN detects it, as it should. I assume that once somebody opts
for a full KASAN kernel build, they don't need the SLUB_DEBUG functionality at that point, as KASAN is more extensive (On the other hand SLUB_DEBUG kernels can be (and are) shipped as production distro kernels where specific targetted debugging can be enabled to help find bugs in production with minimal disruption). So trying to make both cooperate can work only to some extent and for now we've chosen the safer way.
Sounds reasonable. In any case, I'm fine with this version to land and my Reviewed-by above remains valid. :-)
Thanks, -- Marco