Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[Feng Tang]

Hi Thomson,

Thanks for testing!

+ mips maintainer and mail list. The original report is here

https://lore.kernel.org/lkml/becf2ac3-2a90-4f3a-96d9-a70f67c66e4a@app.fastmail.com/

On Tue, Nov 01, 2022 at 12:18:19AM +0000, John Thomson wrote:
> > As I can't reproduce it locally yet, could you help try 3 tests separately:
> > * change the O2/O3 compile option to O1
> > * try the attached 0001 patch (which cut part of commit)
> > * try attached 0001+0002 patch
> 
> None of these changed my outcome.

0001 patch chops off most of the functional changes, and it still
fails. I'm wondering if it is related to the function arguments'
passing, I noticed this is a 32 bits MIPS platform (from your config)   

> I also tried compiling the same linux tree & config with the Bootlin toolchain
> (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0)
> with the same results.
> I will look into finding or building a mips clang toolchain.
 
Yes, Hyeonggon's suggestion makes sense, different compiler may
show some difference.

> No JTAG capability to debug, sorry.
> 
> I get the same outcome with either the ZBOOT vmlinuz, or vmlinux
> 
> Same happening with 6.1-rc3
> 
> 
> After some blind poking around changing (how much of the commit affected) mm/slub.c,
> I may have got lucky. it appears as though this is all I need to boot:
> (against 6.1-rc3), and with the Bootlin toolchain. Will test my other build system as well.
> 
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -3276,7 +3276,7 @@ static void *__slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,
>         c = slub_get_cpu_ptr(s->cpu_slab);
>  #endif
>  
> -       p = ___slab_alloc(s, gfpflags, node, addr, c, orig_size);
> +       p = ___slab_alloc(s, gfpflags, node, addr, c, 0);

___slab_alloc()'s argument number has been changed from 5 to 6, and
some others from 4 to 5.

Thanks,
Feng


>  #ifdef CONFIG_PREEMPT_COUNT
>         slub_put_cpu_ptr(s->cpu_slab);
>  #endif
 

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[John Thomson]

On Tue, 1 Nov 2022, at 07:57, Feng Tang wrote:
> Hi Thomson,
>
> Thanks for testing!
>
> + mips maintainer and mail list. The original report is here
>
> https://lore.kernel.org/lkml/becf2ac3-2a90-4f3a-96d9-a70f67c66e4a@app.fastmail.com/



I am guessing my issue comes from __kmem_cache_alloc_lru accessing s->object_size when (kmem_cache) s is NULL?
If that is the case, this change is not to blame, it only exposes the issue?

I get the following dmesg (note very early NULL kmem_cache) with the below change atop v6.1-rc3:

transfer started ......................................... transfer ok, time=2.02s
setting up elf image... OK
jumping to kernel code
zimage at:     80B842A0 810B4EFC

Uncompressing Linux at load address 80001000

Copy device tree to address  80B80EE0

Now, booting the kernel...

