[PATCH slab hotfixes 0/2] slub kunit tests fixes for 6.12

[PATCH slab hotfixes 0/2] slub kunit tests fixes for 6.12

[Vlastimil Babka]

The SLUB changes for 6.12 included new kunit tests that resulted in
noisy warnings, which we normally suppress, and a boot lockup in some
configurations in case the kunit tests are built-in.

The warnings are addressed in Patch 1.

The lockups I couldn't reproduce, but inspecting boot initialization
order makes me suspect the tests (which call few RCU operations) are
being executed a bit too early before RCU finishes initialization.
Moving the exection later seems to do the trick, so I'd like to ask
kunit folks to ack this change (Patch 2). If RCU folks have any
insights, it would be welcome too.

So these are now fixes for 4e1c44b3db79 ("kunit, slub: add
test_kfree_rcu() and test_leak_destroy()")
Once sent as a full patch, I also want to include comment fixes from
Ulad for kvfree_rcu_queue_batch():
https://lore.kernel.org/all/CA%2BKHdyV%3D0dpJX_v_tcuTQ-_ree-Yb9ch3F_HqfT4YnH8%3DzyWng@mail.gmail.com/

The plan is to take the fixes via slab tree for a 6.12 rcX.

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
Vlastimil Babka (2):
      mm, slab: suppress warnings in test_leak_destroy kunit test
      kunit: move call to kunit_run_all_tests() after rcu_end_inkernel_boot()

 init/main.c      | 4 ++--
 lib/slub_kunit.c | 4 ++--
 mm/slab.h        | 6 ++++++
 mm/slab_common.c | 5 +++--
 mm/slub.c        | 5 +++--
 5 files changed, 16 insertions(+), 8 deletions(-)
---
base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc
change-id: 20240930-b4-slub-kunit-fix-6fba4d1c1742

Best regards,
-- 
Vlastimil Babka <vbabka@suse.cz>

[PATCH slab hotfixes 1/2] mm, slab: suppress warnings in test_leak_destroy kunit test

[Vlastimil Babka]

The test_leak_destroy kunit test intends to test the detection of stray
objects in kmem_cache_destroy(), which normally produces a warning. The
other slab kunit tests suppress the warnings in the kunit test context,
so suppress warnings and related printk output in this test as well.
Automated test running environments then don't need to learn to filter
the warnings.

Also rename the test's kmem_cache, the name was wrongly copy-pasted from
test_kfree_rcu.

Fixes: 4e1c44b3db79 ("kunit, slub: add test_kfree_rcu() and test_leak_destroy()")
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202408251723.42f3d902-oliver.sang@intel.com
Reported-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Closes: https://lore.kernel.org/all/CAB=+i9RHHbfSkmUuLshXGY_ifEZg9vCZi3fqr99+kmmnpDus7Q@mail.gmail.com/
Reported-by: Guenter Roeck <linux@roeck-us.net>
Closes: https://lore.kernel.org/all/6fcb1252-7990-4f0d-8027-5e83f0fb9409@roeck-us.net/
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 lib/slub_kunit.c | 4 ++--
 mm/slab.h        | 6 ++++++
 mm/slab_common.c | 5 +++--
 mm/slub.c        | 5 +++--
 4 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/lib/slub_kunit.c b/lib/slub_kunit.c
index 6e3a1e5a7142f797fe20a28967657f50a466d4ee..85d51ec09846d4fa219db6bda336c6f0b89e98e4 100644
--- a/lib/slub_kunit.c
+++ b/lib/slub_kunit.c
@@ -177,13 +177,13 @@ static void test_kfree_rcu(struct kunit *test)
 
 static void test_leak_destroy(struct kunit *test)
 {
-	struct kmem_cache *s = test_kmem_cache_create("TestSlub_kfree_rcu",
+	struct kmem_cache *s = test_kmem_cache_create("TestSlub_leak_destroy",
 							64, SLAB_NO_MERGE);
 	kmem_cache_alloc(s, GFP_KERNEL);
 
 	kmem_cache_destroy(s);
 
-	KUNIT_EXPECT_EQ(test, 1, slab_errors);
+	KUNIT_EXPECT_EQ(test, 2, slab_errors);
 }
 
 static int test_init(struct kunit *test)
diff --git a/mm/slab.h b/mm/slab.h
index f22fb760b2866124d9d873d28b5a7fa6867aeb90..5cf0fbab0b0554ee70f5371772bc37b290893b8a 100644
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -546,6 +546,12 @@ static inline bool kmem_cache_debug_flags(struct kmem_cache *s, slab_flags_t fla
 	return false;
 }
 
+#if IS_ENABLED(CONFIG_SLUB_DEBUG) && IS_ENABLED(CONFIG_KUNIT)
+bool slab_in_kunit_test(void);
+#else
+static inline bool slab_in_kunit_test(void) { return false; }
+#endif
+
 #ifdef CONFIG_SLAB_OBJ_EXT
 
 /*
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 7443244656150325fb2a7d0158a71821e1418062..3d26c257ed8b57c336ec5c38d4bbd5f5e51d50ff 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -508,8 +508,9 @@ void kmem_cache_destroy(struct kmem_cache *s)
 	kasan_cache_shutdown(s);
 
 	err = __kmem_cache_shutdown(s);
-	WARN(err, "%s %s: Slab cache still has objects when called from %pS",
-	     __func__, s->name, (void *)_RET_IP_);
+	if (!slab_in_kunit_test())
+		WARN(err, "%s %s: Slab cache still has objects when called from %pS",
+		     __func__, s->name, (void *)_RET_IP_);
 
 	list_del(&s->list);
 
diff --git a/mm/slub.c b/mm/slub.c
index 21f71cb6cc06d951a657290421f41170bb3c76cf..5b832512044e3ead8ccde2c02308bd8954246db5 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -827,7 +827,7 @@ static bool slab_add_kunit_errors(void)
 	return true;
 }
 
-static bool slab_in_kunit_test(void)
+bool slab_in_kunit_test(void)
 {
 	struct kunit_resource *resource;
 
@@ -843,7 +843,6 @@ static bool slab_in_kunit_test(void)
 }
 #else
 static inline bool slab_add_kunit_errors(void) { return false; }
-static inline bool slab_in_kunit_test(void) { return false; }
 #endif
 
 static inline unsigned int size_from_object(struct kmem_cache *s)
@@ -5436,6 +5435,8 @@ static void list_slab_objects(struct kmem_cache *s, struct slab *slab,
 	for_each_object(p, s, addr, slab->objects) {
 
 		if (!test_bit(__obj_to_index(s, addr, p), object_map)) {
+			if (slab_add_kunit_errors())
+				continue;
 			pr_err("Object 0x%p @offset=%tu\n", p, p - addr);
 			print_tracking(s, p);
 		}

-- 
2.46.1

Re: [PATCH slab hotfixes 1/2] mm, slab: suppress warnings in test_leak_destroy kunit test

[Hyeonggon Yoo]

On Mon, Sep 30, 2024 at 5:37 PM Vlastimil Babka <vbabka@suse.cz> wrote:
>
> The test_leak_destroy kunit test intends to test the detection of stray
> objects in kmem_cache_destroy(), which normally produces a warning. The
> other slab kunit tests suppress the warnings in the kunit test context,
> so suppress warnings and related printk output in this test as well.
> Automated test running environments then don't need to learn to filter
> the warnings.
>
> Also rename the test's kmem_cache, the name was wrongly copy-pasted from
> test_kfree_rcu.
>
> Fixes: 4e1c44b3db79 ("kunit, slub: add test_kfree_rcu() and test_leak_destroy()")
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202408251723.42f3d902-oliver.sang@intel.com
> Reported-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> Closes: https://lore.kernel.org/all/CAB=+i9RHHbfSkmUuLshXGY_ifEZg9vCZi3fqr99+kmmnpDus7Q@mail.gmail.com/
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Closes: https://lore.kernel.org/all/6fcb1252-7990-4f0d-8027-5e83f0fb9409@roeck-us.net/
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> ---
>  lib/slub_kunit.c | 4 ++--
>  mm/slab.h        | 6 ++++++
>  mm/slab_common.c | 5 +++--
>  mm/slub.c        | 5 +++--
>  4 files changed, 14 insertions(+), 6 deletions(-)

Looks good to me,
Reviewed-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>

> diff --git a/lib/slub_kunit.c b/lib/slub_kunit.c
> index 6e3a1e5a7142f797fe20a28967657f50a466d4ee..85d51ec09846d4fa219db6bda336c6f0b89e98e4 100644
> --- a/lib/slub_kunit.c
> +++ b/lib/slub_kunit.c
> @@ -177,13 +177,13 @@ static void test_kfree_rcu(struct kunit *test)
>
>  static void test_leak_destroy(struct kunit *test)
>  {
> -       struct kmem_cache *s = test_kmem_cache_create("TestSlub_kfree_rcu",
> +       struct kmem_cache *s = test_kmem_cache_create("TestSlub_leak_destroy",
>                                                         64, SLAB_NO_MERGE);
>         kmem_cache_alloc(s, GFP_KERNEL);
>
>         kmem_cache_destroy(s);
>
> -       KUNIT_EXPECT_EQ(test, 1, slab_errors);
> +       KUNIT_EXPECT_EQ(test, 2, slab_errors);
>  }
>
>  static int test_init(struct kunit *test)
> diff --git a/mm/slab.h b/mm/slab.h
> index f22fb760b2866124d9d873d28b5a7fa6867aeb90..5cf0fbab0b0554ee70f5371772bc37b290893b8a 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -546,6 +546,12 @@ static inline bool kmem_cache_debug_flags(struct kmem_cache *s, slab_flags_t fla
>         return false;
>  }
>
> +#if IS_ENABLED(CONFIG_SLUB_DEBUG) && IS_ENABLED(CONFIG_KUNIT)
> +bool slab_in_kunit_test(void);
> +#else
> +static inline bool slab_in_kunit_test(void) { return false; }
> +#endif
> +
>  #ifdef CONFIG_SLAB_OBJ_EXT
>
>  /*
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 7443244656150325fb2a7d0158a71821e1418062..3d26c257ed8b57c336ec5c38d4bbd5f5e51d50ff 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -508,8 +508,9 @@ void kmem_cache_destroy(struct kmem_cache *s)
>         kasan_cache_shutdown(s);
>
>         err = __kmem_cache_shutdown(s);
> -       WARN(err, "%s %s: Slab cache still has objects when called from %pS",
> -            __func__, s->name, (void *)_RET_IP_);
> +       if (!slab_in_kunit_test())
> +               WARN(err, "%s %s: Slab cache still has objects when called from %pS",
> +                    __func__, s->name, (void *)_RET_IP_);
>
>         list_del(&s->list);
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 21f71cb6cc06d951a657290421f41170bb3c76cf..5b832512044e3ead8ccde2c02308bd8954246db5 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -827,7 +827,7 @@ static bool slab_add_kunit_errors(void)
>         return true;
>  }
>
> -static bool slab_in_kunit_test(void)
> +bool slab_in_kunit_test(void)
>  {
>         struct kunit_resource *resource;
>
> @@ -843,7 +843,6 @@ static bool slab_in_kunit_test(void)
>  }
>  #else
>  static inline bool slab_add_kunit_errors(void) { return false; }
> -static inline bool slab_in_kunit_test(void) { return false; }
>  #endif
>
>  static inline unsigned int size_from_object(struct kmem_cache *s)
> @@ -5436,6 +5435,8 @@ static void list_slab_objects(struct kmem_cache *s, struct slab *slab,
>         for_each_object(p, s, addr, slab->objects) {
>
>                 if (!test_bit(__obj_to_index(s, addr, p), object_map)) {
> +                       if (slab_add_kunit_errors())
> +                               continue;
>                         pr_err("Object 0x%p @offset=%tu\n", p, p - addr);
>                         print_tracking(s, p);
>                 }
>
> --
> 2.46.1
>

[PATCH slab hotfixes 2/2] kunit: move call to kunit_run_all_tests() after rcu_end_inkernel_boot()

[Vlastimil Babka]

Guenter Roeck reports that the new slub kunit tests added by commit
4e1c44b3db79 ("kunit, slub: add test_kfree_rcu() and
test_leak_destroy()") cause a lockup on boot on several architectures
when the kunit tests are configured to be built-in and not modules.

These tests invoke kfree_rcu() and kvfree_rcu_barrier() and boot
sequence inspection showed the runner for built-in kunit tests
kunit_run_all_tests() is called before setting system_state to
SYSTEM_RUNNING and calling rcu_end_inkernel_boot(), so this seems like a
likely cause. So while I was unable to reproduce the problem myself,
moving the call to kunit_run_all_tests() a bit later in the boot seems
to have fixed the lockup problem according to Guenter's limited testing.

No kunit tests should be broken by calling the built-in executor a bit
later, as when compiled as modules, they are still executed even later
than this.

Fixes: 4e1c44b3db79 ("kunit, slub: add test_kfree_rcu() and test_leak_destroy()")
Reported-by: Guenter Roeck <linux@roeck-us.net>
Closes: https://lore.kernel.org/all/6fcb1252-7990-4f0d-8027-5e83f0fb9409@roeck-us.net/
Cc: "Paul E. McKenney" <paulmck@kernel.org>
Cc: Boqun Feng <boqun.feng@gmail.com>
Cc: Uladzislau Rezki <urezki@gmail.com>
Cc: rcu@vger.kernel.org
Cc: Brendan Higgins <brendanhiggins@google.com>
Cc: David Gow <davidgow@google.com>
Cc: Rae Moar <rmoar@google.com>
Cc: linux-kselftest@vger.kernel.org
Cc: kunit-dev@googlegroups.com
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 init/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/init/main.c b/init/main.c
index c4778edae7972f512d5eefe8400075ac35a70d1c..7890ebb00e84b8bd7bac28923fb1fe571b3e9ee2 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1489,6 +1489,8 @@ static int __ref kernel_init(void *unused)
 
 	rcu_end_inkernel_boot();
 
+	kunit_run_all_tests();
+
 	do_sysctl_args();
 
 	if (ramdisk_execute_command) {
@@ -1579,8 +1581,6 @@ static noinline void __init kernel_init_freeable(void)
 
 	do_basic_setup();
 
-	kunit_run_all_tests();
-
 	wait_for_initramfs();
 	console_on_rootfs();
 

-- 
2.46.1

Re: [PATCH slab hotfixes 2/2] kunit: move call to kunit_run_all_tests() after rcu_end_inkernel_boot()

[Guenter Roeck]

On 9/30/24 01:37, Vlastimil Babka wrote:
> Guenter Roeck reports that the new slub kunit tests added by commit
> 4e1c44b3db79 ("kunit, slub: add test_kfree_rcu() and
> test_leak_destroy()") cause a lockup on boot on several architectures
> when the kunit tests are configured to be built-in and not modules.
> 
> These tests invoke kfree_rcu() and kvfree_rcu_barrier() and boot
> sequence inspection showed the runner for built-in kunit tests
> kunit_run_all_tests() is called before setting system_state to
> SYSTEM_RUNNING and calling rcu_end_inkernel_boot(), so this seems like a
> likely cause. So while I was unable to reproduce the problem myself,
> moving the call to kunit_run_all_tests() a bit later in the boot seems
> to have fixed the lockup problem according to Guenter's limited testing.
> 
> No kunit tests should be broken by calling the built-in executor a bit
> later, as when compiled as modules, they are still executed even later
> than this.
> 
> Fixes: 4e1c44b3db79 ("kunit, slub: add test_kfree_rcu() and test_leak_destroy()")
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Closes: https://lore.kernel.org/all/6fcb1252-7990-4f0d-8027-5e83f0fb9409@roeck-us.net/
> Cc: "Paul E. McKenney" <paulmck@kernel.org>
> Cc: Boqun Feng <boqun.feng@gmail.com>
> Cc: Uladzislau Rezki <urezki@gmail.com>
> Cc: rcu@vger.kernel.org
> Cc: Brendan Higgins <brendanhiggins@google.com>
> Cc: David Gow <davidgow@google.com>
> Cc: Rae Moar <rmoar@google.com>
> Cc: linux-kselftest@vger.kernel.org
> Cc: kunit-dev@googlegroups.com
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> ---
>   init/main.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/init/main.c b/init/main.c
> index c4778edae7972f512d5eefe8400075ac35a70d1c..7890ebb00e84b8bd7bac28923fb1fe571b3e9ee2 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -1489,6 +1489,8 @@ static int __ref kernel_init(void *unused)
>   
>   	rcu_end_inkernel_boot();
>   
> +	kunit_run_all_tests();
> +
>   	do_sysctl_args();
>   
>   	if (ramdisk_execute_command) {
> @@ -1579,8 +1581,6 @@ static noinline void __init kernel_init_freeable(void)
>   
>   	do_basic_setup();
>   
> -	kunit_run_all_tests();
> -
>   	wait_for_initramfs();
>   	console_on_rootfs();
>   
> 
Unfortunately it doesn't work. With this patch applied, I get many backtraces
similar to the following, and ultimately the image crashes. This is with arm64.
I do not see the problem if I drop this patch.

Guenter

---
[    9.465871]     KTAP version 1
[    9.465964]     # Subtest: iov_iter
[    9.466056]     # module: kunit_iov_iter
[    9.466115]     1..12
[    9.467000] Unable to handle kernel paging request at virtual address ffffc37db5c9f26c
[    9.467244] Mem abort info:
[    9.467332]   ESR = 0x0000000086000007
[    9.467454]   EC = 0x21: IABT (current EL), IL = 32 bits
[    9.467576]   SET = 0, FnV = 0
[    9.467667]   EA = 0, S1PTW = 0
[    9.467762]   FSC = 0x07: level 3 translation fault
[    9.467912] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000042a59000
[    9.468055] [ffffc37db5c9f26c] pgd=0000000000000000, p4d=1000000044b36003, pud=1000000044b37003, pmd=1000000044b3a003, pte=0000000000000000
[    9.469430] Internal error: Oops: 0000000086000007 [#1] PREEMPT SMP
[    9.469687] Modules linked in:
[    9.470035] CPU: 0 UID: 0 PID: 550 Comm: kunit_try_catch Tainted: G                 N 6.12.0-rc1-00005-ga65e3eb58cdb #1
[    9.470290] Tainted: [N]=TEST
[    9.470356] Hardware name: linux,dummy-virt (DT)
[    9.470530] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    9.470656] pc : iov_kunit_copy_to_kvec+0x0/0x334
[    9.471055] lr : kunit_try_run_case+0x6c/0x15c
[    9.471145] sp : ffff800080883de0
[    9.471210] x29: ffff800080883e20 x28: 0000000000000000 x27: 0000000000000000
[    9.471376] x26: 0000000000000000 x25: 0000000000000000 x24: ffff80008000bb68
[    9.471501] x23: ffffc37db3f7093c x22: ffff80008000b940 x21: ffff545847af4c00
[    9.471622] x20: ffff545847cd3940 x19: ffff80008000bb50 x18: 0000000000000006
[    9.471742] x17: 6c61746f7420303a x16: 70696b7320303a6c x15: 0000000000000172
[    9.471863] x14: 0000000000020000 x13: 0000000000000000 x12: ffffc37db6a600c8
[    9.471983] x11: 0000000000000043 x10: 0000000000000043 x9 : 1fffffffffffffff
[    9.472122] x8 : 00000000ffffffff x7 : 000000001040d4fd x6 : ffffc37db70c3810
[    9.472243] x5 : 0000000000000000 x4 : ffffffffc4653600 x3 : 000000003b9ac9ff
[    9.472363] x2 : 0000000000000001 x1 : ffffc37db5c9f26c x0 : ffff80008000bb50
[    9.472572] Call trace:
[    9.472636]  iov_kunit_copy_to_kvec+0x0/0x334
[    9.472740]  kunit_generic_run_threadfn_adapter+0x28/0x4c
[    9.472835]  kthread+0x11c/0x120
[    9.472903]  ret_from_fork+0x10/0x20
[    9.473146] Code: ???????? ???????? ???????? ???????? (????????)
[    9.473505] ---[ end trace 0000000000000000 ]---

Re: [PATCH slab hotfixes 2/2] kunit: move call to kunit_run_all_tests() after rcu_end_inkernel_boot()

[Guenter Roeck]

On 9/30/24 11:50, Guenter Roeck wrote:
> On 9/30/24 01:37, Vlastimil Babka wrote:
>> Guenter Roeck reports that the new slub kunit tests added by commit
>> 4e1c44b3db79 ("kunit, slub: add test_kfree_rcu() and
>> test_leak_destroy()") cause a lockup on boot on several architectures
>> when the kunit tests are configured to be built-in and not modules.
>>
>> These tests invoke kfree_rcu() and kvfree_rcu_barrier() and boot
>> sequence inspection showed the runner for built-in kunit tests
>> kunit_run_all_tests() is called before setting system_state to
>> SYSTEM_RUNNING and calling rcu_end_inkernel_boot(), so this seems like a
>> likely cause. So while I was unable to reproduce the problem myself,
>> moving the call to kunit_run_all_tests() a bit later in the boot seems
>> to have fixed the lockup problem according to Guenter's limited testing.
>>
>> No kunit tests should be broken by calling the built-in executor a bit
>> later, as when compiled as modules, they are still executed even later
>> than this.
>>

Actually, that is wrong.

Turns out kunit_iov_iter (and other kunit tests) are marked __init.
That means those unit tests have to run before the init code is released,
and it actually _is_ harmful to run the tests after rcu_end_inkernel_boot()
because at that time free_initmem() has already been called.

Guenter

>> Fixes: 4e1c44b3db79 ("kunit, slub: add test_kfree_rcu() and test_leak_destroy()")
>> Reported-by: Guenter Roeck <linux@roeck-us.net>
>> Closes: https://lore.kernel.org/all/6fcb1252-7990-4f0d-8027-5e83f0fb9409@roeck-us.net/
>> Cc: "Paul E. McKenney" <paulmck@kernel.org>
>> Cc: Boqun Feng <boqun.feng@gmail.com>
>> Cc: Uladzislau Rezki <urezki@gmail.com>
>> Cc: rcu@vger.kernel.org
>> Cc: Brendan Higgins <brendanhiggins@google.com>
>> Cc: David Gow <davidgow@google.com>
>> Cc: Rae Moar <rmoar@google.com>
>> Cc: linux-kselftest@vger.kernel.org
>> Cc: kunit-dev@googlegroups.com
>> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
>> ---
>>   init/main.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/init/main.c b/init/main.c
>> index c4778edae7972f512d5eefe8400075ac35a70d1c..7890ebb00e84b8bd7bac28923fb1fe571b3e9ee2 100644
>> --- a/init/main.c
>> +++ b/init/main.c
>> @@ -1489,6 +1489,8 @@ static int __ref kernel_init(void *unused)
>>       rcu_end_inkernel_boot();
>> +    kunit_run_all_tests();
>> +
>>       do_sysctl_args();
>>       if (ramdisk_execute_command) {
>> @@ -1579,8 +1581,6 @@ static noinline void __init kernel_init_freeable(void)
>>       do_basic_setup();
>> -    kunit_run_all_tests();
>> -
>>       wait_for_initramfs();
>>       console_on_rootfs();
>>
> Unfortunately it doesn't work. With this patch applied, I get many backtraces
> similar to the following, and ultimately the image crashes. This is with arm64.
> I do not see the problem if I drop this patch.
> 
> Guenter
> 
> ---
> [    9.465871]     KTAP version 1
> [    9.465964]     # Subtest: iov_iter
> [    9.466056]     # module: kunit_iov_iter
> [    9.466115]     1..12
> [    9.467000] Unable to handle kernel paging request at virtual address ffffc37db5c9f26c
> [    9.467244] Mem abort info:
> [    9.467332]   ESR = 0x0000000086000007
> [    9.467454]   EC = 0x21: IABT (current EL), IL = 32 bits
> [    9.467576]   SET = 0, FnV = 0
> [    9.467667]   EA = 0, S1PTW = 0
> [    9.467762]   FSC = 0x07: level 3 translation fault
> [    9.467912] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000042a59000
> [    9.468055] [ffffc37db5c9f26c] pgd=0000000000000000, p4d=1000000044b36003, pud=1000000044b37003, pmd=1000000044b3a003, pte=0000000000000000
> [    9.469430] Internal error: Oops: 0000000086000007 [#1] PREEMPT SMP
> [    9.469687] Modules linked in:
> [    9.470035] CPU: 0 UID: 0 PID: 550 Comm: kunit_try_catch Tainted: G                 N 6.12.0-rc1-00005-ga65e3eb58cdb #1
> [    9.470290] Tainted: [N]=TEST
> [    9.470356] Hardware name: linux,dummy-virt (DT)
> [    9.470530] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    9.470656] pc : iov_kunit_copy_to_kvec+0x0/0x334
> [    9.471055] lr : kunit_try_run_case+0x6c/0x15c
> [    9.471145] sp : ffff800080883de0
> [    9.471210] x29: ffff800080883e20 x28: 0000000000000000 x27: 0000000000000000
> [    9.471376] x26: 0000000000000000 x25: 0000000000000000 x24: ffff80008000bb68
> [    9.471501] x23: ffffc37db3f7093c x22: ffff80008000b940 x21: ffff545847af4c00
> [    9.471622] x20: ffff545847cd3940 x19: ffff80008000bb50 x18: 0000000000000006
> [    9.471742] x17: 6c61746f7420303a x16: 70696b7320303a6c x15: 0000000000000172
> [    9.471863] x14: 0000000000020000 x13: 0000000000000000 x12: ffffc37db6a600c8
> [    9.471983] x11: 0000000000000043 x10: 0000000000000043 x9 : 1fffffffffffffff
> [    9.472122] x8 : 00000000ffffffff x7 : 000000001040d4fd x6 : ffffc37db70c3810
> [    9.472243] x5 : 0000000000000000 x4 : ffffffffc4653600 x3 : 000000003b9ac9ff
> [    9.472363] x2 : 0000000000000001 x1 : ffffc37db5c9f26c x0 : ffff80008000bb50
> [    9.472572] Call trace:
> [    9.472636]  iov_kunit_copy_to_kvec+0x0/0x334
> [    9.472740]  kunit_generic_run_threadfn_adapter+0x28/0x4c
> [    9.472835]  kthread+0x11c/0x120
> [    9.472903]  ret_from_fork+0x10/0x20
> [    9.473146] Code: ???????? ???????? ???????? ???????? (????????)
> [    9.473505] ---[ end trace 0000000000000000 ]---
> 

Re: [PATCH slab hotfixes 2/2] kunit: move call to kunit_run_all_tests() after rcu_end_inkernel_boot()

[Vlastimil Babka]

On 10/1/24 1:55 AM, Guenter Roeck wrote:
> On 9/30/24 11:50, Guenter Roeck wrote:
>> On 9/30/24 01:37, Vlastimil Babka wrote:
>>> Guenter Roeck reports that the new slub kunit tests added by commit
>>> 4e1c44b3db79 ("kunit, slub: add test_kfree_rcu() and
>>> test_leak_destroy()") cause a lockup on boot on several architectures
>>> when the kunit tests are configured to be built-in and not modules.
>>>
>>> These tests invoke kfree_rcu() and kvfree_rcu_barrier() and boot
>>> sequence inspection showed the runner for built-in kunit tests
>>> kunit_run_all_tests() is called before setting system_state to
>>> SYSTEM_RUNNING and calling rcu_end_inkernel_boot(), so this seems like a
>>> likely cause. So while I was unable to reproduce the problem myself,
>>> moving the call to kunit_run_all_tests() a bit later in the boot seems
>>> to have fixed the lockup problem according to Guenter's limited testing.
>>>
>>> No kunit tests should be broken by calling the built-in executor a bit
>>> later, as when compiled as modules, they are still executed even later
>>> than this.
>>>
> 
> Actually, that is wrong.
> 
> Turns out kunit_iov_iter (and other kunit tests) are marked __init.
> That means those unit tests have to run before the init code is released,
> and it actually _is_ harmful to run the tests after rcu_end_inkernel_boot()
> because at that time free_initmem() has already been called.

Oh, guess that explains why the kunit_run_all_tests() executor is called
so suspiciously early. Of course when built as modules, __init has a
different lifetime.

Guess I will just skip the two new tests using kfree_rcu() when the slub
kunit is built-in then. Thanks for testing.

> Guenter
> 
>>> Fixes: 4e1c44b3db79 ("kunit, slub: add test_kfree_rcu() and
>>> test_leak_destroy()")
>>> Reported-by: Guenter Roeck <linux@roeck-us.net>
>>> Closes:
>>> https://lore.kernel.org/all/6fcb1252-7990-4f0d-8027-5e83f0fb9409@roeck-us.net/
>>> Cc: "Paul E. McKenney" <paulmck@kernel.org>
>>> Cc: Boqun Feng <boqun.feng@gmail.com>
>>> Cc: Uladzislau Rezki <urezki@gmail.com>
>>> Cc: rcu@vger.kernel.org
>>> Cc: Brendan Higgins <brendanhiggins@google.com>
>>> Cc: David Gow <davidgow@google.com>
>>> Cc: Rae Moar <rmoar@google.com>
>>> Cc: linux-kselftest@vger.kernel.org
>>> Cc: kunit-dev@googlegroups.com
>>> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
>>> ---
>>>   init/main.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/init/main.c b/init/main.c
>>> index
>>> c4778edae7972f512d5eefe8400075ac35a70d1c..7890ebb00e84b8bd7bac28923fb1fe571b3e9ee2 100644
>>> --- a/init/main.c
>>> +++ b/init/main.c
>>> @@ -1489,6 +1489,8 @@ static int __ref kernel_init(void *unused)
>>>       rcu_end_inkernel_boot();
>>> +    kunit_run_all_tests();
>>> +
>>>       do_sysctl_args();
>>>       if (ramdisk_execute_command) {
>>> @@ -1579,8 +1581,6 @@ static noinline void __init
>>> kernel_init_freeable(void)
>>>       do_basic_setup();
>>> -    kunit_run_all_tests();
>>> -
>>>       wait_for_initramfs();
>>>       console_on_rootfs();
>>>
>> Unfortunately it doesn't work. With this patch applied, I get many
>> backtraces
>> similar to the following, and ultimately the image crashes. This is
>> with arm64.
>> I do not see the problem if I drop this patch.
>>
>> Guenter
>>
>> ---
>> [    9.465871]     KTAP version 1
>> [    9.465964]     # Subtest: iov_iter
>> [    9.466056]     # module: kunit_iov_iter
>> [    9.466115]     1..12
>> [    9.467000] Unable to handle kernel paging request at virtual
>> address ffffc37db5c9f26c
>> [    9.467244] Mem abort info:
>> [    9.467332]   ESR = 0x0000000086000007
>> [    9.467454]   EC = 0x21: IABT (current EL), IL = 32 bits
>> [    9.467576]   SET = 0, FnV = 0
>> [    9.467667]   EA = 0, S1PTW = 0
>> [    9.467762]   FSC = 0x07: level 3 translation fault
>> [    9.467912] swapper pgtable: 4k pages, 48-bit VAs,
>> pgdp=0000000042a59000
>> [    9.468055] [ffffc37db5c9f26c] pgd=0000000000000000,
>> p4d=1000000044b36003, pud=1000000044b37003, pmd=1000000044b3a003,
>> pte=0000000000000000
>> [    9.469430] Internal error: Oops: 0000000086000007 [#1] PREEMPT SMP
>> [    9.469687] Modules linked in:
>> [    9.470035] CPU: 0 UID: 0 PID: 550 Comm: kunit_try_catch Tainted:
>> G                 N 6.12.0-rc1-00005-ga65e3eb58cdb #1
>> [    9.470290] Tainted: [N]=TEST
>> [    9.470356] Hardware name: linux,dummy-virt (DT)
>> [    9.470530] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS
>> BTYPE=--)
>> [    9.470656] pc : iov_kunit_copy_to_kvec+0x0/0x334
>> [    9.471055] lr : kunit_try_run_case+0x6c/0x15c
>> [    9.471145] sp : ffff800080883de0
>> [    9.471210] x29: ffff800080883e20 x28: 0000000000000000 x27:
>> 0000000000000000
>> [    9.471376] x26: 0000000000000000 x25: 0000000000000000 x24:
>> ffff80008000bb68
>> [    9.471501] x23: ffffc37db3f7093c x22: ffff80008000b940 x21:
>> ffff545847af4c00
>> [    9.471622] x20: ffff545847cd3940 x19: ffff80008000bb50 x18:
>> 0000000000000006
>> [    9.471742] x17: 6c61746f7420303a x16: 70696b7320303a6c x15:
>> 0000000000000172
>> [    9.471863] x14: 0000000000020000 x13: 0000000000000000 x12:
>> ffffc37db6a600c8
>> [    9.471983] x11: 0000000000000043 x10: 0000000000000043 x9 :
>> 1fffffffffffffff
>> [    9.472122] x8 : 00000000ffffffff x7 : 000000001040d4fd x6 :
>> ffffc37db70c3810
>> [    9.472243] x5 : 0000000000000000 x4 : ffffffffc4653600 x3 :
>> 000000003b9ac9ff
>> [    9.472363] x2 : 0000000000000001 x1 : ffffc37db5c9f26c x0 :
>> ffff80008000bb50
>> [    9.472572] Call trace:
>> [    9.472636]  iov_kunit_copy_to_kvec+0x0/0x334
>> [    9.472740]  kunit_generic_run_threadfn_adapter+0x28/0x4c
>> [    9.472835]  kthread+0x11c/0x120
>> [    9.472903]  ret_from_fork+0x10/0x20
>> [    9.473146] Code: ???????? ???????? ???????? ???????? (????????)
>> [    9.473505] ---[ end trace 0000000000000000 ]---
>>
> 

Re: [PATCH slab hotfixes 2/2] kunit: move call to kunit_run_all_tests() after rcu_end_inkernel_boot()

[David Gow]

[-- Attachment #1: Type: text/plain, Size: 6302 bytes --]

On Tue, 1 Oct 2024 at 07:55, Guenter Roeck <linux@roeck-us.net> wrote:
>
> On 9/30/24 11:50, Guenter Roeck wrote:
> > On 9/30/24 01:37, Vlastimil Babka wrote:
> >> Guenter Roeck reports that the new slub kunit tests added by commit
> >> 4e1c44b3db79 ("kunit, slub: add test_kfree_rcu() and
> >> test_leak_destroy()") cause a lockup on boot on several architectures
> >> when the kunit tests are configured to be built-in and not modules.
> >>
> >> These tests invoke kfree_rcu() and kvfree_rcu_barrier() and boot
> >> sequence inspection showed the runner for built-in kunit tests
> >> kunit_run_all_tests() is called before setting system_state to
> >> SYSTEM_RUNNING and calling rcu_end_inkernel_boot(), so this seems like a
> >> likely cause. So while I was unable to reproduce the problem myself,
> >> moving the call to kunit_run_all_tests() a bit later in the boot seems
> >> to have fixed the lockup problem according to Guenter's limited testing.
> >>
> >> No kunit tests should be broken by calling the built-in executor a bit
> >> later, as when compiled as modules, they are still executed even later
> >> than this.
> >>
>
> Actually, that is wrong.
>
> Turns out kunit_iov_iter (and other kunit tests) are marked __init.
> That means those unit tests have to run before the init code is released,
> and it actually _is_ harmful to run the tests after rcu_end_inkernel_boot()
> because at that time free_initmem() has already been called.

Yeah: some tests are marked __init. KUnit does actually mark these
with an attribute, so we can potentially split the execution up into
an 'init' part which runs early, and a later part, but there are some
complications if we still want to track the total number of tests and
support filtering, etc. properly.

That's something I think we'll look at for 6.13: in the meantime,
skipping the problematic slub tests when built-in seems to be the
right short-term fix. I'll look into having the built-in executor
moved later for non-init tests once we've worked out how best to adapt
the filter/KTAP output code to do so as cleanly as possible.

Cheers,
-- David


>
> Guenter
>
> >> Fixes: 4e1c44b3db79 ("kunit, slub: add test_kfree_rcu() and test_leak_destroy()")
> >> Reported-by: Guenter Roeck <linux@roeck-us.net>
> >> Closes: https://lore.kernel.org/all/6fcb1252-7990-4f0d-8027-5e83f0fb9409@roeck-us.net/
> >> Cc: "Paul E. McKenney" <paulmck@kernel.org>
> >> Cc: Boqun Feng <boqun.feng@gmail.com>
> >> Cc: Uladzislau Rezki <urezki@gmail.com>
> >> Cc: rcu@vger.kernel.org
> >> Cc: Brendan Higgins <brendanhiggins@google.com>
> >> Cc: David Gow <davidgow@google.com>
> >> Cc: Rae Moar <rmoar@google.com>
> >> Cc: linux-kselftest@vger.kernel.org
> >> Cc: kunit-dev@googlegroups.com
> >> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> >> ---
> >>   init/main.c | 4 ++--
> >>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/init/main.c b/init/main.c
> >> index c4778edae7972f512d5eefe8400075ac35a70d1c..7890ebb00e84b8bd7bac28923fb1fe571b3e9ee2 100644
> >> --- a/init/main.c
> >> +++ b/init/main.c
> >> @@ -1489,6 +1489,8 @@ static int __ref kernel_init(void *unused)
> >>       rcu_end_inkernel_boot();
> >> +    kunit_run_all_tests();
> >> +
> >>       do_sysctl_args();
> >>       if (ramdisk_execute_command) {
> >> @@ -1579,8 +1581,6 @@ static noinline void __init kernel_init_freeable(void)
> >>       do_basic_setup();
> >> -    kunit_run_all_tests();
> >> -
> >>       wait_for_initramfs();
> >>       console_on_rootfs();
> >>
> > Unfortunately it doesn't work. With this patch applied, I get many backtraces
> > similar to the following, and ultimately the image crashes. This is with arm64.
> > I do not see the problem if I drop this patch.
> >
> > Guenter
> >
> > ---
> > [    9.465871]     KTAP version 1
> > [    9.465964]     # Subtest: iov_iter
> > [    9.466056]     # module: kunit_iov_iter
> > [    9.466115]     1..12
> > [    9.467000] Unable to handle kernel paging request at virtual address ffffc37db5c9f26c
> > [    9.467244] Mem abort info:
> > [    9.467332]   ESR = 0x0000000086000007
> > [    9.467454]   EC = 0x21: IABT (current EL), IL = 32 bits
> > [    9.467576]   SET = 0, FnV = 0
> > [    9.467667]   EA = 0, S1PTW = 0
> > [    9.467762]   FSC = 0x07: level 3 translation fault
> > [    9.467912] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000042a59000
> > [    9.468055] [ffffc37db5c9f26c] pgd=0000000000000000, p4d=1000000044b36003, pud=1000000044b37003, pmd=1000000044b3a003, pte=0000000000000000
> > [    9.469430] Internal error: Oops: 0000000086000007 [#1] PREEMPT SMP
> > [    9.469687] Modules linked in:
> > [    9.470035] CPU: 0 UID: 0 PID: 550 Comm: kunit_try_catch Tainted: G                 N 6.12.0-rc1-00005-ga65e3eb58cdb #1
> > [    9.470290] Tainted: [N]=TEST
> > [    9.470356] Hardware name: linux,dummy-virt (DT)
> > [    9.470530] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > [    9.470656] pc : iov_kunit_copy_to_kvec+0x0/0x334
> > [    9.471055] lr : kunit_try_run_case+0x6c/0x15c
> > [    9.471145] sp : ffff800080883de0
> > [    9.471210] x29: ffff800080883e20 x28: 0000000000000000 x27: 0000000000000000
> > [    9.471376] x26: 0000000000000000 x25: 0000000000000000 x24: ffff80008000bb68
> > [    9.471501] x23: ffffc37db3f7093c x22: ffff80008000b940 x21: ffff545847af4c00
> > [    9.471622] x20: ffff545847cd3940 x19: ffff80008000bb50 x18: 0000000000000006
> > [    9.471742] x17: 6c61746f7420303a x16: 70696b7320303a6c x15: 0000000000000172
> > [    9.471863] x14: 0000000000020000 x13: 0000000000000000 x12: ffffc37db6a600c8
> > [    9.471983] x11: 0000000000000043 x10: 0000000000000043 x9 : 1fffffffffffffff
> > [    9.472122] x8 : 00000000ffffffff x7 : 000000001040d4fd x6 : ffffc37db70c3810
> > [    9.472243] x5 : 0000000000000000 x4 : ffffffffc4653600 x3 : 000000003b9ac9ff
> > [    9.472363] x2 : 0000000000000001 x1 : ffffc37db5c9f26c x0 : ffff80008000bb50
> > [    9.472572] Call trace:
> > [    9.472636]  iov_kunit_copy_to_kvec+0x0/0x334
> > [    9.472740]  kunit_generic_run_threadfn_adapter+0x28/0x4c
> > [    9.472835]  kthread+0x11c/0x120
> > [    9.472903]  ret_from_fork+0x10/0x20
> > [    9.473146] Code: ???????? ???????? ???????? ???????? (????????)
> > [    9.473505] ---[ end trace 0000000000000000 ]---
> >
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5294 bytes --]