[    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #61 SMP Tue Nov  1 18:04:13 AEST 2022
[    0.000000] slub: kmem_cache_alloc called with kmem_cache: 0x0
[    0.000000] slub: __kmem_cache_alloc_lru called with kmem_cache: 0x0
[    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3
[    0.000000] printk: bootconsole [early0] enabled
[    0.000000] CPU0 revision is: 0001992f (MIPS 1004Kc)
[    0.000000] MIPS: machine is MikroTik RouterBOARD 760iGS

normal boot


diff --git a/mm/slub.c b/mm/slub.c
index 157527d7101b..10fcdf2520d2 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3410,7 +3410,13 @@ static __always_inline
 void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
 			     gfp_t gfpflags)
 {
-	void *ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
+	void *ret;
+	if (IS_ERR_OR_NULL(s)) {
+		pr_warn("slub: __kmem_cache_alloc_lru called with kmem_cache: %pSR\n", s);
+		ret = slab_alloc(s, lru, gfpflags, _RET_IP_, 0);
+	} else {
+		ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
+	}
 
 	trace_kmem_cache_alloc(_RET_IP_, ret, s, gfpflags, NUMA_NO_NODE);
 
@@ -3419,6 +3425,8 @@ void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
 
 void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
 {
+	if (IS_ERR_OR_NULL(s))
+		pr_warn("slub: kmem_cache_alloc called with kmem_cache: %pSR\n", s);
 	return __kmem_cache_alloc_lru(s, NULL, gfpflags);
 }
 EXPORT_SYMBOL(kmem_cache_alloc);
@@ -3426,6 +3434,8 @@ EXPORT_SYMBOL(kmem_cache_alloc);
 void *kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
 			   gfp_t gfpflags)
 {
+	if (IS_ERR_OR_NULL(s))
+		pr_warn("slub: __kmem_cache_alloc_lru called with kmem_cache: %pSR\n", s);
 	return __kmem_cache_alloc_lru(s, lru, gfpflags);
 }
 EXPORT_SYMBOL(kmem_cache_alloc_lru);


Any hints on where kmem_cache_alloc would be being called from this early?
I will start looking from /init/main.c around pr_notice("%s", linux_banner);

Thank you for your help.

Let me know if you want me to stop replying to this mm/slub debug memory wasting email,
and take this to a new email?

Cheers,
-- 
  John Thomson

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[Hyeonggon Yoo]

On Tue, Nov 01, 2022 at 09:20:21AM +0000, John Thomson wrote:
> On Tue, 1 Nov 2022, at 07:57, Feng Tang wrote:
> > Hi Thomson,
> >
> > Thanks for testing!
> >
> > + mips maintainer and mail list. The original report is here
> >
> > https://lore.kernel.org/lkml/becf2ac3-2a90-4f3a-96d9-a70f67c66e4a@app.fastmail.com/
>
> I am guessing my issue comes from __kmem_cache_alloc_lru accessing s->object_size when (kmem_cache) s is NULL?
> If that is the case, this change is not to blame, it only exposes the issue?
> 
> I get the following dmesg (note very early NULL kmem_cache) with the below change atop v6.1-rc3:
> 
> transfer started ......................................... transfer ok, time=2.02s
> setting up elf image... OK
> jumping to kernel code
> zimage at:     80B842A0 810B4EFC
> 
> Uncompressing Linux at load address 80001000
> 
> Copy device tree to address  80B80EE0
> 
> Now, booting the kernel...
> 
> [    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #61 SMP Tue Nov  1 18:04:13 AEST 2022
> [    0.000000] slub: kmem_cache_alloc called with kmem_cache: 0x0
> [    0.000000] slub: __kmem_cache_alloc_lru called with kmem_cache: 0x0
> [    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3
> [    0.000000] printk: bootconsole [early0] enabled
> [    0.000000] CPU0 revision is: 0001992f (MIPS 1004Kc)
> [    0.000000] MIPS: machine is MikroTik RouterBOARD 760iGS
> 
> normal boot
> 
> 
> diff --git a/mm/slub.c b/mm/slub.c
> index 157527d7101b..10fcdf2520d2 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -3410,7 +3410,13 @@ static __always_inline
>  void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
>  			     gfp_t gfpflags)
>  {
> -	void *ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
> +	void *ret;
> +	if (IS_ERR_OR_NULL(s)) {
> +		pr_warn("slub: __kmem_cache_alloc_lru called with kmem_cache: %pSR\n", s);
> +		ret = slab_alloc(s, lru, gfpflags, _RET_IP_, 0);
> +	} else {
> +		ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
> +	}
>  
>  	trace_kmem_cache_alloc(_RET_IP_, ret, s, gfpflags, NUMA_NO_NODE);
>  
> @@ -3419,6 +3425,8 @@ void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
>  
>  void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
>  {
> +	if (IS_ERR_OR_NULL(s))
> +		pr_warn("slub: kmem_cache_alloc called with kmem_cache: %pSR\n", s);
>  	return __kmem_cache_alloc_lru(s, NULL, gfpflags);
>  }
>  EXPORT_SYMBOL(kmem_cache_alloc);
> @@ -3426,6 +3434,8 @@ EXPORT_SYMBOL(kmem_cache_alloc);
>  void *kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
>  			   gfp_t gfpflags)
>  {
> +	if (IS_ERR_OR_NULL(s))
> +		pr_warn("slub: __kmem_cache_alloc_lru called with kmem_cache: %pSR\n", s);
>  	return __kmem_cache_alloc_lru(s, lru, gfpflags);
>  }
>  EXPORT_SYMBOL(kmem_cache_alloc_lru);
> 
> 
> Any hints on where kmem_cache_alloc would be being called from this early?
> I will start looking from /init/main.c around pr_notice("%s", linux_banner);

Great. Would you try calling dump_stack(); when we observed s == NULL?
That would give more information about who passed s == NULL to these
functions.

-- 
Thanks,
Hyeonggon

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[John Thomson]

On Tue, 1 Nov 2022, at 09:31, Hyeonggon Yoo wrote:
> On Tue, Nov 01, 2022 at 09:20:21AM +0000, John Thomson wrote:
>> On Tue, 1 Nov 2022, at 07:57, Feng Tang wrote:
>> > Hi Thomson,
>> >
>> > Thanks for testing!
>> >
>> > + mips maintainer and mail list. The original report is here
>> >
>> > https://lore.kernel.org/lkml/becf2ac3-2a90-4f3a-96d9-a70f67c66e4a@app.fastmail.com/
>>
>> I am guessing my issue comes from __kmem_cache_alloc_lru accessing s->object_size when (kmem_cache) s is NULL?
>> If that is the case, this change is not to blame, it only exposes the issue?
>> 
>> I get the following dmesg (note very early NULL kmem_cache) with the below change atop v6.1-rc3:
>> 
>> transfer started ......................................... transfer ok, time=2.02s
>> setting up elf image... OK
>> jumping to kernel code
>> zimage at:     80B842A0 810B4EFC
>> 
>> Uncompressing Linux at load address 80001000
>> 
>> Copy device tree to address  80B80EE0
>> 
>> Now, booting the kernel...
>> 
>> [    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #61 SMP Tue Nov  1 18:04:13 AEST 2022
>> [    0.000000] slub: kmem_cache_alloc called with kmem_cache: 0x0
>> [    0.000000] slub: __kmem_cache_alloc_lru called with kmem_cache: 0x0
>> [    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3
>> [    0.000000] printk: bootconsole [early0] enabled
>> [    0.000000] CPU0 revision is: 0001992f (MIPS 1004Kc)
>> [    0.000000] MIPS: machine is MikroTik RouterBOARD 760iGS
>> 
>> normal boot
>> 
>> 
>> diff --git a/mm/slub.c b/mm/slub.c
>> index 157527d7101b..10fcdf2520d2 100644
>> --- a/mm/slub.c
>> +++ b/mm/slub.c
>> @@ -3410,7 +3410,13 @@ static __always_inline
>>  void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
>>  			     gfp_t gfpflags)
>>  {
>> -	void *ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
>> +	void *ret;
>> +	if (IS_ERR_OR_NULL(s)) {
>> +		pr_warn("slub: __kmem_cache_alloc_lru called with kmem_cache: %pSR\n", s);
>> +		ret = slab_alloc(s, lru, gfpflags, _RET_IP_, 0);
>> +	} else {
>> +		ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
>> +	}
>>  
>>  	trace_kmem_cache_alloc(_RET_IP_, ret, s, gfpflags, NUMA_NO_NODE);
>>  
>> @@ -3419,6 +3425,8 @@ void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
>>  
>>  void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
>>  {
>> +	if (IS_ERR_OR_NULL(s))
>> +		pr_warn("slub: kmem_cache_alloc called with kmem_cache: %pSR\n", s);
>>  	return __kmem_cache_alloc_lru(s, NULL, gfpflags);
>>  }
>>  EXPORT_SYMBOL(kmem_cache_alloc);
>> @@ -3426,6 +3434,8 @@ EXPORT_SYMBOL(kmem_cache_alloc);
>>  void *kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
>>  			   gfp_t gfpflags)
>>  {
>> +	if (IS_ERR_OR_NULL(s))
>> +		pr_warn("slub: __kmem_cache_alloc_lru called with kmem_cache: %pSR\n", s);
>>  	return __kmem_cache_alloc_lru(s, lru, gfpflags);
>>  }
>>  EXPORT_SYMBOL(kmem_cache_alloc_lru);
>> 
>> 
>> Any hints on where kmem_cache_alloc would be being called from this early?
>> I will start looking from /init/main.c around pr_notice("%s", linux_banner);
>
> Great. Would you try calling dump_stack(); when we observed s == NULL?
> That would give more information about who passed s == NULL to these
> functions.
>

With the dump_stack() in place:

Now, booting the kernel...

[    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #62 SMP Tue Nov  1 19:49:52 AEST 2022
[    0.000000] slub: __kmem_cache_alloc_lru called with kmem_cache ptr: 0x0
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 6.1.0-rc3+ #62
[    0.000000] Stack : 810fff78 80084d98 80889d00 00000004 00000000 00000000 80889d5c 80c90000
[    0.000000]         80920000 807bd380 8089d368 80923bd3 00000000 00000001 80889d08 00000000
[    0.000000]         00000000 00000000 807bd380 8084bd51 00000002 00000002 00000001 6d6f4320
[    0.000000]         00000000 80c97ce9 80c97d14 fffffffc 807bd380 00000000 00000003 00000dc0
[    0.000000]         00000000 a0000000 80910000 8110a0b4 00000000 00000020 80010000 80010000
[    0.000000]         ...
[    0.000000] Call Trace:
[    0.000000] [<80008260>] show_stack+0x28/0xf0
[    0.000000] [<8070cdc0>] dump_stack_lvl+0x60/0x80
[    0.000000] [<801c1428>] kmem_cache_alloc+0x5c0/0x740
[    0.000000] [<8092856c>] prom_soc_init+0x1fc/0x2b4
[    0.000000] [<80928060>] prom_init+0x44/0xf0
[    0.000000] [<80929214>] setup_arch+0x4c/0x6a8
[    0.000000] [<809257e0>] start_kernel+0x88/0x7c0
[    0.000000] 
[    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3



Now, booting the kernel...

[    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #62 SMP Tue Nov  1 19:49:52 AEST 2022
[    0.000000] slub: __kmem_cache_alloc_lru called with kmem_cache ptr: 0x0
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 6.1.0-rc3+ #62
[    0.000000] Stack : 810fff78 80084d98 80889d00 00000004 00000000 00000000 80889d5c 80c90000
[    0.000000]         80920000 807bd380 8089d368 80923bd3 00000000 00000001 80889d08 00000000
[    0.000000]         00000000 00000000 807bd380 8084bd51 00000002 00000002 00000001 6d6f4320
[    0.000000]         00000000 80c97ce9 80c97d14 fffffffc 807bd380 00000000 00000003 00000dc0
[    0.000000]         00000000 a0000000 80910000 8110a0b4 00000000 00000020 80010000 80010000
[    0.000000]         ...
[    0.000000] Call Trace:
[    0.000000] show_stack (/mnt/pool_ssd/code/linux/linux-stable-mt7621/./arch/mips/include/asm/stacktrace.h:43 /mnt/pool_ssd/code/linux/linux-stable-mt7621/arch/mips/kernel/traps.c:223) 
[    0.000000] dump_stack_lvl (/mnt/pool_ssd/code/linux/linux-stable-mt7621/lib/dump_stack.c:107 (discriminator 1)) 
[    0.000000] kmem_cache_alloc (/mnt/pool_ssd/code/linux/linux-stable-mt7621/mm/slub.c:3318 /mnt/pool_ssd/code/linux/linux-stable-mt7621/mm/slub.c:3406 /mnt/pool_ssd/code/linux/linux-stable-mt7621/mm/slub.c:3418 /mnt/pool_ssd/code/linux/linux-stable-mt7621/mm/slub.c:3430) 
[    0.000000] prom_soc_init (/mnt/pool_ssd/code/linux/linux-stable-mt7621/arch/mips/ralink/mt7621.c:106 /mnt/pool_ssd/code/linux/linux-stable-mt7621/arch/mips/ralink/mt7621.c:177) 
[    0.000000] prom_init (/mnt/pool_ssd/code/linux/linux-stable-mt7621/arch/mips/ralink/prom.c:64) 
[    0.000000] setup_arch (/mnt/pool_ssd/code/linux/linux-stable-mt7621/arch/mips/kernel/setup.c:786) 
[    0.000000] start_kernel (/mnt/pool_ssd/code/linux/linux-stable-mt7621/init/main.c:279 /mnt/pool_ssd/code/linux/linux-stable-mt7621/init/main.c:477 /mnt/pool_ssd/code/linux/linux-stable-mt7621/init/main.c:960) 
[    0.000000] 
[    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3


I have not found it yet.


Cheers,
-- 
  John Thomson

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[Hyeonggon Yoo]

On Tue, Nov 01, 2022 at 10:33:32AM +0000, John Thomson wrote:
> On Tue, 1 Nov 2022, at 09:31, Hyeonggon Yoo wrote:
> > On Tue, Nov 01, 2022 at 09:20:21AM +0000, John Thomson wrote:
> >> On Tue, 1 Nov 2022, at 07:57, Feng Tang wrote:
> >> > Hi Thomson,
> >> >
> >> > Thanks for testing!
> >> >
> >> > + mips maintainer and mail list. The original report is here
> >> >
> >> > https://lore.kernel.org/lkml/becf2ac3-2a90-4f3a-96d9-a70f67c66e4a@app.fastmail.com/
> >>
> >> I am guessing my issue comes from __kmem_cache_alloc_lru accessing s->object_size when (kmem_cache) s is NULL?
> >> If that is the case, this change is not to blame, it only exposes the issue?
> >> 
> >> I get the following dmesg (note very early NULL kmem_cache) with the below change atop v6.1-rc3:
> >> 
> >> transfer started ......................................... transfer ok, time=2.02s
> >> setting up elf image... OK
> >> jumping to kernel code
> >> zimage at:     80B842A0 810B4EFC
> >> 
> >> Uncompressing Linux at load address 80001000
> >> 
> >> Copy device tree to address  80B80EE0
> >> 
> >> Now, booting the kernel...
> >> 
> >> [    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #61 SMP Tue Nov  1 18:04:13 AEST 2022
> >> [    0.000000] slub: kmem_cache_alloc called with kmem_cache: 0x0
> >> [    0.000000] slub: __kmem_cache_alloc_lru called with kmem_cache: 0x0
> >> [    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3
> >> [    0.000000] printk: bootconsole [early0] enabled
> >> [    0.000000] CPU0 revision is: 0001992f (MIPS 1004Kc)
> >> [    0.000000] MIPS: machine is MikroTik RouterBOARD 760iGS
> >> 
> >> normal boot
> >> 
> >> 
> >> diff --git a/mm/slub.c b/mm/slub.c
> >> index 157527d7101b..10fcdf2520d2 100644
> >> --- a/mm/slub.c
> >> +++ b/mm/slub.c
> >> @@ -3410,7 +3410,13 @@ static __always_inline
> >>  void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
> >>  			     gfp_t gfpflags)
> >>  {
> >> -	void *ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
> >> +	void *ret;
> >> +	if (IS_ERR_OR_NULL(s)) {
> >> +		pr_warn("slub: __kmem_cache_alloc_lru called with kmem_cache: %pSR\n", s);
> >> +		ret = slab_alloc(s, lru, gfpflags, _RET_IP_, 0);
> >> +	} else {
> >> +		ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
> >> +	}
> >>  
> >>  	trace_kmem_cache_alloc(_RET_IP_, ret, s, gfpflags, NUMA_NO_NODE);
> >>  
> >> @@ -3419,6 +3425,8 @@ void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
> >>  
> >>  void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
> >>  {
> >> +	if (IS_ERR_OR_NULL(s))
> >> +		pr_warn("slub: kmem_cache_alloc called with kmem_cache: %pSR\n", s);
> >>  	return __kmem_cache_alloc_lru(s, NULL, gfpflags);
> >>  }
> >>  EXPORT_SYMBOL(kmem_cache_alloc);
> >> @@ -3426,6 +3434,8 @@ EXPORT_SYMBOL(kmem_cache_alloc);
> >>  void *kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
> >>  			   gfp_t gfpflags)
> >>  {
> >> +	if (IS_ERR_OR_NULL(s))
> >> +		pr_warn("slub: __kmem_cache_alloc_lru called with kmem_cache: %pSR\n", s);
> >>  	return __kmem_cache_alloc_lru(s, lru, gfpflags);
> >>  }
> >>  EXPORT_SYMBOL(kmem_cache_alloc_lru);
> >> 
> >> 
> >> Any hints on where kmem_cache_alloc would be being called from this early?
> >> I will start looking from /init/main.c around pr_notice("%s", linux_banner);
> >
> > Great. Would you try calling dump_stack(); when we observed s == NULL?
> > That would give more information about who passed s == NULL to these
> > functions.
> >
> 
> With the dump_stack() in place:
> 
> Now, booting the kernel...
> 
> [    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #62 SMP Tue Nov  1 19:49:52 AEST 2022
> [    0.000000] slub: __kmem_cache_alloc_lru called with kmem_cache ptr: 0x0
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 6.1.0-rc3+ #62
> [    0.000000] Stack : 810fff78 80084d98 80889d00 00000004 00000000 00000000 80889d5c 80c90000
> [    0.000000]         80920000 807bd380 8089d368 80923bd3 00000000 00000001 80889d08 00000000
> [    0.000000]         00000000 00000000 807bd380 8084bd51 00000002 00000002 00000001 6d6f4320
> [    0.000000]         00000000 80c97ce9 80c97d14 fffffffc 807bd380 00000000 00000003 00000dc0
> [    0.000000]         00000000 a0000000 80910000 8110a0b4 00000000 00000020 80010000 80010000
> [    0.000000]         ...
> [    0.000000] Call Trace:
> [    0.000000] [<80008260>] show_stack+0x28/0xf0
> [    0.000000] [<8070cdc0>] dump_stack_lvl+0x60/0x80
> [    0.000000] [<801c1428>] kmem_cache_alloc+0x5c0/0x740
> [    0.000000] [<8092856c>] prom_soc_init+0x1fc/0x2b4
> [    0.000000] [<80928060>] prom_init+0x44/0xf0
> [    0.000000] [<80929214>] setup_arch+0x4c/0x6a8
> [    0.000000] [<809257e0>] start_kernel+0x88/0x7c0
> [    0.000000] 
> [    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3

setup_arch() is too early to use slab allocators.
I think slab received NULL pointer because kmalloc is not initialized.

It seems arch/mips/ralink/mt7621.c is using slab too early.

> 
> 
> Now, booting the kernel...
> 
> [    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #62 SMP Tue Nov  1 19:49:52 AEST 2022
> [    0.000000] slub: __kmem_cache_alloc_lru called with kmem_cache ptr: 0x0
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 6.1.0-rc3+ #62
> [    0.000000] Stack : 810fff78 80084d98 80889d00 00000004 00000000 00000000 80889d5c 80c90000
> [    0.000000]         80920000 807bd380 8089d368 80923bd3 00000000 00000001 80889d08 00000000
> [    0.000000]         00000000 00000000 807bd380 8084bd51 00000002 00000002 00000001 6d6f4320
> [    0.000000]         00000000 80c97ce9 80c97d14 fffffffc 807bd380 00000000 00000003 00000dc0
> [    0.000000]         00000000 a0000000 80910000 8110a0b4 00000000 00000020 80010000 80010000
> [    0.000000]         ...
> [    0.000000] Call Trace:
> [    0.000000] show_stack (/mnt/pool_ssd/code/linux/linux-stable-mt7621/./arch/mips/include/asm/stacktrace.h:43 /mnt/pool_ssd/code/linux/linux-stable-mt7621/arch/mips/kernel/traps.c:223) 
> [    0.000000] dump_stack_lvl (/mnt/pool_ssd/code/linux/linux-stable-mt7621/lib/dump_stack.c:107 (discriminator 1)) 
> [    0.000000] kmem_cache_alloc (/mnt/pool_ssd/code/linux/linux-stable-mt7621/mm/slub.c:3318 /mnt/pool_ssd/code/linux/linux-stable-mt7621/mm/slub.c:3406 /mnt/pool_ssd/code/linux/linux-stable-mt7621/mm/slub.c:3418 /mnt/pool_ssd/code/linux/linux-stable-mt7621/mm/slub.c:3430) 
> [    0.000000] prom_soc_init (/mnt/pool_ssd/code/linux/linux-stable-mt7621/arch/mips/ralink/mt7621.c:106 /mnt/pool_ssd/code/linux/linux-stable-mt7621/arch/mips/ralink/mt7621.c:177) 
> [    0.000000] prom_init (/mnt/pool_ssd/code/linux/linux-stable-mt7621/arch/mips/ralink/prom.c:64) 
> [    0.000000] setup_arch (/mnt/pool_ssd/code/linux/linux-stable-mt7621/arch/mips/kernel/setup.c:786) 
> [    0.000000] start_kernel (/mnt/pool_ssd/code/linux/linux-stable-mt7621/init/main.c:279 /mnt/pool_ssd/code/linux/linux-stable-mt7621/init/main.c:477 /mnt/pool_ssd/code/linux/linux-stable-mt7621/init/main.c:960) 
> [    0.000000] 
> [    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3
> 
> 
> I have not found it yet.
> 
> 
> Cheers,
> -- 
>   John Thomson

-- 
Thanks,
Hyeonggon

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[Feng Tang]

On Tue, Nov 01, 2022 at 06:42:23PM +0800, Hyeonggon Yoo wrote:
> On Tue, Nov 01, 2022 at 10:33:32AM +0000, John Thomson wrote:
> > On Tue, 1 Nov 2022, at 09:31, Hyeonggon Yoo wrote:
> > > On Tue, Nov 01, 2022 at 09:20:21AM +0000, John Thomson wrote:
> > >> On Tue, 1 Nov 2022, at 07:57, Feng Tang wrote:
> > >> > Hi Thomson,
> > >> >
> > >> > Thanks for testing!
> > >> >
> > >> > + mips maintainer and mail list. The original report is here
> > >> >
> > >> > https://lore.kernel.org/lkml/becf2ac3-2a90-4f3a-96d9-a70f67c66e4a@app.fastmail.com/
> > >>
> > >> I am guessing my issue comes from __kmem_cache_alloc_lru accessing s->object_size when (kmem_cache) s is NULL?
> > >> If that is the case, this change is not to blame, it only exposes the issue?
> > >> 
> > >> I get the following dmesg (note very early NULL kmem_cache) with the below change atop v6.1-rc3:
> > >> 
> > >> transfer started ......................................... transfer ok, time=2.02s
> > >> setting up elf image... OK
> > >> jumping to kernel code
> > >> zimage at:     80B842A0 810B4EFC
> > >> 
> > >> Uncompressing Linux at load address 80001000
> > >> 
> > >> Copy device tree to address  80B80EE0
> > >> 
> > >> Now, booting the kernel...
> > >> 
> > >> [    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #61 SMP Tue Nov  1 18:04:13 AEST 2022
> > >> [    0.000000] slub: kmem_cache_alloc called with kmem_cache: 0x0
> > >> [    0.000000] slub: __kmem_cache_alloc_lru called with kmem_cache: 0x0
> > >> [    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3
> > >> [    0.000000] printk: bootconsole [early0] enabled
> > >> [    0.000000] CPU0 revision is: 0001992f (MIPS 1004Kc)
> > >> [    0.000000] MIPS: machine is MikroTik RouterBOARD 760iGS
> > >> 
> > >> normal boot
> > >> 
> > >> 
> > >> diff --git a/mm/slub.c b/mm/slub.c
> > >> index 157527d7101b..10fcdf2520d2 100644
> > >> --- a/mm/slub.c
> > >> +++ b/mm/slub.c
> > >> @@ -3410,7 +3410,13 @@ static __always_inline
> > >>  void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
> > >>  			     gfp_t gfpflags)
> > >>  {
> > >> -	void *ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
> > >> +	void *ret;
> > >> +	if (IS_ERR_OR_NULL(s)) {
> > >> +		pr_warn("slub: __kmem_cache_alloc_lru called with kmem_cache: %pSR\n", s);
> > >> +		ret = slab_alloc(s, lru, gfpflags, _RET_IP_, 0);
> > >> +	} else {
> > >> +		ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
> > >> +	}
> > >>  
> > >>  	trace_kmem_cache_alloc(_RET_IP_, ret, s, gfpflags, NUMA_NO_NODE);
> > >>  
> > >> @@ -3419,6 +3425,8 @@ void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
> > >>  
> > >>  void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
> > >>  {
> > >> +	if (IS_ERR_OR_NULL(s))
> > >> +		pr_warn("slub: kmem_cache_alloc called with kmem_cache: %pSR\n", s);
> > >>  	return __kmem_cache_alloc_lru(s, NULL, gfpflags);
> > >>  }
> > >>  EXPORT_SYMBOL(kmem_cache_alloc);
> > >> @@ -3426,6 +3434,8 @@ EXPORT_SYMBOL(kmem_cache_alloc);
> > >>  void *kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
> > >>  			   gfp_t gfpflags)
> > >>  {
> > >> +	if (IS_ERR_OR_NULL(s))
> > >> +		pr_warn("slub: __kmem_cache_alloc_lru called with kmem_cache: %pSR\n", s);
> > >>  	return __kmem_cache_alloc_lru(s, lru, gfpflags);
> > >>  }
> > >>  EXPORT_SYMBOL(kmem_cache_alloc_lru);
> > >> 
> > >> 
> > >> Any hints on where kmem_cache_alloc would be being called from this early?
> > >> I will start looking from /init/main.c around pr_notice("%s", linux_banner);
> > >
> > > Great. Would you try calling dump_stack(); when we observed s == NULL?
> > > That would give more information about who passed s == NULL to these
> > > functions.
> > >
> > 
> > With the dump_stack() in place:
> > 
> > Now, booting the kernel...
> > 
> > [    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #62 SMP Tue Nov  1 19:49:52 AEST 2022
> > [    0.000000] slub: __kmem_cache_alloc_lru called with kmem_cache ptr: 0x0
> > [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 6.1.0-rc3+ #62
> > [    0.000000] Stack : 810fff78 80084d98 80889d00 00000004 00000000 00000000 80889d5c 80c90000
> > [    0.000000]         80920000 807bd380 8089d368 80923bd3 00000000 00000001 80889d08 00000000
> > [    0.000000]         00000000 00000000 807bd380 8084bd51 00000002 00000002 00000001 6d6f4320
> > [    0.000000]         00000000 80c97ce9 80c97d14 fffffffc 807bd380 00000000 00000003 00000dc0
> > [    0.000000]         00000000 a0000000 80910000 8110a0b4 00000000 00000020 80010000 80010000
> > [    0.000000]         ...
> > [    0.000000] Call Trace:
> > [    0.000000] [<80008260>] show_stack+0x28/0xf0
> > [    0.000000] [<8070cdc0>] dump_stack_lvl+0x60/0x80
> > [    0.000000] [<801c1428>] kmem_cache_alloc+0x5c0/0x740
> > [    0.000000] [<8092856c>] prom_soc_init+0x1fc/0x2b4
> > [    0.000000] [<80928060>] prom_init+0x44/0xf0
> > [    0.000000] [<80929214>] setup_arch+0x4c/0x6a8
> > [    0.000000] [<809257e0>] start_kernel+0x88/0x7c0
> > [    0.000000] 
> > [    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3
> 
> setup_arch() is too early to use slab allocators.
> I think slab received NULL pointer because kmalloc is not initialized.
> 
> It seems arch/mips/ralink/mt7621.c is using slab too early.

Cool! it is finally root caused :) Thanks!

The following patch should solve it and give it a warning message, though
I'm not sure if there is other holes.  

Thanks,
Feng

---
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 33b1886b06eb..429c21b7ecbc 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1043,7 +1043,14 @@ size_t __ksize(const void *object)
 #ifdef CONFIG_TRACING
 void *kmalloc_trace(struct kmem_cache *s, gfp_t gfpflags, size_t size)
 {
-	void *ret = __kmem_cache_alloc_node(s, gfpflags, NUMA_NO_NODE,
+	void *ret;
+
+	if (unlikely(ZERO_OR_NULL_PTR(s))) {
+		WARN_ON_ONCE(1);
+		return s;
+	}
+
+	ret = __kmem_cache_alloc_node(s, gfpflags, NUMA_NO_NODE,
 					    size, _RET_IP_);
 
 	trace_kmalloc(_RET_IP_, ret, size, s->size, gfpflags, NUMA_NO_NODE);
diff --git a/mm/slub.c b/mm/slub.c
index 157527d7101b..85d24bb6eda7 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -3410,8 +3410,14 @@ static __always_inline
 void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
 			     gfp_t gfpflags)
 {
-	void *ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
+	void *ret;
 
+	if (unlikely(ZERO_OR_NULL_PTR(s))) {
+		WARN_ON_ONCE(1);
+		return s;
+	}
+
+	ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
 	trace_kmem_cache_alloc(_RET_IP_, ret, s, gfpflags, NUMA_NO_NODE);
 
 	return ret;

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[John Thomson]

On Tue, 1 Nov 2022, at 13:55, Feng Tang wrote:
> On Tue, Nov 01, 2022 at 06:42:23PM +0800, Hyeonggon Yoo wrote:
>> setup_arch() is too early to use slab allocators.
>> I think slab received NULL pointer because kmalloc is not initialized.
>> 
>> It seems arch/mips/ralink/mt7621.c is using slab too early.
>
> Cool! it is finally root caused :) Thanks!
>
> The following patch should solve it and give it a warning message, though
> I'm not sure if there is other holes.  
>
> Thanks,
> Feng
>
> ---
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 33b1886b06eb..429c21b7ecbc 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1043,7 +1043,14 @@ size_t __ksize(const void *object)
>  #ifdef CONFIG_TRACING
>  void *kmalloc_trace(struct kmem_cache *s, gfp_t gfpflags, size_t size)
>  {
> -	void *ret = __kmem_cache_alloc_node(s, gfpflags, NUMA_NO_NODE,
> +	void *ret;
> +
> +	if (unlikely(ZERO_OR_NULL_PTR(s))) {
> +		WARN_ON_ONCE(1);
> +		return s;
> +	}
> +
> +	ret = __kmem_cache_alloc_node(s, gfpflags, NUMA_NO_NODE,
>  					    size, _RET_IP_);
> 
>  	trace_kmalloc(_RET_IP_, ret, size, s->size, gfpflags, NUMA_NO_NODE);
> diff --git a/mm/slub.c b/mm/slub.c
> index 157527d7101b..85d24bb6eda7 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -3410,8 +3410,14 @@ static __always_inline
>  void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
>  			     gfp_t gfpflags)
>  {
> -	void *ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
> +	void *ret;
> 
> +	if (unlikely(ZERO_OR_NULL_PTR(s))) {
> +		WARN_ON_ONCE(1);
> +		return s;
> +	}
> +
> +	ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
>  	trace_kmem_cache_alloc(_RET_IP_, ret, s, gfpflags, NUMA_NO_NODE);
> 
>  	return ret;

Yes, thank you, that patch atop v6.1-rc3 lets me boot, and shows the warning and stack dump.
Will you submit that, or how do we want to proceed?

transfer started ......................................... transfer ok, time=2.11s
setting up elf image... OK
jumping to kernel code
zimage at:     80B842A0 810B4BC0

Uncompressing Linux at load address 80001000

Copy device tree to address  80B80EE0

Now, booting the kernel...

[    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #73 SMP Wed Nov  2 05:10:01 AEST 2022
[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: CPU: 0 PID: 0 at mm/slub.c:3416 kmem_cache_alloc+0x5a4/0x5e8
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 6.1.0-rc3+ #73
[    0.000000] Stack : 810fff78 80084d98 00000000 00000004 00000000 00000000 80889d04 80c90000
[    0.000000]         80920000 807bd328 8089d368 80923bd3 00000000 00000001 80889cb0 00000000
[    0.000000]         00000000 00000000 807bd328 8084bcb1 00000002 00000002 00000001 6d6f4320
[    0.000000]         00000000 80c97d3d 80c97d68 fffffffc 807bd328 00000000 00000000 00000000
[    0.000000]         00000000 a0000000 80910000 8110a0b4 00000000 00000020 80010000 80010000
[    0.000000]         ...
[    0.000000] Call Trace:
[    0.000000] [<80008260>] show_stack+0x28/0xf0
[    0.000000] [<8070c958>] dump_stack_lvl+0x60/0x80
[    0.000000] [<8002e184>] __warn+0xc4/0xf8
[    0.000000] [<8002e210>] warn_slowpath_fmt+0x58/0xa4
[    0.000000] [<801c0fac>] kmem_cache_alloc+0x5a4/0x5e8
[    0.000000] [<8092856c>] prom_soc_init+0x1fc/0x2b4
[    0.000000] [<80928060>] prom_init+0x44/0xf0
[    0.000000] [<80929214>] setup_arch+0x4c/0x6a8
[    0.000000] [<809257e0>] start_kernel+0x88/0x7c0
[    0.000000] 
[    0.000000] ---[ end trace 0000000000000000 ]---
[    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3
[    0.000000] printk: bootconsole [early0] enabled

Thank you for working through this with me.
I will try to address the root cause in mt7621.c.
It looks like other arch/** soc_device_register users use postcore_initcall, device_initcall,
or the ARM DT_MACHINE_START .init_machine. A quick hack to use postcore_initcall in mt7621
avoided this zero ptr kmem_cache passed to kmem_cache_alloc_lru.


Thanks,

-- 
  John Thomson

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[Feng Tang]

On Tue, Nov 01, 2022 at 07:39:13PM +0000, John Thomson wrote:
> 
> 
> On Tue, 1 Nov 2022, at 13:55, Feng Tang wrote:
> > On Tue, Nov 01, 2022 at 06:42:23PM +0800, Hyeonggon Yoo wrote:
> >> setup_arch() is too early to use slab allocators.
> >> I think slab received NULL pointer because kmalloc is not initialized.
> >> 
> >> It seems arch/mips/ralink/mt7621.c is using slab too early.
> >
> > Cool! it is finally root caused :) Thanks!
> >
> > The following patch should solve it and give it a warning message, though
> > I'm not sure if there is other holes.  
> >
> > Thanks,
> > Feng
> >
> > ---
> > diff --git a/mm/slab_common.c b/mm/slab_common.c
> > index 33b1886b06eb..429c21b7ecbc 100644
> > --- a/mm/slab_common.c
> > +++ b/mm/slab_common.c
> > @@ -1043,7 +1043,14 @@ size_t __ksize(const void *object)
> >  #ifdef CONFIG_TRACING
> >  void *kmalloc_trace(struct kmem_cache *s, gfp_t gfpflags, size_t size)
> >  {
> > -	void *ret = __kmem_cache_alloc_node(s, gfpflags, NUMA_NO_NODE,
> > +	void *ret;
> > +
> > +	if (unlikely(ZERO_OR_NULL_PTR(s))) {
> > +		WARN_ON_ONCE(1);
> > +		return s;
> > +	}
> > +
> > +	ret = __kmem_cache_alloc_node(s, gfpflags, NUMA_NO_NODE,
> >  					    size, _RET_IP_);
> > 
> >  	trace_kmalloc(_RET_IP_, ret, size, s->size, gfpflags, NUMA_NO_NODE);
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 157527d7101b..85d24bb6eda7 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -3410,8 +3410,14 @@ static __always_inline
> >  void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
> >  			     gfp_t gfpflags)
> >  {
> > -	void *ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
> > +	void *ret;
> > 
> > +	if (unlikely(ZERO_OR_NULL_PTR(s))) {
> > +		WARN_ON_ONCE(1);
> > +		return s;
> > +	}
> > +
> > +	ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
> >  	trace_kmem_cache_alloc(_RET_IP_, ret, s, gfpflags, NUMA_NO_NODE);
> > 
> >  	return ret;
> 
> Yes, thank you, that patch atop v6.1-rc3 lets me boot, and shows the warning and stack dump.
> Will you submit that, or how do we want to proceed?

Thanks for confirming. I wanted to wait for Vlastimil, Hyeonggon and
other developer's opinion. And yes, I can also post a more formal one.

> transfer started ......................................... transfer ok, time=2.11s
> setting up elf image... OK
> jumping to kernel code
> zimage at:     80B842A0 810B4BC0
> 
> Uncompressing Linux at load address 80001000
> 
> Copy device tree to address  80B80EE0
> 
> Now, booting the kernel...
> 
> [    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #73 SMP Wed Nov  2 05:10:01 AEST 2022
> [    0.000000] ------------[ cut here ]------------
> [    0.000000] WARNING: CPU: 0 PID: 0 at mm/slub.c:3416 kmem_cache_alloc+0x5a4/0x5e8
> [    0.000000] Modules linked in:
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 6.1.0-rc3+ #73
> [    0.000000] Stack : 810fff78 80084d98 00000000 00000004 00000000 00000000 80889d04 80c90000
> [    0.000000]         80920000 807bd328 8089d368 80923bd3 00000000 00000001 80889cb0 00000000
> [    0.000000]         00000000 00000000 807bd328 8084bcb1 00000002 00000002 00000001 6d6f4320
> [    0.000000]         00000000 80c97d3d 80c97d68 fffffffc 807bd328 00000000 00000000 00000000
> [    0.000000]         00000000 a0000000 80910000 8110a0b4 00000000 00000020 80010000 80010000
> [    0.000000]         ...
> [    0.000000] Call Trace:
> [    0.000000] [<80008260>] show_stack+0x28/0xf0
> [    0.000000] [<8070c958>] dump_stack_lvl+0x60/0x80
> [    0.000000] [<8002e184>] __warn+0xc4/0xf8
> [    0.000000] [<8002e210>] warn_slowpath_fmt+0x58/0xa4
> [    0.000000] [<801c0fac>] kmem_cache_alloc+0x5a4/0x5e8
> [    0.000000] [<8092856c>] prom_soc_init+0x1fc/0x2b4
> [    0.000000] [<80928060>] prom_init+0x44/0xf0
> [    0.000000] [<80929214>] setup_arch+0x4c/0x6a8
> [    0.000000] [<809257e0>] start_kernel+0x88/0x7c0
> [    0.000000] 
> [    0.000000] ---[ end trace 0000000000000000 ]---
> [    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3
> [    0.000000] printk: bootconsole [early0] enabled
> 
> Thank you for working through this with me.
> I will try to address the root cause in mt7621.c.
> It looks like other arch/** soc_device_register users use postcore_initcall, device_initcall,
> or the ARM DT_MACHINE_START .init_machine. A quick hack to use postcore_initcall in mt7621
> avoided this zero ptr kmem_cache passed to kmem_cache_alloc_lru.

If IIUC, the prom_soc_init() is only called once in kernel, can the
'soc_dev_attr' just be defined as a global data structure instead
of calling kzalloc(), as its size is small only containing 7 pointers.

Thanks,
Feng

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[Hyeonggon Yoo]

On Wed, Nov 02, 2022 at 02:08:09PM +0800, Feng Tang wrote:
> On Tue, Nov 01, 2022 at 07:39:13PM +0000, John Thomson wrote:
> > 
> > 
> > On Tue, 1 Nov 2022, at 13:55, Feng Tang wrote:
> > > On Tue, Nov 01, 2022 at 06:42:23PM +0800, Hyeonggon Yoo wrote:
> > >> setup_arch() is too early to use slab allocators.
> > >> I think slab received NULL pointer because kmalloc is not initialized.
> > >> 
> > >> It seems arch/mips/ralink/mt7621.c is using slab too early.
> > >
> > > Cool! it is finally root caused :) Thanks!
> > >
> > > The following patch should solve it and give it a warning message, though
> > > I'm not sure if there is other holes.  
> > >
> > > Thanks,
> > > Feng
> > >
> > > ---
> > > diff --git a/mm/slab_common.c b/mm/slab_common.c
> > > index 33b1886b06eb..429c21b7ecbc 100644
> > > --- a/mm/slab_common.c
> > > +++ b/mm/slab_common.c
> > > @@ -1043,7 +1043,14 @@ size_t __ksize(const void *object)
> > >  #ifdef CONFIG_TRACING
> > >  void *kmalloc_trace(struct kmem_cache *s, gfp_t gfpflags, size_t size)
> > >  {
> > > -	void *ret = __kmem_cache_alloc_node(s, gfpflags, NUMA_NO_NODE,
> > > +	void *ret;
> > > +
> > > +	if (unlikely(ZERO_OR_NULL_PTR(s))) {
> > > +		WARN_ON_ONCE(1);
> > > +		return s;
> > > +	}
> > > +
> > > +	ret = __kmem_cache_alloc_node(s, gfpflags, NUMA_NO_NODE,
> > >  					    size, _RET_IP_);
> > > 
> > >  	trace_kmalloc(_RET_IP_, ret, size, s->size, gfpflags, NUMA_NO_NODE);
> > > diff --git a/mm/slub.c b/mm/slub.c
> > > index 157527d7101b..85d24bb6eda7 100644
> > > --- a/mm/slub.c
> > > +++ b/mm/slub.c
> > > @@ -3410,8 +3410,14 @@ static __always_inline
> > >  void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
> > >  			     gfp_t gfpflags)
> > >  {
> > > -	void *ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
> > > +	void *ret;
> > > 
> > > +	if (unlikely(ZERO_OR_NULL_PTR(s))) {
> > > +		WARN_ON_ONCE(1);
> > > +		return s;
> > > +	}
> > > +

Thank you for suggestion!

I think the holes are:
	kmalloc_node_trace(), kmem_cache_alloc_node(), __do_kmalloc_node()

And want to suggest:
	What about using VM_WARN_ON_ONCE() instead?

> > > +	ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
> > >  	trace_kmem_cache_alloc(_RET_IP_, ret, s, gfpflags, NUMA_NO_NODE);
> > > 
> > >  	return ret;
> > 
> > Yes, thank you, that patch atop v6.1-rc3 lets me boot, and shows the warning and stack dump.
> > Will you submit that, or how do we want to proceed?
> 
> Thanks for confirming. I wanted to wait for Vlastimil, Hyeonggon and
> other developer's opinion. And yes, I can also post a more formal one.
> 
> > transfer started ......................................... transfer ok, time=2.11s
> > setting up elf image... OK
> > jumping to kernel code
> > zimage at:     80B842A0 810B4BC0
> > 
> > Uncompressing Linux at load address 80001000
> > 
> > Copy device tree to address  80B80EE0
> > 
> > Now, booting the kernel...
> > 
> > [    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #73 SMP Wed Nov  2 05:10:01 AEST 2022
> > [    0.000000] ------------[ cut here ]------------
> > [    0.000000] WARNING: CPU: 0 PID: 0 at mm/slub.c:3416 kmem_cache_alloc+0x5a4/0x5e8
> > [    0.000000] Modules linked in:
> > [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 6.1.0-rc3+ #73
> > [    0.000000] Stack : 810fff78 80084d98 00000000 00000004 00000000 00000000 80889d04 80c90000
> > [    0.000000]         80920000 807bd328 8089d368 80923bd3 00000000 00000001 80889cb0 00000000
> > [    0.000000]         00000000 00000000 807bd328 8084bcb1 00000002 00000002 00000001 6d6f4320
> > [    0.000000]         00000000 80c97d3d 80c97d68 fffffffc 807bd328 00000000 00000000 00000000
> > [    0.000000]         00000000 a0000000 80910000 8110a0b4 00000000 00000020 80010000 80010000
> > [    0.000000]         ...
> > [    0.000000] Call Trace:
> > [    0.000000] [<80008260>] show_stack+0x28/0xf0
> > [    0.000000] [<8070c958>] dump_stack_lvl+0x60/0x80
> > [    0.000000] [<8002e184>] __warn+0xc4/0xf8
> > [    0.000000] [<8002e210>] warn_slowpath_fmt+0x58/0xa4
> > [    0.000000] [<801c0fac>] kmem_cache_alloc+0x5a4/0x5e8
> > [    0.000000] [<8092856c>] prom_soc_init+0x1fc/0x2b4
> > [    0.000000] [<80928060>] prom_init+0x44/0xf0
> > [    0.000000] [<80929214>] setup_arch+0x4c/0x6a8
> > [    0.000000] [<809257e0>] start_kernel+0x88/0x7c0
> > [    0.000000] 
> > [    0.000000] ---[ end trace 0000000000000000 ]---
> > [    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3
> > [    0.000000] printk: bootconsole [early0] enabled
> > 
> > Thank you for working through this with me.
> > I will try to address the root cause in mt7621.c.
> > It looks like other arch/** soc_device_register users use postcore_initcall, device_initcall,
> > or the ARM DT_MACHINE_START .init_machine. A quick hack to use postcore_initcall in mt7621
> > avoided this zero ptr kmem_cache passed to kmem_cache_alloc_lru.
> 
> If IIUC, the prom_soc_init() is only called once in kernel, can the
> 'soc_dev_attr' just be defined as a global data structure instead
> of calling kzalloc(), as its size is small only containing 7 pointers.

But soc_device_registers() too uses kmalloc. I think calling it
after slab initialization will be best solution - if that is correct.

> 
> Thanks,
> Feng
> 

-- 
Thanks,
Hyeonggon

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[Feng Tang]

On Wed, Nov 02, 2022 at 04:16:06PM +0900, Hyeonggon Yoo wrote:
> On Wed, Nov 02, 2022 at 02:08:09PM +0800, Feng Tang wrote:
[...]
> > > transfer started ......................................... transfer ok, time=2.11s
> > > setting up elf image... OK
> > > jumping to kernel code
> > > zimage at:     80B842A0 810B4BC0
> > > 
> > > Uncompressing Linux at load address 80001000
> > > 
> > > Copy device tree to address  80B80EE0
> > > 
> > > Now, booting the kernel...
> > > 
> > > [    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #73 SMP Wed Nov  2 05:10:01 AEST 2022
> > > [    0.000000] ------------[ cut here ]------------
> > > [    0.000000] WARNING: CPU: 0 PID: 0 at mm/slub.c:3416 kmem_cache_alloc+0x5a4/0x5e8
> > > [    0.000000] Modules linked in:
> > > [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 6.1.0-rc3+ #73
> > > [    0.000000] Stack : 810fff78 80084d98 00000000 00000004 00000000 00000000 80889d04 80c90000
> > > [    0.000000]         80920000 807bd328 8089d368 80923bd3 00000000 00000001 80889cb0 00000000
> > > [    0.000000]         00000000 00000000 807bd328 8084bcb1 00000002 00000002 00000001 6d6f4320
> > > [    0.000000]         00000000 80c97d3d 80c97d68 fffffffc 807bd328 00000000 00000000 00000000
> > > [    0.000000]         00000000 a0000000 80910000 8110a0b4 00000000 00000020 80010000 80010000
> > > [    0.000000]         ...
> > > [    0.000000] Call Trace:
> > > [    0.000000] [<80008260>] show_stack+0x28/0xf0
> > > [    0.000000] [<8070c958>] dump_stack_lvl+0x60/0x80
> > > [    0.000000] [<8002e184>] __warn+0xc4/0xf8
> > > [    0.000000] [<8002e210>] warn_slowpath_fmt+0x58/0xa4
> > > [    0.000000] [<801c0fac>] kmem_cache_alloc+0x5a4/0x5e8
> > > [    0.000000] [<8092856c>] prom_soc_init+0x1fc/0x2b4
> > > [    0.000000] [<80928060>] prom_init+0x44/0xf0
> > > [    0.000000] [<80929214>] setup_arch+0x4c/0x6a8
> > > [    0.000000] [<809257e0>] start_kernel+0x88/0x7c0
> > > [    0.000000] 
> > > [    0.000000] ---[ end trace 0000000000000000 ]---
> > > [    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3
> > > [    0.000000] printk: bootconsole [early0] enabled
> > > 
> > > Thank you for working through this with me.
> > > I will try to address the root cause in mt7621.c.
> > > It looks like other arch/** soc_device_register users use postcore_initcall, device_initcall,
> > > or the ARM DT_MACHINE_START .init_machine. A quick hack to use postcore_initcall in mt7621
> > > avoided this zero ptr kmem_cache passed to kmem_cache_alloc_lru.
> > 
> > If IIUC, the prom_soc_init() is only called once in kernel, can the
> > 'soc_dev_attr' just be defined as a global data structure instead
> > of calling kzalloc(), as its size is small only containing 7 pointers.
> 
> But soc_device_registers() too uses kmalloc. I think calling it
> after slab initialization will be best solution - if that is correct.

Yes, you are right, there is other kmalloc() down the call chain.

Hi John,

Will you verify and submit a patch for your proposal of deferring
calling prom_soc_init()? thanks

- Feng

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[John Thomson]

On Thu, 3 Nov 2022, at 07:18, Feng Tang wrote:
> On Wed, Nov 02, 2022 at 04:16:06PM +0900, Hyeonggon Yoo wrote:
>> On Wed, Nov 02, 2022 at 02:08:09PM +0800, Feng Tang wrote:
> [...]
>> > > transfer started ......................................... transfer ok, time=2.11s
>> > > setting up elf image... OK
>> > > jumping to kernel code
>> > > zimage at:     80B842A0 810B4BC0
>> > > 
>> > > Uncompressing Linux at load address 80001000
>> > > 
>> > > Copy device tree to address  80B80EE0
>> > > 
>> > > Now, booting the kernel...
>> > > 
>> > > [    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #73 SMP Wed Nov  2 05:10:01 AEST 2022
>> > > [    0.000000] ------------[ cut here ]------------
>> > > [    0.000000] WARNING: CPU: 0 PID: 0 at mm/slub.c:3416 kmem_cache_alloc+0x5a4/0x5e8
>> > > [    0.000000] Modules linked in:
>> > > [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 6.1.0-rc3+ #73
>> > > [    0.000000] Stack : 810fff78 80084d98 00000000 00000004 00000000 00000000 80889d04 80c90000
>> > > [    0.000000]         80920000 807bd328 8089d368 80923bd3 00000000 00000001 80889cb0 00000000
>> > > [    0.000000]         00000000 00000000 807bd328 8084bcb1 00000002 00000002 00000001 6d6f4320
>> > > [    0.000000]         00000000 80c97d3d 80c97d68 fffffffc 807bd328 00000000 00000000 00000000
>> > > [    0.000000]         00000000 a0000000 80910000 8110a0b4 00000000 00000020 80010000 80010000
>> > > [    0.000000]         ...
>> > > [    0.000000] Call Trace:
>> > > [    0.000000] [<80008260>] show_stack+0x28/0xf0
>> > > [    0.000000] [<8070c958>] dump_stack_lvl+0x60/0x80
>> > > [    0.000000] [<8002e184>] __warn+0xc4/0xf8
>> > > [    0.000000] [<8002e210>] warn_slowpath_fmt+0x58/0xa4
>> > > [    0.000000] [<801c0fac>] kmem_cache_alloc+0x5a4/0x5e8
>> > > [    0.000000] [<8092856c>] prom_soc_init+0x1fc/0x2b4
>> > > [    0.000000] [<80928060>] prom_init+0x44/0xf0
>> > > [    0.000000] [<80929214>] setup_arch+0x4c/0x6a8
>> > > [    0.000000] [<809257e0>] start_kernel+0x88/0x7c0
>> > > [    0.000000] 
>> > > [    0.000000] ---[ end trace 0000000000000000 ]---
>> > > [    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3
>> > > [    0.000000] printk: bootconsole [early0] enabled
>> > > 
>> > > Thank you for working through this with me.
>> > > I will try to address the root cause in mt7621.c.
>> > > It looks like other arch/** soc_device_register users use postcore_initcall, device_initcall,
>> > > or the ARM DT_MACHINE_START .init_machine. A quick hack to use postcore_initcall in mt7621
>> > > avoided this zero ptr kmem_cache passed to kmem_cache_alloc_lru.
>> > 
>> > If IIUC, the prom_soc_init() is only called once in kernel, can the
>> > 'soc_dev_attr' just be defined as a global data structure instead
>> > of calling kzalloc(), as its size is small only containing 7 pointers.
>> 
>> But soc_device_registers() too uses kmalloc. I think calling it
>> after slab initialization will be best solution - if that is correct.
>
> Yes, you are right, there is other kmalloc() down the call chain.
>
> Hi John,
>
> Will you verify and submit a patch for your proposal of deferring
> calling prom_soc_init()? thanks
>
> - Feng

Hi Feng,

My proposed mt7621.c changes are RFC here:
https://lore.kernel.org/lkml/20221103050538.1930758-1-git@johnthomson.fastmail.com.au/
That series lets me boot the v6.1-rc3 kernel. I have only tried it with my config (as sent earlier). If there are other suspect config settings that I should test, please let me know?
I used device_initcall, but postcore_initcall also works fine.
I rephrased Vlastimil's explanation and used it in patch 3 description.
I have not referenced a Fixes tag yet (unsure which/if any I should use)

Cheers,
-- 
  John Thomson

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[Feng Tang]

On Thu, Nov 03, 2022 at 07:45:49AM +0000, John Thomson wrote:
> On Thu, 3 Nov 2022, at 07:18, Feng Tang wrote:
> > On Wed, Nov 02, 2022 at 04:16:06PM +0900, Hyeonggon Yoo wrote:
> >> On Wed, Nov 02, 2022 at 02:08:09PM +0800, Feng Tang wrote:
> > [...]
> >> > > transfer started ......................................... transfer ok, time=2.11s
> >> > > setting up elf image... OK
> >> > > jumping to kernel code
> >> > > zimage at:     80B842A0 810B4BC0
> >> > > 
> >> > > Uncompressing Linux at load address 80001000
> >> > > 
> >> > > Copy device tree to address  80B80EE0
> >> > > 
> >> > > Now, booting the kernel...
> >> > > 
> >> > > [    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #73 SMP Wed Nov  2 05:10:01 AEST 2022
> >> > > [    0.000000] ------------[ cut here ]------------
> >> > > [    0.000000] WARNING: CPU: 0 PID: 0 at mm/slub.c:3416 kmem_cache_alloc+0x5a4/0x5e8
> >> > > [    0.000000] Modules linked in:
> >> > > [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 6.1.0-rc3+ #73
> >> > > [    0.000000] Stack : 810fff78 80084d98 00000000 00000004 00000000 00000000 80889d04 80c90000
> >> > > [    0.000000]         80920000 807bd328 8089d368 80923bd3 00000000 00000001 80889cb0 00000000
> >> > > [    0.000000]         00000000 00000000 807bd328 8084bcb1 00000002 00000002 00000001 6d6f4320
> >> > > [    0.000000]         00000000 80c97d3d 80c97d68 fffffffc 807bd328 00000000 00000000 00000000
> >> > > [    0.000000]         00000000 a0000000 80910000 8110a0b4 00000000 00000020 80010000 80010000
> >> > > [    0.000000]         ...
> >> > > [    0.000000] Call Trace:
> >> > > [    0.000000] [<80008260>] show_stack+0x28/0xf0
> >> > > [    0.000000] [<8070c958>] dump_stack_lvl+0x60/0x80
> >> > > [    0.000000] [<8002e184>] __warn+0xc4/0xf8
> >> > > [    0.000000] [<8002e210>] warn_slowpath_fmt+0x58/0xa4
> >> > > [    0.000000] [<801c0fac>] kmem_cache_alloc+0x5a4/0x5e8
> >> > > [    0.000000] [<8092856c>] prom_soc_init+0x1fc/0x2b4
> >> > > [    0.000000] [<80928060>] prom_init+0x44/0xf0
> >> > > [    0.000000] [<80929214>] setup_arch+0x4c/0x6a8
> >> > > [    0.000000] [<809257e0>] start_kernel+0x88/0x7c0
> >> > > [    0.000000] 
> >> > > [    0.000000] ---[ end trace 0000000000000000 ]---
> >> > > [    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3
> >> > > [    0.000000] printk: bootconsole [early0] enabled
> >> > > 
> >> > > Thank you for working through this with me.
> >> > > I will try to address the root cause in mt7621.c.
> >> > > It looks like other arch/** soc_device_register users use postcore_initcall, device_initcall,
> >> > > or the ARM DT_MACHINE_START .init_machine. A quick hack to use postcore_initcall in mt7621
> >> > > avoided this zero ptr kmem_cache passed to kmem_cache_alloc_lru.
> >> > 
> >> > If IIUC, the prom_soc_init() is only called once in kernel, can the
> >> > 'soc_dev_attr' just be defined as a global data structure instead
> >> > of calling kzalloc(), as its size is small only containing 7 pointers.
> >> 
> >> But soc_device_registers() too uses kmalloc. I think calling it
> >> after slab initialization will be best solution - if that is correct.
> >
> > Yes, you are right, there is other kmalloc() down the call chain.
> >
> > Hi John,
> >
> > Will you verify and submit a patch for your proposal of deferring
> > calling prom_soc_init()? thanks
> >
> > - Feng
> 
> Hi Feng,
> 
> My proposed mt7621.c changes are RFC here:
> https://lore.kernel.org/lkml/20221103050538.1930758-1-git@johnthomson.fastmail.com.au/

Great!

> That series lets me boot the v6.1-rc3 kernel. I have only tried it with my config (as sent earlier). If there are other suspect config settings that I should test, please let me know?
> I used device_initcall, but postcore_initcall also works fine.

I'm not sure which order is better, due to lack of mips platform
knowledge.

> I rephrased Vlastimil's explanation and used it in patch 3 description.
> I have not referenced a Fixes tag yet (unsure which/if any I should use)

With older version, the kernel boots fine with soc_dev_init() not
being actually called, and I don't know if they also need to get
this called.

Thanks,
Feng

> Cheers,
> -- 
>   John Thomson
> 

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[Vlastimil Babka]

On 11/1/22 11:33, John Thomson wrote:
> On Tue, 1 Nov 2022, at 09:31, Hyeonggon Yoo wrote:
>> On Tue, Nov 01, 2022 at 09:20:21AM +0000, John Thomson wrote:
>>> On Tue, 1 Nov 2022, at 07:57, Feng Tang wrote:
>>> > Hi Thomson,
>>> >
>>> > Thanks for testing!
>>> >
>>> > + mips maintainer and mail list. The original report is here
>>> >
>>> > https://lore.kernel.org/lkml/becf2ac3-2a90-4f3a-96d9-a70f67c66e4a@app.fastmail.com/
>>>
>>> I am guessing my issue comes from __kmem_cache_alloc_lru accessing s->object_size when (kmem_cache) s is NULL?
>>> If that is the case, this change is not to blame, it only exposes the issue?
>>> 
>>> I get the following dmesg (note very early NULL kmem_cache) with the below change atop v6.1-rc3:
>>> 
>>> transfer started ......................................... transfer ok, time=2.02s
>>> setting up elf image... OK
>>> jumping to kernel code
>>> zimage at:     80B842A0 810B4EFC
>>> 
>>> Uncompressing Linux at load address 80001000
>>> 
>>> Copy device tree to address  80B80EE0
>>> 
>>> Now, booting the kernel...
>>> 
>>> [    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #61 SMP Tue Nov  1 18:04:13 AEST 2022
>>> [    0.000000] slub: kmem_cache_alloc called with kmem_cache: 0x0
>>> [    0.000000] slub: __kmem_cache_alloc_lru called with kmem_cache: 0x0
>>> [    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3
>>> [    0.000000] printk: bootconsole [early0] enabled
>>> [    0.000000] CPU0 revision is: 0001992f (MIPS 1004Kc)
>>> [    0.000000] MIPS: machine is MikroTik RouterBOARD 760iGS
>>> 
>>> normal boot
>>> 
>>> 
>>> diff --git a/mm/slub.c b/mm/slub.c
>>> index 157527d7101b..10fcdf2520d2 100644
>>> --- a/mm/slub.c
>>> +++ b/mm/slub.c
>>> @@ -3410,7 +3410,13 @@ static __always_inline
>>>  void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
>>>  			     gfp_t gfpflags)
>>>  {
>>> -	void *ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
>>> +	void *ret;
>>> +	if (IS_ERR_OR_NULL(s)) {
>>> +		pr_warn("slub: __kmem_cache_alloc_lru called with kmem_cache: %pSR\n", s);
>>> +		ret = slab_alloc(s, lru, gfpflags, _RET_IP_, 0);
>>> +	} else {
>>> +		ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
>>> +	}
>>>  
>>>  	trace_kmem_cache_alloc(_RET_IP_, ret, s, gfpflags, NUMA_NO_NODE);
>>>  
>>> @@ -3419,6 +3425,8 @@ void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
>>>  
>>>  void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
>>>  {
>>> +	if (IS_ERR_OR_NULL(s))
>>> +		pr_warn("slub: kmem_cache_alloc called with kmem_cache: %pSR\n", s);
>>>  	return __kmem_cache_alloc_lru(s, NULL, gfpflags);
>>>  }
>>>  EXPORT_SYMBOL(kmem_cache_alloc);
>>> @@ -3426,6 +3434,8 @@ EXPORT_SYMBOL(kmem_cache_alloc);
>>>  void *kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
>>>  			   gfp_t gfpflags)
>>>  {
>>> +	if (IS_ERR_OR_NULL(s))
>>> +		pr_warn("slub: __kmem_cache_alloc_lru called with kmem_cache: %pSR\n", s);
>>>  	return __kmem_cache_alloc_lru(s, lru, gfpflags);
>>>  }
>>>  EXPORT_SYMBOL(kmem_cache_alloc_lru);
>>> 
>>> 
>>> Any hints on where kmem_cache_alloc would be being called from this early?
>>> I will start looking from /init/main.c around pr_notice("%s", linux_banner);
>>
>> Great. Would you try calling dump_stack(); when we observed s == NULL?
>> That would give more information about who passed s == NULL to these
>> functions.
>>
> 
> With the dump_stack() in place:
> 
> Now, booting the kernel...
> 
> [    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #62 SMP Tue Nov  1 19:49:52 AEST 2022
> [    0.000000] slub: __kmem_cache_alloc_lru called with kmem_cache ptr: 0x0
> [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 6.1.0-rc3+ #62
> [    0.000000] Stack : 810fff78 80084d98 80889d00 00000004 00000000 00000000 80889d5c 80c90000
> [    0.000000]         80920000 807bd380 8089d368 80923bd3 00000000 00000001 80889d08 00000000
> [    0.000000]         00000000 00000000 807bd380 8084bd51 00000002 00000002 00000001 6d6f4320
> [    0.000000]         00000000 80c97ce9 80c97d14 fffffffc 807bd380 00000000 00000003 00000dc0
> [    0.000000]         00000000 a0000000 80910000 8110a0b4 00000000 00000020 80010000 80010000
> [    0.000000]         ...
> [    0.000000] Call Trace:
> [    0.000000] [<80008260>] show_stack+0x28/0xf0
> [    0.000000] [<8070cdc0>] dump_stack_lvl+0x60/0x80
> [    0.000000] [<801c1428>] kmem_cache_alloc+0x5c0/0x740
> [    0.000000] [<8092856c>] prom_soc_init+0x1fc/0x2b4
> [    0.000000] [<80928060>] prom_init+0x44/0xf0
> [    0.000000] [<80929214>] setup_arch+0x4c/0x6a8
> [    0.000000] [<809257e0>] start_kernel+0x88/0x7c0
> [    0.000000] 
> [    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3

The stack means CONFIG_TRACING=n, is that right?

That would mean
prom_soc_init()
  soc_dev_init()
    kzalloc() -> kmalloc()
      kmalloc_trace()  // after #else /* CONFIG_TRACING */
        kmem_cache_alloc(s, flags);

Looks like this path is a small bug in the wasting detection patch, as we
throw away size there.

AFAICS before this patch, we "survive" "kmem_cache *s" being NULL as
slab_pre_alloc_hook() will happen to return NULL and we bail out from
slab_alloc_node(). But this is a side-effect, not an intended protection.
Also the CONFIG_TRACING variant of kmalloc_trace() would have called
trace_kmalloc dereferencing s->size anyway even before this patch.

I don't think we should add WARNS in the slab hot paths just to prevent this
rare error of using slab too early. At most VM_WARN... would be acceptable
but still not necessary as crashing immediately from a NULL pointer is
sufficient.

So IMHO mips should fix their soc init, and we should look into the
CONFIG_TRACING=n variant of kmalloc_trace(), to pass orig_size properly.

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[Feng Tang]

On Wed, Nov 02, 2022 at 04:22:37PM +0800, Vlastimil Babka wrote:
> On 11/1/22 11:33, John Thomson wrote:
[...]
> > 
> > [    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #62 SMP Tue Nov  1 19:49:52 AEST 2022
> > [    0.000000] slub: __kmem_cache_alloc_lru called with kmem_cache ptr: 0x0
> > [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 6.1.0-rc3+ #62
> > [    0.000000] Stack : 810fff78 80084d98 80889d00 00000004 00000000 00000000 80889d5c 80c90000
> > [    0.000000]         80920000 807bd380 8089d368 80923bd3 00000000 00000001 80889d08 00000000
> > [    0.000000]         00000000 00000000 807bd380 8084bd51 00000002 00000002 00000001 6d6f4320
> > [    0.000000]         00000000 80c97ce9 80c97d14 fffffffc 807bd380 00000000 00000003 00000dc0
> > [    0.000000]         00000000 a0000000 80910000 8110a0b4 00000000 00000020 80010000 80010000
> > [    0.000000]         ...
> > [    0.000000] Call Trace:
> > [    0.000000] [<80008260>] show_stack+0x28/0xf0
> > [    0.000000] [<8070cdc0>] dump_stack_lvl+0x60/0x80
> > [    0.000000] [<801c1428>] kmem_cache_alloc+0x5c0/0x740
> > [    0.000000] [<8092856c>] prom_soc_init+0x1fc/0x2b4
> > [    0.000000] [<80928060>] prom_init+0x44/0xf0
> > [    0.000000] [<80929214>] setup_arch+0x4c/0x6a8
> > [    0.000000] [<809257e0>] start_kernel+0x88/0x7c0
> > [    0.000000] 
> > [    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3
> 
> The stack means CONFIG_TRACING=n, is that right?
 
Yes, from the kconfig, CONFIG_TRACING is not set.

> That would mean
> prom_soc_init()
>   soc_dev_init()
>     kzalloc() -> kmalloc()
>       kmalloc_trace()  // after #else /* CONFIG_TRACING */
>         kmem_cache_alloc(s, flags);
> 
> Looks like this path is a small bug in the wasting detection patch, as we
> throw away size there.

Yes, from the code reading and log from John, it is.

One strange thing is, I reset the code to v6.0, and found that 
__kmem_cache_alloc_lru() also access the 's->object_size'

void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
			     gfp_t gfpflags)
{
	void *ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
	...
}

And from John's dump_stack() info, this call is also where the NULL pointer
happens, which I still can't figue out.

> AFAICS before this patch, we "survive" "kmem_cache *s" being NULL as
> slab_pre_alloc_hook() will happen to return NULL and we bail out from
> slab_alloc_node(). But this is a side-effect, not an intended protection.
> Also the CONFIG_TRACING variant of kmalloc_trace() would have called
> trace_kmalloc dereferencing s->size anyway even before this patch.
> 
> I don't think we should add WARNS in the slab hot paths just to prevent this
> rare error of using slab too early. At most VM_WARN... would be acceptable
> but still not necessary as crashing immediately from a NULL pointer is
> sufficient.
> 
> So IMHO mips should fix their soc init, 

Yes, for the mips fix, John has proposed to defer the calling of prom_soc_init(),
which looks reasonable.

> and we should look into the
> CONFIG_TRACING=n variant of kmalloc_trace(), to pass orig_size properly.

You mean check if the pointer is NULL and bail out early. 

Thanks,
Feng

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[Vlastimil Babka]

On 11/3/22 06:54, Feng Tang wrote:
> On Wed, Nov 02, 2022 at 04:22:37PM +0800, Vlastimil Babka wrote:
>> On 11/1/22 11:33, John Thomson wrote:
> [...]
>> > 
>> > [    0.000000] Linux version 6.1.0-rc3+ (john@john) (mipsel-buildroot-linux-gnu-gcc.br_real (Buildroot 2021.11-4428-g6b6741b) 12.2.0, GNU ld (GNU Binutils) 2.39) #62 SMP Tue Nov  1 19:49:52 AEST 2022
>> > [    0.000000] slub: __kmem_cache_alloc_lru called with kmem_cache ptr: 0x0
>> > [    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 6.1.0-rc3+ #62
>> > [    0.000000] Stack : 810fff78 80084d98 80889d00 00000004 00000000 00000000 80889d5c 80c90000
>> > [    0.000000]         80920000 807bd380 8089d368 80923bd3 00000000 00000001 80889d08 00000000
>> > [    0.000000]         00000000 00000000 807bd380 8084bd51 00000002 00000002 00000001 6d6f4320
>> > [    0.000000]         00000000 80c97ce9 80c97d14 fffffffc 807bd380 00000000 00000003 00000dc0
>> > [    0.000000]         00000000 a0000000 80910000 8110a0b4 00000000 00000020 80010000 80010000
>> > [    0.000000]         ...
>> > [    0.000000] Call Trace:
>> > [    0.000000] [<80008260>] show_stack+0x28/0xf0
>> > [    0.000000] [<8070cdc0>] dump_stack_lvl+0x60/0x80
>> > [    0.000000] [<801c1428>] kmem_cache_alloc+0x5c0/0x740
>> > [    0.000000] [<8092856c>] prom_soc_init+0x1fc/0x2b4
>> > [    0.000000] [<80928060>] prom_init+0x44/0xf0
>> > [    0.000000] [<80929214>] setup_arch+0x4c/0x6a8
>> > [    0.000000] [<809257e0>] start_kernel+0x88/0x7c0
>> > [    0.000000] 
>> > [    0.000000] SoC Type: MediaTek MT7621 ver:1 eco:3
>> 
>> The stack means CONFIG_TRACING=n, is that right?
>  
> Yes, from the kconfig, CONFIG_TRACING is not set.
> 
>> That would mean
>> prom_soc_init()
>>   soc_dev_init()
>>     kzalloc() -> kmalloc()
>>       kmalloc_trace()  // after #else /* CONFIG_TRACING */
>>         kmem_cache_alloc(s, flags);
>> 
>> Looks like this path is a small bug in the wasting detection patch, as we
>> throw away size there.
> 
> Yes, from the code reading and log from John, it is.
> 
> One strange thing is, I reset the code to v6.0, and found that 
> __kmem_cache_alloc_lru() also access the 's->object_size'
> 
> void *__kmem_cache_alloc_lru(struct kmem_cache *s, struct list_lru *lru,
> 			     gfp_t gfpflags)
> {
> 	void *ret = slab_alloc(s, lru, gfpflags, _RET_IP_, s->object_size);
> 	...
> }
> 
> And from John's dump_stack() info, this call is also where the NULL pointer
> happens, which I still can't figue out.
> 
>> AFAICS before this patch, we "survive" "kmem_cache *s" being NULL as
>> slab_pre_alloc_hook() will happen to return NULL and we bail out from
>> slab_alloc_node(). But this is a side-effect, not an intended protection.
>> Also the CONFIG_TRACING variant of kmalloc_trace() would have called
>> trace_kmalloc dereferencing s->size anyway even before this patch.
>> 
>> I don't think we should add WARNS in the slab hot paths just to prevent this
>> rare error of using slab too early. At most VM_WARN... would be acceptable
>> but still not necessary as crashing immediately from a NULL pointer is
>> sufficient.
>> 
>> So IMHO mips should fix their soc init, 
> 
> Yes, for the mips fix, John has proposed to defer the calling of prom_soc_init(),
> which looks reasonable.
> 
>> and we should look into the
>> CONFIG_TRACING=n variant of kmalloc_trace(), to pass orig_size properly.
> 
> You mean check if the pointer is NULL and bail out early. 

No I mean here:

#else /* CONFIG_TRACING */
/* Save a function call when CONFIG_TRACING=n */
static __always_inline __alloc_size(3)                                   
void *kmalloc_trace(struct kmem_cache *s, gfp_t flags, size_t size)
{       
        void *ret = kmem_cache_alloc(s, flags);
                    
        ret = kasan_kmalloc(s, ret, size, flags);
        return ret;
}

we call kmem_cache_alloc() and discard the size parameter, so it will assume
s->object_size (and as the side-effect, crash if s is NULL). We shouldn't
add "s is NULL?" checks, but fix passing the size - probably switch to
__kmem_cache_alloc_node()? and in the following kmalloc_node_trace() analogically.

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[Feng Tang]

On Thu, Nov 03, 2022 at 09:33:28AM +0100, Vlastimil Babka wrote:
[...]
> >> AFAICS before this patch, we "survive" "kmem_cache *s" being NULL as
> >> slab_pre_alloc_hook() will happen to return NULL and we bail out from
> >> slab_alloc_node(). But this is a side-effect, not an intended protection.
> >> Also the CONFIG_TRACING variant of kmalloc_trace() would have called
> >> trace_kmalloc dereferencing s->size anyway even before this patch.
> >> 
> >> I don't think we should add WARNS in the slab hot paths just to prevent this
> >> rare error of using slab too early. At most VM_WARN... would be acceptable
> >> but still not necessary as crashing immediately from a NULL pointer is
> >> sufficient.
> >> 
> >> So IMHO mips should fix their soc init, 
> > 
> > Yes, for the mips fix, John has proposed to defer the calling of prom_soc_init(),
> > which looks reasonable.
> > 
> >> and we should look into the
> >> CONFIG_TRACING=n variant of kmalloc_trace(), to pass orig_size properly.
> > 
> > You mean check if the pointer is NULL and bail out early. 
> 
> No I mean here:
> 
> #else /* CONFIG_TRACING */
> /* Save a function call when CONFIG_TRACING=n */
> static __always_inline __alloc_size(3)                                   
> void *kmalloc_trace(struct kmem_cache *s, gfp_t flags, size_t size)
> {       
>         void *ret = kmem_cache_alloc(s, flags);
>                     
>         ret = kasan_kmalloc(s, ret, size, flags);
>         return ret;
> }
> 
> we call kmem_cache_alloc() and discard the size parameter, so it will assume
> s->object_size (and as the side-effect, crash if s is NULL). We shouldn't
> add "s is NULL?" checks, but fix passing the size - probably switch to
> __kmem_cache_alloc_node()? and in the following kmalloc_node_trace() analogically.
 
Got it, thanks! I might have missed it during some rebasing for the
kmalloc wastage debug patch.

How about the following fix?

Thanks,
Feng

---
From 9f9fa9da8946fd44625f873c0f51167357075be1 Mon Sep 17 00:00:00 2001
From: Feng Tang <feng.tang@intel.com>
Date: Thu, 3 Nov 2022 21:32:10 +0800
Subject: [PATCH] mm/slub: Add missing orig_size parameter for wastage debug

commit 6edf2576a6cc ("mm/slub: enable debugging memory wasting of
kmalloc") was introduced for debugging kmalloc memory wastage,
and it missed to pass the original request size for kmalloc_trace()
and kmalloc_node_trace() in CONFIG_TRACING=n path.

Fix it by using __kmem_cache_alloc_node() with correct original
request size.

Fixes: 6edf2576a6cc ("mm/slub: enable debugging memory wasting of kmalloc")
Suggested-by: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Feng Tang <feng.tang@intel.com>
---
 include/linux/slab.h | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 90877fcde70b..9691afa569e1 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -469,6 +469,9 @@ void *__kmalloc_node(size_t size, gfp_t flags, int node) __assume_kmalloc_alignm
 							 __alloc_size(1);
 void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t flags, int node) __assume_slab_alignment
 									 __malloc;
+void *__kmem_cache_alloc_node(struct kmem_cache *s, gfp_t flags, int node,
+				size_t orig_size, unsigned long caller) __assume_slab_alignment
+									 __malloc;
 
 #ifdef CONFIG_TRACING
 void *kmalloc_trace(struct kmem_cache *s, gfp_t flags, size_t size)
@@ -482,7 +485,8 @@ void *kmalloc_node_trace(struct kmem_cache *s, gfp_t gfpflags,
 static __always_inline __alloc_size(3)
 void *kmalloc_trace(struct kmem_cache *s, gfp_t flags, size_t size)
 {
-	void *ret = kmem_cache_alloc(s, flags);
+	void *ret = __kmem_cache_alloc_node(s, flags, NUMA_NO_NODE,
+					    size, _RET_IP_);
 
 	ret = kasan_kmalloc(s, ret, size, flags);
 	return ret;
@@ -492,7 +496,8 @@ static __always_inline __alloc_size(4)
 void *kmalloc_node_trace(struct kmem_cache *s, gfp_t gfpflags,
 			 int node, size_t size)
 {
-	void *ret = kmem_cache_alloc_node(s, gfpflags, node);
+	void *ret = __kmem_cache_alloc_node(s, gfpflags, node,
+					    size, _RET_IP_);
 
 	ret = kasan_kmalloc(s, ret, size, gfpflags);
 	return ret;
-- 
2.34.1

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[Hyeonggon Yoo]

On Thu, Nov 03, 2022 at 10:16:12PM +0800, Feng Tang wrote:
> On Thu, Nov 03, 2022 at 09:33:28AM +0100, Vlastimil Babka wrote:
> [...]
> > >> AFAICS before this patch, we "survive" "kmem_cache *s" being NULL as
> > >> slab_pre_alloc_hook() will happen to return NULL and we bail out from
> > >> slab_alloc_node(). But this is a side-effect, not an intended protection.
> > >> Also the CONFIG_TRACING variant of kmalloc_trace() would have called
> > >> trace_kmalloc dereferencing s->size anyway even before this patch.
> > >> 
> > >> I don't think we should add WARNS in the slab hot paths just to prevent this
> > >> rare error of using slab too early. At most VM_WARN... would be acceptable
> > >> but still not necessary as crashing immediately from a NULL pointer is
> > >> sufficient.
> > >> 
> > >> So IMHO mips should fix their soc init, 
> > > 
> > > Yes, for the mips fix, John has proposed to defer the calling of prom_soc_init(),
> > > which looks reasonable.
> > > 
> > >> and we should look into the
> > >> CONFIG_TRACING=n variant of kmalloc_trace(), to pass orig_size properly.
> > > 
> > > You mean check if the pointer is NULL and bail out early. 
> > 
> > No I mean here:
> > 
> > #else /* CONFIG_TRACING */
> > /* Save a function call when CONFIG_TRACING=n */
> > static __always_inline __alloc_size(3)                                   
> > void *kmalloc_trace(struct kmem_cache *s, gfp_t flags, size_t size)
> > {       
> >         void *ret = kmem_cache_alloc(s, flags);
> >                     
> >         ret = kasan_kmalloc(s, ret, size, flags);
> >         return ret;
> > }
> > 
> > we call kmem_cache_alloc() and discard the size parameter, so it will assume
> > s->object_size (and as the side-effect, crash if s is NULL). We shouldn't
> > add "s is NULL?" checks, but fix passing the size - probably switch to
> > __kmem_cache_alloc_node()? and in the following kmalloc_node_trace() analogically.
>  
> Got it, thanks! I might have missed it during some rebasing for the
> kmalloc wastage debug patch.

That was good catch and I missed too!
But FYI I'm suggesting to drop CONFIG_TRACING=n variant:

https://lore.kernel.org/linux-mm/20221101222520.never.109-kees@kernel.org/T/#m20ecf14390e406247bde0ea9cce368f469c539ed

Any thoughts?

> 
> How about the following fix?
> 
> Thanks,
> Feng
> 
> ---
> From 9f9fa9da8946fd44625f873c0f51167357075be1 Mon Sep 17 00:00:00 2001
> From: Feng Tang <feng.tang@intel.com>
> Date: Thu, 3 Nov 2022 21:32:10 +0800
> Subject: [PATCH] mm/slub: Add missing orig_size parameter for wastage debug
> 
> commit 6edf2576a6cc ("mm/slub: enable debugging memory wasting of
> kmalloc") was introduced for debugging kmalloc memory wastage,
> and it missed to pass the original request size for kmalloc_trace()
> and kmalloc_node_trace() in CONFIG_TRACING=n path.
> 
> Fix it by using __kmem_cache_alloc_node() with correct original
> request size.
> 
> Fixes: 6edf2576a6cc ("mm/slub: enable debugging memory wasting of kmalloc")
> Suggested-by: Vlastimil Babka <vbabka@suse.cz>
> Signed-off-by: Feng Tang <feng.tang@intel.com>
> ---
>  include/linux/slab.h | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 90877fcde70b..9691afa569e1 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -469,6 +469,9 @@ void *__kmalloc_node(size_t size, gfp_t flags, int node) __assume_kmalloc_alignm
>  							 __alloc_size(1);
>  void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t flags, int node) __assume_slab_alignment
>  									 __malloc;
> +void *__kmem_cache_alloc_node(struct kmem_cache *s, gfp_t flags, int node,
> +				size_t orig_size, unsigned long caller) __assume_slab_alignment
> +									 __malloc;
>  
>  #ifdef CONFIG_TRACING
>  void *kmalloc_trace(struct kmem_cache *s, gfp_t flags, size_t size)
> @@ -482,7 +485,8 @@ void *kmalloc_node_trace(struct kmem_cache *s, gfp_t gfpflags,
>  static __always_inline __alloc_size(3)
>  void *kmalloc_trace(struct kmem_cache *s, gfp_t flags, size_t size)
>  {
> -	void *ret = kmem_cache_alloc(s, flags);
> +	void *ret = __kmem_cache_alloc_node(s, flags, NUMA_NO_NODE,
> +					    size, _RET_IP_);
>  
>  	ret = kasan_kmalloc(s, ret, size, flags);
>  	return ret;
> @@ -492,7 +496,8 @@ static __always_inline __alloc_size(4)
>  void *kmalloc_node_trace(struct kmem_cache *s, gfp_t gfpflags,
>  			 int node, size_t size)
>  {
> -	void *ret = kmem_cache_alloc_node(s, gfpflags, node);
> +	void *ret = __kmem_cache_alloc_node(s, gfpflags, node,
> +					    size, _RET_IP_);
>  
>  	ret = kasan_kmalloc(s, ret, size, gfpflags);
>  	return ret;
> -- 
> 2.34.1
> 
> 
> 

-- 
Thanks,
Hyeonggon

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[Vlastimil Babka]

On 11/3/22 15:36, Hyeonggon Yoo wrote:
> On Thu, Nov 03, 2022 at 10:16:12PM +0800, Feng Tang wrote:
>> On Thu, Nov 03, 2022 at 09:33:28AM +0100, Vlastimil Babka wrote:
>> [...]
>> > >> AFAICS before this patch, we "survive" "kmem_cache *s" being NULL as
>> > >> slab_pre_alloc_hook() will happen to return NULL and we bail out from
>> > >> slab_alloc_node(). But this is a side-effect, not an intended protection.
>> > >> Also the CONFIG_TRACING variant of kmalloc_trace() would have called
>> > >> trace_kmalloc dereferencing s->size anyway even before this patch.
>> > >> 
>> > >> I don't think we should add WARNS in the slab hot paths just to prevent this
>> > >> rare error of using slab too early. At most VM_WARN... would be acceptable
>> > >> but still not necessary as crashing immediately from a NULL pointer is
>> > >> sufficient.
>> > >> 
>> > >> So IMHO mips should fix their soc init, 
>> > > 
>> > > Yes, for the mips fix, John has proposed to defer the calling of prom_soc_init(),
>> > > which looks reasonable.
>> > > 
>> > >> and we should look into the
>> > >> CONFIG_TRACING=n variant of kmalloc_trace(), to pass orig_size properly.
>> > > 
>> > > You mean check if the pointer is NULL and bail out early. 
>> > 
>> > No I mean here:
>> > 
>> > #else /* CONFIG_TRACING */
>> > /* Save a function call when CONFIG_TRACING=n */
>> > static __always_inline __alloc_size(3)                                   
>> > void *kmalloc_trace(struct kmem_cache *s, gfp_t flags, size_t size)
>> > {       
>> >         void *ret = kmem_cache_alloc(s, flags);
>> >                     
>> >         ret = kasan_kmalloc(s, ret, size, flags);
>> >         return ret;
>> > }
>> > 
>> > we call kmem_cache_alloc() and discard the size parameter, so it will assume
>> > s->object_size (and as the side-effect, crash if s is NULL). We shouldn't
>> > add "s is NULL?" checks, but fix passing the size - probably switch to
>> > __kmem_cache_alloc_node()? and in the following kmalloc_node_trace() analogically.
>>  
>> Got it, thanks! I might have missed it during some rebasing for the
>> kmalloc wastage debug patch.
> 
> That was good catch and I missed too!
> But FYI I'm suggesting to drop CONFIG_TRACING=n variant:
> 
> https://lore.kernel.org/linux-mm/20221101222520.never.109-kees@kernel.org/T/#m20ecf14390e406247bde0ea9cce368f469c539ed
> 
> Any thoughts?

I'll get to it, also I think we were pondering that within your series too,
but I wanted to postpone in case somebody objects to the extra function call
it creates.
But that would be for 6.2 anyway while I'll collect the fix here for 6.1.

>> 
>> How about the following fix?
>> 
>> Thanks,
>> Feng
>> 
>> ---
>> From 9f9fa9da8946fd44625f873c0f51167357075be1 Mon Sep 17 00:00:00 2001
>> From: Feng Tang <feng.tang@intel.com>
>> Date: Thu, 3 Nov 2022 21:32:10 +0800
>> Subject: [PATCH] mm/slub: Add missing orig_size parameter for wastage debug
>> 
>> commit 6edf2576a6cc ("mm/slub: enable debugging memory wasting of
>> kmalloc") was introduced for debugging kmalloc memory wastage,
>> and it missed to pass the original request size for kmalloc_trace()
>> and kmalloc_node_trace() in CONFIG_TRACING=n path.
>> 
>> Fix it by using __kmem_cache_alloc_node() with correct original
>> request size.
>> 
>> Fixes: 6edf2576a6cc ("mm/slub: enable debugging memory wasting of kmalloc")
>> Suggested-by: Vlastimil Babka <vbabka@suse.cz>
>> Signed-off-by: Feng Tang <feng.tang@intel.com>
>> ---
>>  include/linux/slab.h | 9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>> 
>> diff --git a/include/linux/slab.h b/include/linux/slab.h
>> index 90877fcde70b..9691afa569e1 100644
>> --- a/include/linux/slab.h
>> +++ b/include/linux/slab.h
>> @@ -469,6 +469,9 @@ void *__kmalloc_node(size_t size, gfp_t flags, int node) __assume_kmalloc_alignm
>>  							 __alloc_size(1);
>>  void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t flags, int node) __assume_slab_alignment
>>  									 __malloc;
>> +void *__kmem_cache_alloc_node(struct kmem_cache *s, gfp_t flags, int node,
>> +				size_t orig_size, unsigned long caller) __assume_slab_alignment
>> +									 __malloc;
>>  
>>  #ifdef CONFIG_TRACING
>>  void *kmalloc_trace(struct kmem_cache *s, gfp_t flags, size_t size)
>> @@ -482,7 +485,8 @@ void *kmalloc_node_trace(struct kmem_cache *s, gfp_t gfpflags,
>>  static __always_inline __alloc_size(3)
>>  void *kmalloc_trace(struct kmem_cache *s, gfp_t flags, size_t size)
>>  {
>> -	void *ret = kmem_cache_alloc(s, flags);
>> +	void *ret = __kmem_cache_alloc_node(s, flags, NUMA_NO_NODE,
>> +					    size, _RET_IP_);
>>  
>>  	ret = kasan_kmalloc(s, ret, size, flags);
>>  	return ret;
>> @@ -492,7 +496,8 @@ static __always_inline __alloc_size(4)
>>  void *kmalloc_node_trace(struct kmem_cache *s, gfp_t gfpflags,
>>  			 int node, size_t size)
>>  {
>> -	void *ret = kmem_cache_alloc_node(s, gfpflags, node);
>> +	void *ret = __kmem_cache_alloc_node(s, gfpflags, node,
>> +					    size, _RET_IP_);
>>  
>>  	ret = kasan_kmalloc(s, ret, size, gfpflags);
>>  	return ret;
>> -- 
>> 2.34.1
>> 
>> 
>> 
> 

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[Vlastimil Babka]

On 11/3/22 17:57, Vlastimil Babka wrote:
> On 11/3/22 15:36, Hyeonggon Yoo wrote:
>> On Thu, Nov 03, 2022 at 10:16:12PM +0800, Feng Tang wrote:
>>> On Thu, Nov 03, 2022 at 09:33:28AM +0100, Vlastimil Babka wrote:
>>> [...]
>>> > >> AFAICS before this patch, we "survive" "kmem_cache *s" being NULL as
>>> > >> slab_pre_alloc_hook() will happen to return NULL and we bail out from
>>> > >> slab_alloc_node(). But this is a side-effect, not an intended protection.
>>> > >> Also the CONFIG_TRACING variant of kmalloc_trace() would have called
>>> > >> trace_kmalloc dereferencing s->size anyway even before this patch.
>>> > >> 
>>> > >> I don't think we should add WARNS in the slab hot paths just to prevent this
>>> > >> rare error of using slab too early. At most VM_WARN... would be acceptable
>>> > >> but still not necessary as crashing immediately from a NULL pointer is
>>> > >> sufficient.
>>> > >> 
>>> > >> So IMHO mips should fix their soc init, 
>>> > > 
>>> > > Yes, for the mips fix, John has proposed to defer the calling of prom_soc_init(),
>>> > > which looks reasonable.
>>> > > 
>>> > >> and we should look into the
>>> > >> CONFIG_TRACING=n variant of kmalloc_trace(), to pass orig_size properly.
>>> > > 
>>> > > You mean check if the pointer is NULL and bail out early. 
>>> > 
>>> > No I mean here:
>>> > 
>>> > #else /* CONFIG_TRACING */
>>> > /* Save a function call when CONFIG_TRACING=n */
>>> > static __always_inline __alloc_size(3)                                   
>>> > void *kmalloc_trace(struct kmem_cache *s, gfp_t flags, size_t size)
>>> > {       
>>> >         void *ret = kmem_cache_alloc(s, flags);
>>> >                     
>>> >         ret = kasan_kmalloc(s, ret, size, flags);
>>> >         return ret;
>>> > }
>>> > 
>>> > we call kmem_cache_alloc() and discard the size parameter, so it will assume
>>> > s->object_size (and as the side-effect, crash if s is NULL). We shouldn't
>>> > add "s is NULL?" checks, but fix passing the size - probably switch to
>>> > __kmem_cache_alloc_node()? and in the following kmalloc_node_trace() analogically.
>>>  
>>> Got it, thanks! I might have missed it during some rebasing for the
>>> kmalloc wastage debug patch.
>> 
>> That was good catch and I missed too!
>> But FYI I'm suggesting to drop CONFIG_TRACING=n variant:
>> 
>> https://lore.kernel.org/linux-mm/20221101222520.never.109-kees@kernel.org/T/#m20ecf14390e406247bde0ea9cce368f469c539ed
>> 
>> Any thoughts?
> 
> I'll get to it, also I think we were pondering that within your series too,
> but I wanted to postpone in case somebody objects to the extra function call
> it creates.
> But that would be for 6.2 anyway while I'll collect the fix here for 6.1.

On second thought, the fix is making the inlined kmalloc_trace() expand to a
call that had 2 parameters and now it has 5, which seems to me like a worse
thing (code bloat) than the function call. With the other reasons to ditch
the CONFIG_TRACING=n variant I'm inclined to just do it right now.

>>> 
>>> How about the following fix?
>>> 
>>> Thanks,
>>> Feng
>>> 
>>> ---
>>> From 9f9fa9da8946fd44625f873c0f51167357075be1 Mon Sep 17 00:00:00 2001
>>> From: Feng Tang <feng.tang@intel.com>
>>> Date: Thu, 3 Nov 2022 21:32:10 +0800
>>> Subject: [PATCH] mm/slub: Add missing orig_size parameter for wastage debug
>>> 
>>> commit 6edf2576a6cc ("mm/slub: enable debugging memory wasting of
>>> kmalloc") was introduced for debugging kmalloc memory wastage,
>>> and it missed to pass the original request size for kmalloc_trace()
>>> and kmalloc_node_trace() in CONFIG_TRACING=n path.
>>> 
>>> Fix it by using __kmem_cache_alloc_node() with correct original
>>> request size.
>>> 
>>> Fixes: 6edf2576a6cc ("mm/slub: enable debugging memory wasting of kmalloc")
>>> Suggested-by: Vlastimil Babka <vbabka@suse.cz>
>>> Signed-off-by: Feng Tang <feng.tang@intel.com>
>>> ---
>>>  include/linux/slab.h | 9 +++++++--
>>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/include/linux/slab.h b/include/linux/slab.h
>>> index 90877fcde70b..9691afa569e1 100644
>>> --- a/include/linux/slab.h
>>> +++ b/include/linux/slab.h
>>> @@ -469,6 +469,9 @@ void *__kmalloc_node(size_t size, gfp_t flags, int node) __assume_kmalloc_alignm
>>>  							 __alloc_size(1);
>>>  void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t flags, int node) __assume_slab_alignment
>>>  									 __malloc;
>>> +void *__kmem_cache_alloc_node(struct kmem_cache *s, gfp_t flags, int node,
>>> +				size_t orig_size, unsigned long caller) __assume_slab_alignment
>>> +									 __malloc;
>>>  
>>>  #ifdef CONFIG_TRACING
>>>  void *kmalloc_trace(struct kmem_cache *s, gfp_t flags, size_t size)
>>> @@ -482,7 +485,8 @@ void *kmalloc_node_trace(struct kmem_cache *s, gfp_t gfpflags,
>>>  static __always_inline __alloc_size(3)
>>>  void *kmalloc_trace(struct kmem_cache *s, gfp_t flags, size_t size)
>>>  {
>>> -	void *ret = kmem_cache_alloc(s, flags);
>>> +	void *ret = __kmem_cache_alloc_node(s, flags, NUMA_NO_NODE,
>>> +					    size, _RET_IP_);
>>>  
>>>  	ret = kasan_kmalloc(s, ret, size, flags);
>>>  	return ret;
>>> @@ -492,7 +496,8 @@ static __always_inline __alloc_size(4)
>>>  void *kmalloc_node_trace(struct kmem_cache *s, gfp_t gfpflags,
>>>  			 int node, size_t size)
>>>  {
>>> -	void *ret = kmem_cache_alloc_node(s, gfpflags, node);
>>> +	void *ret = __kmem_cache_alloc_node(s, gfpflags, node,
>>> +					    size, _RET_IP_);
>>>  
>>>  	ret = kasan_kmalloc(s, ret, size, gfpflags);
>>>  	return ret;
>>> -- 
>>> 2.34.1
>>> 
>>> 
>>> 
>> 
> 

Re: [PATCH v6 1/4] mm/slub: enable debugging memory wasting of kmalloc

[Feng Tang]

On Thu, Nov 03, 2022 at 06:35:53PM +0100, Vlastimil Babka wrote:

[...]

> >> But FYI I'm suggesting to drop CONFIG_TRACING=n variant:
> >> 
> >> https://lore.kernel.org/linux-mm/20221101222520.never.109-kees@kernel.org/T/#m20ecf14390e406247bde0ea9cce368f469c539ed
> >> 
> >> Any thoughts?
> > 
> > I'll get to it, also I think we were pondering that within your series too,
> > but I wanted to postpone in case somebody objects to the extra function call
> > it creates.
> > But that would be for 6.2 anyway while I'll collect the fix here for 6.1.
> 
> On second thought, the fix is making the inlined kmalloc_trace() expand to a
> call that had 2 parameters and now it has 5, which seems to me like a worse
> thing (code bloat) than the function call. With the other reasons to ditch
> the CONFIG_TRACING=n variant I'm inclined to just do it right now.

That's great! It will save much trouble, and reduce code complexity.

Btw, the patch below also has some compiling issue for some kconfig
(thanks to 0Day's kbuild bot).

Thanks,
Feng

> >>> 
> >>> How about the following fix?
> >>> 
> >>> Thanks,
> >>> Feng
> >>> 
> >>> ---
> >>> From 9f9fa9da8946fd44625f873c0f51167357075be1 Mon Sep 17 00:00:00 2001
> >>> From: Feng Tang <feng.tang@intel.com>
> >>> Date: Thu, 3 Nov 2022 21:32:10 +0800
> >>> Subject: [PATCH] mm/slub: Add missing orig_size parameter for wastage debug
> >>> 
> >>> commit 6edf2576a6cc ("mm/slub: enable debugging memory wasting of
> >>> kmalloc") was introduced for debugging kmalloc memory wastage,
> >>> and it missed to pass the original request size for kmalloc_trace()
> >>> and kmalloc_node_trace() in CONFIG_TRACING=n path.
> >>> 
> >>> Fix it by using __kmem_cache_alloc_node() with correct original
> >>> request size.
> >>> 
> >>> Fixes: 6edf2576a6cc ("mm/slub: enable debugging memory wasting of kmalloc")
> >>> Suggested-by: Vlastimil Babka <vbabka@suse.cz>
> >>> Signed-off-by: Feng Tang <feng.tang@intel.com>
> >>> ---
> >>>  include/linux/slab.h | 9 +++++++--
> >>>  1 file changed, 7 insertions(+), 2 deletions(-)
> >>> 
> >>> diff --git a/include/linux/slab.h b/include/linux/slab.h
> >>> index 90877fcde70b..9691afa569e1 100644
> >>> --- a/include/linux/slab.h
> >>> +++ b/include/linux/slab.h
> >>> @@ -469,6 +469,9 @@ void *__kmalloc_node(size_t size, gfp_t flags, int node) __assume_kmalloc_alignm
> >>>  							 __alloc_size(1);
> >>>  void *kmem_cache_alloc_node(struct kmem_cache *s, gfp_t flags, int node) __assume_slab_alignment
> >>>  									 __malloc;
> >>> +void *__kmem_cache_alloc_node(struct kmem_cache *s, gfp_t flags, int node,
> >>> +				size_t orig_size, unsigned long caller) __assume_slab_alignment
> >>> +									 __malloc;
> >>>  
> >>>  #ifdef CONFIG_TRACING
> >>>  void *kmalloc_trace(struct kmem_cache *s, gfp_t flags, size_t size)
> >>> @@ -482,7 +485,8 @@ void *kmalloc_node_trace(struct kmem_cache *s, gfp_t gfpflags,
> >>>  static __always_inline __alloc_size(3)
> >>>  void *kmalloc_trace(struct kmem_cache *s, gfp_t flags, size_t size)
> >>>  {
> >>> -	void *ret = kmem_cache_alloc(s, flags);
> >>> +	void *ret = __kmem_cache_alloc_node(s, flags, NUMA_NO_NODE,
> >>> +					    size, _RET_IP_);
> >>>  
> >>>  	ret = kasan_kmalloc(s, ret, size, flags);
> >>>  	return ret;
> >>> @@ -492,7 +496,8 @@ static __always_inline __alloc_size(4)
> >>>  void *kmalloc_node_trace(struct kmem_cache *s, gfp_t gfpflags,
> >>>  			 int node, size_t size)
> >>>  {
> >>> -	void *ret = kmem_cache_alloc_node(s, gfpflags, node);
> >>> +	void *ret = __kmem_cache_alloc_node(s, gfpflags, node,
> >>> +					    size, _RET_IP_);
> >>>  
> >>>  	ret = kasan_kmalloc(s, ret, size, gfpflags);
> >>>  	return ret;
> >>> -- 
> >>> 2.34.1
> >>> 
> >>> 
> >>> 
> >> 
> > 
> 
>