include/linux/slab.h | 6 ++++++ include/linux/stackdepot.h | 3 ++- mm/slub.c | 18 +++++++++++++++--- 3 files changed, 23 insertions(+), 4 deletions(-)
commit ba10d4b46655 ("mm/slub: use stackdepot to save stack trace in
objects") initializes stack depot in cache creation if SLAB_STORE_USER
flag is set.
This can make kernel crash because a cache can be crashed in various
contexts. For example if user sets slub_debug=U, kernel crashes
because create_boot_cache() calls stack_depot_init(), which tries to
allocate hash table using memblock_alloc() if slab is not available.
But memblock is also not available at that time.
This patch solves the problem by initializing stack depot early
in boot process if SLAB_STORE_USER debug flag is set globally
or the flag is set for at least one cache.
[ elver@google.com: initialize stack depot depending on slub_debug
parameter instead of allowing stack_depot_init() to be called
during kmem_cache_init() for simplicity. ]
[ vbabka@suse.cz: parse slub_debug parameter in setup_slub_debug()
and initialize stack depot in stack_depot_early_init(). ]
Link: https://lore.kernel.org/all/YhyeaP8lrzKgKm5A@ip-172-31-19-208.ap-northeast-1.compute.internal/
Fixes: ba10d4b46655 ("mm/slub: use stackdepot to save stack trace in objects")
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
---
include/linux/slab.h | 6 ++++++
include/linux/stackdepot.h | 3 ++-
mm/slub.c | 18 +++++++++++++++---
3 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 37bde99b74af..062128e0db10 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -763,6 +763,12 @@ extern void kvfree_sensitive(const void *addr, size_t len);
unsigned int kmem_cache_size(struct kmem_cache *s);
void __init kmem_cache_init_late(void);
+#if defined(CONFIG_SLUB_DEBUG) && defined(CONFIG_STACKDEPOT)
+int slab_stack_depot_init(void);
+#else
+int slab_stack_depot_init(void) { return 0; }
+#endif
+
#if defined(CONFIG_SMP) && defined(CONFIG_SLAB)
int slab_prepare_cpu(unsigned int cpu);
int slab_dead_cpu(unsigned int cpu);
diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
index 17f992fe6355..a813a2673c48 100644
--- a/include/linux/stackdepot.h
+++ b/include/linux/stackdepot.h
@@ -12,6 +12,7 @@
#define _LINUX_STACKDEPOT_H
#include <linux/gfp.h>
+#include <linux/slab.h>
typedef u32 depot_stack_handle_t;
@@ -32,7 +33,7 @@ int stack_depot_init(void);
#ifdef CONFIG_STACKDEPOT_ALWAYS_INIT
static inline int stack_depot_early_init(void) { return stack_depot_init(); }
#else
-static inline int stack_depot_early_init(void) { return 0; }
+static inline int stack_depot_early_init(void) { return slab_stack_depot_init(); }
#endif
depot_stack_handle_t stack_depot_save(unsigned long *entries,
diff --git a/mm/slub.c b/mm/slub.c
index a74afe59a403..2419fc3cc9f3 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -646,6 +646,14 @@ static slab_flags_t slub_debug;
static char *slub_debug_string;
static int disable_higher_order_debug;
+static bool init_stack_depot;
+
+int slab_stack_depot_init(void)
+{
+ if (init_stack_depot)
+ stack_depot_init();
+ return 0;
+}
/*
* slub is about to manipulate internal object metadata. This memory lies
@@ -1531,6 +1539,8 @@ static int __init setup_slub_debug(char *str)
global_slub_debug_changed = true;
} else {
slab_list_specified = true;
+ if (flags & SLAB_STORE_USER)
+ init_stack_depot = true;
}
}
@@ -1546,6 +1556,10 @@ static int __init setup_slub_debug(char *str)
global_flags = slub_debug;
slub_debug_string = saved_str;
}
+
+ if (global_flags & SLAB_STORE_USER)
+ init_stack_depot = true;
+
out:
slub_debug = global_flags;
if (slub_debug != 0 || slub_debug_string)
@@ -1556,6 +1570,7 @@ static int __init setup_slub_debug(char *str)
static_branch_unlikely(&init_on_free)) &&
(slub_debug & SLAB_POISON))
pr_info("mem auto-init: SLAB_POISON will take precedence over init_on_alloc/init_on_free\n");
+
return 1;
}
@@ -4221,9 +4236,6 @@ static int kmem_cache_open(struct kmem_cache *s, slab_flags_t flags)
s->remote_node_defrag_ratio = 1000;
#endif
- if (s->flags & SLAB_STORE_USER && IS_ENABLED(CONFIG_STACKDEPOT))
- stack_depot_init();
-
/* Initialize the pre-computed randomized freelist if slab is up */
if (slab_state >= UP) {
if (init_cache_random_seq(s))
--
2.33.1On Tue, Mar 01, 2022 at 03:36:38AM +0000, Hyeonggon Yoo wrote:
> commit ba10d4b46655 ("mm/slub: use stackdepot to save stack trace in
> objects") initializes stack depot in cache creation if SLAB_STORE_USER
> flag is set.
>
> This can make kernel crash because a cache can be crashed in various
> contexts. For example if user sets slub_debug=U, kernel crashes
> because create_boot_cache() calls stack_depot_init(), which tries to
> allocate hash table using memblock_alloc() if slab is not available.
> But memblock is also not available at that time.
>
> This patch solves the problem by initializing stack depot early
> in boot process if SLAB_STORE_USER debug flag is set globally
> or the flag is set for at least one cache.
>
Hello Vlastimil, would you pick this up into slub-stackdepot-v1,
or fold it into original patch (2/5)?
Thanks!
--
Thank you, You are awesome!
Hyeonggon :-)
> [ elver@google.com: initialize stack depot depending on slub_debug
> parameter instead of allowing stack_depot_init() to be called
> during kmem_cache_init() for simplicity. ]
>
> [ vbabka@suse.cz: parse slub_debug parameter in setup_slub_debug()
> and initialize stack depot in stack_depot_early_init(). ]
>
> Link: https://lore.kernel.org/all/YhyeaP8lrzKgKm5A@ip-172-31-19-208.ap-northeast-1.compute.internal/
> Fixes: ba10d4b46655 ("mm/slub: use stackdepot to save stack trace in objects")
> Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> ---
> include/linux/slab.h | 6 ++++++
> include/linux/stackdepot.h | 3 ++-
> mm/slub.c | 18 +++++++++++++++---
> 3 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 37bde99b74af..062128e0db10 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -763,6 +763,12 @@ extern void kvfree_sensitive(const void *addr, size_t len);
> unsigned int kmem_cache_size(struct kmem_cache *s);
> void __init kmem_cache_init_late(void);
>
> +#if defined(CONFIG_SLUB_DEBUG) && defined(CONFIG_STACKDEPOT)
> +int slab_stack_depot_init(void);
> +#else
> +int slab_stack_depot_init(void) { return 0; }
> +#endif
> +
> #if defined(CONFIG_SMP) && defined(CONFIG_SLAB)
> int slab_prepare_cpu(unsigned int cpu);
> int slab_dead_cpu(unsigned int cpu);
> diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
> index 17f992fe6355..a813a2673c48 100644
> --- a/include/linux/stackdepot.h
> +++ b/include/linux/stackdepot.h
> @@ -12,6 +12,7 @@
> #define _LINUX_STACKDEPOT_H
>
> #include <linux/gfp.h>
> +#include <linux/slab.h>
>
> typedef u32 depot_stack_handle_t;
>
> @@ -32,7 +33,7 @@ int stack_depot_init(void);
> #ifdef CONFIG_STACKDEPOT_ALWAYS_INIT
> static inline int stack_depot_early_init(void) { return stack_depot_init(); }
> #else
> -static inline int stack_depot_early_init(void) { return 0; }
> +static inline int stack_depot_early_init(void) { return slab_stack_depot_init(); }
> #endif
>
> depot_stack_handle_t stack_depot_save(unsigned long *entries,
> diff --git a/mm/slub.c b/mm/slub.c
> index a74afe59a403..2419fc3cc9f3 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -646,6 +646,14 @@ static slab_flags_t slub_debug;
>
> static char *slub_debug_string;
> static int disable_higher_order_debug;
> +static bool init_stack_depot;
> +
> +int slab_stack_depot_init(void)
> +{
> + if (init_stack_depot)
> + stack_depot_init();
> + return 0;
> +}
>
> /*
> * slub is about to manipulate internal object metadata. This memory lies
> @@ -1531,6 +1539,8 @@ static int __init setup_slub_debug(char *str)
> global_slub_debug_changed = true;
> } else {
> slab_list_specified = true;
> + if (flags & SLAB_STORE_USER)
> + init_stack_depot = true;
> }
> }
>
> @@ -1546,6 +1556,10 @@ static int __init setup_slub_debug(char *str)
> global_flags = slub_debug;
> slub_debug_string = saved_str;
> }
> +
> + if (global_flags & SLAB_STORE_USER)
> + init_stack_depot = true;
> +
> out:
> slub_debug = global_flags;
> if (slub_debug != 0 || slub_debug_string)
> @@ -1556,6 +1570,7 @@ static int __init setup_slub_debug(char *str)
> static_branch_unlikely(&init_on_free)) &&
> (slub_debug & SLAB_POISON))
> pr_info("mem auto-init: SLAB_POISON will take precedence over init_on_alloc/init_on_free\n");
> +
> return 1;
> }
>
> @@ -4221,9 +4236,6 @@ static int kmem_cache_open(struct kmem_cache *s, slab_flags_t flags)
> s->remote_node_defrag_ratio = 1000;
> #endif
>
> - if (s->flags & SLAB_STORE_USER && IS_ENABLED(CONFIG_STACKDEPOT))
> - stack_depot_init();
> -
> /* Initialize the pre-computed randomized freelist if slab is up */
> if (slab_state >= UP) {
> if (init_cache_random_seq(s))
> --
> 2.33.1
Hi Hyeonggon,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on vbabka/slub-stackdepot-v1]
[also build test ERROR on linus/master v5.17-rc6 next-20220228]
[cannot apply to hnaz-mm/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Hyeonggon-Yoo/mm-slub-initialize-stack-depot-in-boot-process/20220301-113825
base: https://git.kernel.org/pub/scm/linux/kernel/git/vbabka/linux.git slub-stackdepot-v1
config: m68k-randconfig-r014-20220301 (https://download.01.org/0day-ci/archive/20220301/202203011429.a87fKdU7-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/dd9dbeec7444b13b510dc4a863e9593d1799f965
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Hyeonggon-Yoo/mm-slub-initialize-stack-depot-in-boot-process/20220301-113825
git checkout dd9dbeec7444b13b510dc4a863e9593d1799f965
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=m68k SHELL=/bin/bash arch/m68k/coldfire/ mm/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All error/warnings (new ones prefixed by >>):
In file included from include/linux/irq.h:21,
from include/asm-generic/hardirq.h:17,
from ./arch/m68k/include/generated/asm/hardirq.h:1,
from include/linux/hardirq.h:11,
from include/linux/interrupt.h:11,
from include/linux/kernel_stat.h:9,
from arch/m68k/kernel/asm-offsets.c:16:
>> include/linux/slab.h:769:5: warning: no previous prototype for 'slab_stack_depot_init' [-Wmissing-prototypes]
769 | int slab_stack_depot_init(void) { return 0; }
| ^~~~~~~~~~~~~~~~~~~~~
--
In file included from include/linux/irq.h:21,
from arch/m68k/coldfire/vectors.c:14:
>> include/linux/slab.h:769:5: warning: no previous prototype for 'slab_stack_depot_init' [-Wmissing-prototypes]
769 | int slab_stack_depot_init(void) { return 0; }
| ^~~~~~~~~~~~~~~~~~~~~
arch/m68k/coldfire/vectors.c:43:13: warning: no previous prototype for 'trap_init' [-Wmissing-prototypes]
43 | void __init trap_init(void)
| ^~~~~~~~~
--
In file included from include/linux/irq.h:21,
from include/asm-generic/hardirq.h:17,
from ./arch/m68k/include/generated/asm/hardirq.h:1,
from include/linux/hardirq.h:11,
from include/linux/interrupt.h:11,
from arch/m68k/coldfire/intc-simr.c:16:
>> include/linux/slab.h:769:5: warning: no previous prototype for 'slab_stack_depot_init' [-Wmissing-prototypes]
769 | int slab_stack_depot_init(void) { return 0; }
| ^~~~~~~~~~~~~~~~~~~~~
arch/m68k/coldfire/intc-simr.c:177:13: warning: no previous prototype for 'init_IRQ' [-Wmissing-prototypes]
177 | void __init init_IRQ(void)
| ^~~~~~~~
--
In file included from include/linux/irq.h:21,
from include/linux/gpio/driver.h:7,
from arch/m68k/coldfire/gpio.c:12:
>> include/linux/slab.h:769:5: warning: no previous prototype for 'slab_stack_depot_init' [-Wmissing-prototypes]
769 | int slab_stack_depot_init(void) { return 0; }
| ^~~~~~~~~~~~~~~~~~~~~
arch/m68k/coldfire/gpio.c:19:5: warning: no previous prototype for '__mcfgpio_get_value' [-Wmissing-prototypes]
19 | int __mcfgpio_get_value(unsigned gpio)
| ^~~~~~~~~~~~~~~~~~~
arch/m68k/coldfire/gpio.c:25:6: warning: no previous prototype for '__mcfgpio_set_value' [-Wmissing-prototypes]
25 | void __mcfgpio_set_value(unsigned gpio, int value)
| ^~~~~~~~~~~~~~~~~~~
arch/m68k/coldfire/gpio.c:50:5: warning: no previous prototype for '__mcfgpio_direction_input' [-Wmissing-prototypes]
50 | int __mcfgpio_direction_input(unsigned gpio)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
arch/m68k/coldfire/gpio.c:65:5: warning: no previous prototype for '__mcfgpio_direction_output' [-Wmissing-prototypes]
65 | int __mcfgpio_direction_output(unsigned gpio, int value)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~
arch/m68k/coldfire/gpio.c:96:5: warning: no previous prototype for '__mcfgpio_request' [-Wmissing-prototypes]
96 | int __mcfgpio_request(unsigned gpio)
| ^~~~~~~~~~~~~~~~~
arch/m68k/coldfire/gpio.c:102:6: warning: no previous prototype for '__mcfgpio_free' [-Wmissing-prototypes]
102 | void __mcfgpio_free(unsigned gpio)
| ^~~~~~~~~~~~~~
--
In file included from include/linux/stackdepot.h:15,
from include/linux/page_ext.h:7,
from include/linux/mm.h:25,
from mm/page_alloc.c:19:
>> include/linux/slab.h:769:5: warning: no previous prototype for 'slab_stack_depot_init' [-Wmissing-prototypes]
769 | int slab_stack_depot_init(void) { return 0; }
| ^~~~~~~~~~~~~~~~~~~~~
mm/page_alloc.c:3820:15: warning: no previous prototype for 'should_fail_alloc_page' [-Wmissing-prototypes]
3820 | noinline bool should_fail_alloc_page(gfp_t gfp_mask, unsigned int order)
| ^~~~~~~~~~~~~~~~~~~~~~
--
In file included from include/linux/stackdepot.h:15,
from include/linux/page_ext.h:7,
from include/linux/mm.h:25,
from mm/page_poison.c:4:
>> include/linux/slab.h:769:5: warning: no previous prototype for 'slab_stack_depot_init' [-Wmissing-prototypes]
769 | int slab_stack_depot_init(void) { return 0; }
| ^~~~~~~~~~~~~~~~~~~~~
mm/page_poison.c:102:6: warning: no previous prototype for '__kernel_map_pages' [-Wmissing-prototypes]
102 | void __kernel_map_pages(struct page *page, int numpages, int enable)
| ^~~~~~~~~~~~~~~~~~
--
In file included from include/linux/stackdepot.h:15,
from include/linux/page_ext.h:7,
from include/linux/mm.h:25,
from mm/slub.c:13:
>> include/linux/slab.h:769:5: warning: no previous prototype for 'slab_stack_depot_init' [-Wmissing-prototypes]
769 | int slab_stack_depot_init(void) { return 0; }
| ^~~~~~~~~~~~~~~~~~~~~
>> mm/slub.c:651:5: error: redefinition of 'slab_stack_depot_init'
651 | int slab_stack_depot_init(void)
| ^~~~~~~~~~~~~~~~~~~~~
In file included from include/linux/stackdepot.h:15,
from include/linux/page_ext.h:7,
from include/linux/mm.h:25,
from mm/slub.c:13:
include/linux/slab.h:769:5: note: previous definition of 'slab_stack_depot_init' with type 'int(void)'
769 | int slab_stack_depot_init(void) { return 0; }
| ^~~~~~~~~~~~~~~~~~~~~
--
In file included from include/linux/irq.h:21,
from include/asm-generic/hardirq.h:17,
from ./arch/m68k/include/generated/asm/hardirq.h:1,
from include/linux/hardirq.h:11,
from include/linux/interrupt.h:11,
from include/linux/kernel_stat.h:9,
from arch/m68k/kernel/asm-offsets.c:16:
>> include/linux/slab.h:769:5: warning: no previous prototype for 'slab_stack_depot_init' [-Wmissing-prototypes]
769 | int slab_stack_depot_init(void) { return 0; }
| ^~~~~~~~~~~~~~~~~~~~~
vim +/slab_stack_depot_init +651 mm/slub.c
650
> 651 int slab_stack_depot_init(void)
652 {
653 if (init_stack_depot)
654 stack_depot_init();
655 return 0;
656 }
657
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Hyeonggon,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on vbabka/slub-stackdepot-v1]
[also build test ERROR on linus/master v5.17-rc6 next-20220228]
[cannot apply to hnaz-mm/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Hyeonggon-Yoo/mm-slub-initialize-stack-depot-in-boot-process/20220301-113825
base: https://git.kernel.org/pub/scm/linux/kernel/git/vbabka/linux.git slub-stackdepot-v1
config: i386-tinyconfig (https://download.01.org/0day-ci/archive/20220301/202203011512.U0o5cAx4-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/dd9dbeec7444b13b510dc4a863e9593d1799f965
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Hyeonggon-Yoo/mm-slub-initialize-stack-depot-in-boot-process/20220301-113825
git checkout dd9dbeec7444b13b510dc4a863e9593d1799f965
# save the config file to linux build tree
mkdir build_dir
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
ld: arch/x86/kernel/ebda.o: in function `slab_stack_depot_init':
>> ebda.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: init/main.o: in function `slab_stack_depot_init':
main.c:(.text+0x32): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: init/do_mounts.o: in function `slab_stack_depot_init':
do_mounts.c:(.text+0x5): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: init/noinitramfs.o: in function `slab_stack_depot_init':
noinitramfs.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: init/init_task.o: in function `slab_stack_depot_init':
init_task.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/entry/syscall_32.o: in function `slab_stack_depot_init':
syscall_32.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/entry/common.o: in function `slab_stack_depot_init':
common.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/entry/vdso/vma.o: in function `slab_stack_depot_init':
vma.c:(.text+0x1d0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/entry/vdso/extable.o: in function `slab_stack_depot_init':
extable.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/events/core.o: in function `slab_stack_depot_init':
core.c:(.text+0x903): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/events/amd/core.o: in function `slab_stack_depot_init':
core.c:(.text+0x70b): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/events/intel/core.o: in function `slab_stack_depot_init':
core.c:(.text+0x1cbc): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/events/intel/bts.o: in function `slab_stack_depot_init':
bts.c:(.text+0x6b2): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/events/intel/ds.o: in function `slab_stack_depot_init':
ds.c:(.text+0x14ad): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/events/intel/knc.o: in function `slab_stack_depot_init':
knc.c:(.text+0x2b1): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/events/intel/lbr.o: in function `slab_stack_depot_init':
lbr.c:(.text+0x6c8): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/events/intel/p4.o: in function `slab_stack_depot_init':
p4.c:(.text+0x5f8): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/events/intel/p6.o: in function `slab_stack_depot_init':
p6.c:(.text+0xc4): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/events/intel/pt.o: in function `slab_stack_depot_init':
pt.c:(.text+0x641): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/events/zhaoxin/core.o: in function `slab_stack_depot_init':
core.c:(.text+0x460): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/realmode/init.o: in function `slab_stack_depot_init':
init.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/process_32.o: in function `slab_stack_depot_init':
process_32.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/signal.o: in function `slab_stack_depot_init':
signal.c:(.text+0x5b4): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/traps.o: in function `slab_stack_depot_init':
traps.c:(.text+0x288): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/idt.o: in function `slab_stack_depot_init':
idt.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/irq.o: in function `slab_stack_depot_init':
irq.c:(.text+0x4f): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/irq_32.o: in function `slab_stack_depot_init':
irq_32.c:(.text+0x1): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/dumpstack_32.o: in function `slab_stack_depot_init':
dumpstack_32.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/time.o: in function `slab_stack_depot_init':
time.c:(.text+0xd): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/ioport.o: in function `slab_stack_depot_init':
ioport.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/dumpstack.o: in function `slab_stack_depot_init':
dumpstack.c:(.text+0x11e): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/nmi.o: in function `slab_stack_depot_init':
nmi.c:(.text+0x74): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/setup.o: in function `slab_stack_depot_init':
setup.c:(.text+0x3): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/x86_init.o: in function `slab_stack_depot_init':
x86_init.c:(.text+0x39): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/i8259.o: in function `slab_stack_depot_init':
i8259.c:(.text+0x2c4): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/irqinit.o: in function `slab_stack_depot_init':
irqinit.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/irq_work.o: in function `slab_stack_depot_init':
irq_work.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/probe_roms.o: in function `slab_stack_depot_init':
probe_roms.c:(.text+0x1a3): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/sys_ia32.o: in function `slab_stack_depot_init':
sys_ia32.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/bootflag.o: in function `slab_stack_depot_init':
bootflag.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/e820.o: in function `slab_stack_depot_init':
e820.c:(.text+0xbf): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/pci-dma.o: in function `slab_stack_depot_init':
pci-dma.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/quirks.o: in function `slab_stack_depot_init':
quirks.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/topology.o: in function `slab_stack_depot_init':
topology.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/kdebugfs.o: in function `slab_stack_depot_init':
kdebugfs.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/alternative.o: in function `slab_stack_depot_init':
alternative.c:(.text+0x320): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/hw_breakpoint.o: in function `slab_stack_depot_init':
hw_breakpoint.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/rtc.o: in function `slab_stack_depot_init':
rtc.c:(.text+0x41): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/pci-iommu_table.o: in function `slab_stack_depot_init':
pci-iommu_table.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/process.o: in function `slab_stack_depot_init':
process.c:(.text+0xcf): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
ld: arch/x86/kernel/fpu/init.o: in function `slab_stack_depot_init':
init.c:(.text+0x0): multiple definition of `slab_stack_depot_init'; arch/x86/kernel/head32.o:head32.c:(.text+0x0): first defined here
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
commit ba10d4b46655 ("mm/slub: use stackdepot to save stack trace in
objects") initializes stack depot in cache creation if SLAB_STORE_USER
flag is set.
This can make kernel crash because a cache can be crashed in various
contexts. For example if user sets slub_debug=U, kernel crashes
because create_boot_cache() calls stack_depot_init(), which tries to
allocate hash table using memblock_alloc() if slab is not available.
But memblock is also not available at that time.
This patch solves the problem by initializing stack depot early
in boot process if SLAB_STORE_USER debug flag is set globally
or the flag is set for at least one cache.
[ elver@google.com: initialize stack depot depending on slub_debug
parameter instead of allowing stack_depot_init() to be called
during kmem_cache_init() for simplicity. ]
[ vbabka@suse.cz: parse slub_debug parameter in setup_slub_debug()
and initialize stack depot in stack_depot_early_init(). ]
[ lkp@intel.com: Fix build error. ]
Link: https://lore.kernel.org/all/YhyeaP8lrzKgKm5A@ip-172-31-19-208.ap-northeast-1.compute.internal/
Fixes: ba10d4b46655 ("mm/slub: use stackdepot to save stack trace in objects")
Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
---
include/linux/slab.h | 1 +
include/linux/stackdepot.h | 3 ++-
mm/slab.c | 5 +++++
mm/slob.c | 5 +++++
mm/slub.c | 19 ++++++++++++++++---
5 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 37bde99b74af..d2b0f8f9e5e6 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -762,6 +762,7 @@ extern void kvfree_sensitive(const void *addr, size_t len);
unsigned int kmem_cache_size(struct kmem_cache *s);
void __init kmem_cache_init_late(void);
+int __init slab_stack_depot_init(void);
#if defined(CONFIG_SMP) && defined(CONFIG_SLAB)
int slab_prepare_cpu(unsigned int cpu);
diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
index 17f992fe6355..a813a2673c48 100644
--- a/include/linux/stackdepot.h
+++ b/include/linux/stackdepot.h
@@ -12,6 +12,7 @@
#define _LINUX_STACKDEPOT_H
#include <linux/gfp.h>
+#include <linux/slab.h>
typedef u32 depot_stack_handle_t;
@@ -32,7 +33,7 @@ int stack_depot_init(void);
#ifdef CONFIG_STACKDEPOT_ALWAYS_INIT
static inline int stack_depot_early_init(void) { return stack_depot_init(); }
#else
-static inline int stack_depot_early_init(void) { return 0; }
+static inline int stack_depot_early_init(void) { return slab_stack_depot_init(); }
#endif
depot_stack_handle_t stack_depot_save(unsigned long *entries,
diff --git a/mm/slab.c b/mm/slab.c
index ddf5737c63d9..c7f929665fbe 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -1196,6 +1196,11 @@ static void __init set_up_node(struct kmem_cache *cachep, int index)
}
}
+int __init slab_stack_depot_init(void)
+{
+ return 0;
+}
+
/*
* Initialisation. Called after the page allocator have been initialised and
* before smp_init().
diff --git a/mm/slob.c b/mm/slob.c
index 60c5842215f1..7597c219f061 100644
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -725,3 +725,8 @@ void __init kmem_cache_init_late(void)
{
slab_state = FULL;
}
+
+int __init slab_stack_depot_init(void)
+{
+ return 0;
+}
diff --git a/mm/slub.c b/mm/slub.c
index a74afe59a403..8f130f917977 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -646,6 +646,16 @@ static slab_flags_t slub_debug;
static char *slub_debug_string;
static int disable_higher_order_debug;
+static bool __initdata init_stack_depot;
+
+int __init slab_stack_depot_init(void)
+{
+#ifdef CONFIG_STACKDEPOT
+ if (init_stack_depot)
+ stack_depot_init();
+#endif
+ return 0;
+}
/*
* slub is about to manipulate internal object metadata. This memory lies
@@ -1531,6 +1541,8 @@ static int __init setup_slub_debug(char *str)
global_slub_debug_changed = true;
} else {
slab_list_specified = true;
+ if (flags & SLAB_STORE_USER)
+ init_stack_depot = true;
}
}
@@ -1546,6 +1558,10 @@ static int __init setup_slub_debug(char *str)
global_flags = slub_debug;
slub_debug_string = saved_str;
}
+
+ if (global_flags & SLAB_STORE_USER)
+ init_stack_depot = true;
+
out:
slub_debug = global_flags;
if (slub_debug != 0 || slub_debug_string)
@@ -4221,9 +4237,6 @@ static int kmem_cache_open(struct kmem_cache *s, slab_flags_t flags)
s->remote_node_defrag_ratio = 1000;
#endif
- if (s->flags & SLAB_STORE_USER && IS_ENABLED(CONFIG_STACKDEPOT))
- stack_depot_init();
-
/* Initialize the pre-computed randomized freelist if slab is up */
if (slab_state >= UP) {
if (init_cache_random_seq(s))
--
2.33.1On 3/1/22 09:51, Hyeonggon Yoo wrote:
> commit ba10d4b46655 ("mm/slub: use stackdepot to save stack trace in
> objects") initializes stack depot in cache creation if SLAB_STORE_USER
> flag is set.
As pointed out, this is not a stable commit, the series was just posted for
review and there will be v2. So instead of "this fixes the commit..." I
suggest writing the patch assuming it's a preparation for the patch
"mm/slub: use stackdepot"... and I can then make it part of the series.
So it should instead explain that for slub_debug we will need a way to
trigger stack_depot_early_init() based on boot options and so this patch
introduces it...
> This can make kernel crash because a cache can be crashed in various
> contexts. For example if user sets slub_debug=U, kernel crashes
> because create_boot_cache() calls stack_depot_init(), which tries to
> allocate hash table using memblock_alloc() if slab is not available.
> But memblock is also not available at that time.
>
> This patch solves the problem by initializing stack depot early
> in boot process if SLAB_STORE_USER debug flag is set globally
> or the flag is set for at least one cache.
>
> [ elver@google.com: initialize stack depot depending on slub_debug
> parameter instead of allowing stack_depot_init() to be called
> during kmem_cache_init() for simplicity. ]
>
> [ vbabka@suse.cz: parse slub_debug parameter in setup_slub_debug()
> and initialize stack depot in stack_depot_early_init(). ]
>
> [ lkp@intel.com: Fix build error. ]
>
> Link: https://lore.kernel.org/all/YhyeaP8lrzKgKm5A@ip-172-31-19-208.ap-northeast-1.compute.internal/
> Fixes: ba10d4b46655 ("mm/slub: use stackdepot to save stack trace in objects")
> Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> ---
> include/linux/slab.h | 1 +
> include/linux/stackdepot.h | 3 ++-
> mm/slab.c | 5 +++++
> mm/slob.c | 5 +++++
> mm/slub.c | 19 ++++++++++++++++---
> 5 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index 37bde99b74af..d2b0f8f9e5e6 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -762,6 +762,7 @@ extern void kvfree_sensitive(const void *addr, size_t len);
>
> unsigned int kmem_cache_size(struct kmem_cache *s);
> void __init kmem_cache_init_late(void);
> +int __init slab_stack_depot_init(void);
>
> #if defined(CONFIG_SMP) && defined(CONFIG_SLAB)
> int slab_prepare_cpu(unsigned int cpu);
> diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
> index 17f992fe6355..a813a2673c48 100644
> --- a/include/linux/stackdepot.h
> +++ b/include/linux/stackdepot.h
> @@ -12,6 +12,7 @@
> #define _LINUX_STACKDEPOT_H
>
> #include <linux/gfp.h>
> +#include <linux/slab.h>
>
> typedef u32 depot_stack_handle_t;
>
> @@ -32,7 +33,7 @@ int stack_depot_init(void);
> #ifdef CONFIG_STACKDEPOT_ALWAYS_INIT
> static inline int stack_depot_early_init(void) { return stack_depot_init(); }
> #else
> -static inline int stack_depot_early_init(void) { return 0; }
> +static inline int stack_depot_early_init(void) { return slab_stack_depot_init(); }
> #endif
I think the approach should be generic for stackdepot, not tied to a
function that belongs to slab with 3 different implementations.
E.g. in stackdepot.h declare a variable e.g. "stack_depot_want_early_init"
that is checked in stack_depot_early_init() above to call stack_depot_init().
> depot_stack_handle_t stack_depot_save(unsigned long *entries,
> diff --git a/mm/slab.c b/mm/slab.c
> index ddf5737c63d9..c7f929665fbe 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -1196,6 +1196,11 @@ static void __init set_up_node(struct kmem_cache *cachep, int index)
> }
> }
>
> +int __init slab_stack_depot_init(void)
> +{
> + return 0;
> +}
> +
> /*
> * Initialisation. Called after the page allocator have been initialised and
> * before smp_init().
> diff --git a/mm/slob.c b/mm/slob.c
> index 60c5842215f1..7597c219f061 100644
> --- a/mm/slob.c
> +++ b/mm/slob.c
> @@ -725,3 +725,8 @@ void __init kmem_cache_init_late(void)
> {
> slab_state = FULL;
> }
> +
> +int __init slab_stack_depot_init(void)
> +{
> + return 0;
> +}
> diff --git a/mm/slub.c b/mm/slub.c
> index a74afe59a403..8f130f917977 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -646,6 +646,16 @@ static slab_flags_t slub_debug;
>
> static char *slub_debug_string;
> static int disable_higher_order_debug;
> +static bool __initdata init_stack_depot;
> +
> +int __init slab_stack_depot_init(void)
> +{
> +#ifdef CONFIG_STACKDEPOT
> + if (init_stack_depot)
> + stack_depot_init();
> +#endif
> + return 0;
> +}
> /*
> * slub is about to manipulate internal object metadata. This memory lies
> @@ -1531,6 +1541,8 @@ static int __init setup_slub_debug(char *str)
> global_slub_debug_changed = true;
> } else {
> slab_list_specified = true;
> + if (flags & SLAB_STORE_USER)
> + init_stack_depot = true;
> }
> }
>
> @@ -1546,6 +1558,10 @@ static int __init setup_slub_debug(char *str)
> global_flags = slub_debug;
> slub_debug_string = saved_str;
> }
> +
> + if (global_flags & SLAB_STORE_USER)
> + init_stack_depot = true;
This looks good, it would just set the "stack_depot_want_early_init"
variable instead. But logically should be part of "mm/slub: use
stackdepot...", not part of patch that introduces the variable. That so if
you don't mind I would move it there with credit.
> out:
> slub_debug = global_flags;
> if (slub_debug != 0 || slub_debug_string)
> @@ -4221,9 +4237,6 @@ static int kmem_cache_open(struct kmem_cache *s, slab_flags_t flags)
> s->remote_node_defrag_ratio = 1000;
> #endif
>
> - if (s->flags & SLAB_STORE_USER && IS_ENABLED(CONFIG_STACKDEPOT))
> - stack_depot_init();
> -
> /* Initialize the pre-computed randomized freelist if slab is up */
> if (slab_state >= UP) {
> if (init_cache_random_seq(s))
On Tue, Mar 01, 2022 at 10:14:30AM +0100, Vlastimil Babka wrote:
> On 3/1/22 09:51, Hyeonggon Yoo wrote:
> > commit ba10d4b46655 ("mm/slub: use stackdepot to save stack trace in
> > objects") initializes stack depot in cache creation if SLAB_STORE_USER
> > flag is set.
>
> As pointed out, this is not a stable commit, the series was just posted for
> review and there will be v2. So instead of "this fixes the commit..." I
> suggest writing the patch assuming it's a preparation for the patch
> "mm/slub: use stackdepot"... and I can then make it part of the series.
> So it should instead explain that for slub_debug we will need a way to
> trigger stack_depot_early_init() based on boot options and so this patch
> introduces it...
>
Agreed.
> > This can make kernel crash because a cache can be crashed in various
> > contexts. For example if user sets slub_debug=U, kernel crashes
> > because create_boot_cache() calls stack_depot_init(), which tries to
> > allocate hash table using memblock_alloc() if slab is not available.
> > But memblock is also not available at that time.
> >
> > This patch solves the problem by initializing stack depot early
> > in boot process if SLAB_STORE_USER debug flag is set globally
> > or the flag is set for at least one cache.
> >
> > [ elver@google.com: initialize stack depot depending on slub_debug
> > parameter instead of allowing stack_depot_init() to be called
> > during kmem_cache_init() for simplicity. ]
> >
> > [ vbabka@suse.cz: parse slub_debug parameter in setup_slub_debug()
> > and initialize stack depot in stack_depot_early_init(). ]
> >
> > [ lkp@intel.com: Fix build error. ]
> >
> > Link: https://lore.kernel.org/all/YhyeaP8lrzKgKm5A@ip-172-31-19-208.ap-northeast-1.compute.internal/
> > Fixes: ba10d4b46655 ("mm/slub: use stackdepot to save stack trace in objects")
> > Signed-off-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> > ---
> > include/linux/slab.h | 1 +
> > include/linux/stackdepot.h | 3 ++-
> > mm/slab.c | 5 +++++
> > mm/slob.c | 5 +++++
> > mm/slub.c | 19 ++++++++++++++++---
> > 5 files changed, 29 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/linux/slab.h b/include/linux/slab.h
> > index 37bde99b74af..d2b0f8f9e5e6 100644
> > --- a/include/linux/slab.h
> > +++ b/include/linux/slab.h
> > @@ -762,6 +762,7 @@ extern void kvfree_sensitive(const void *addr, size_t len);
> >
> > unsigned int kmem_cache_size(struct kmem_cache *s);
> > void __init kmem_cache_init_late(void);
> > +int __init slab_stack_depot_init(void);
> >
> > #if defined(CONFIG_SMP) && defined(CONFIG_SLAB)
> > int slab_prepare_cpu(unsigned int cpu);
> > diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
> > index 17f992fe6355..a813a2673c48 100644
> > --- a/include/linux/stackdepot.h
> > +++ b/include/linux/stackdepot.h
> > @@ -12,6 +12,7 @@
> > #define _LINUX_STACKDEPOT_H
> >
> > #include <linux/gfp.h>
> > +#include <linux/slab.h>
> >
> > typedef u32 depot_stack_handle_t;
> >
> > @@ -32,7 +33,7 @@ int stack_depot_init(void);
> > #ifdef CONFIG_STACKDEPOT_ALWAYS_INIT
> > static inline int stack_depot_early_init(void) { return stack_depot_init(); }
> > #else
> > -static inline int stack_depot_early_init(void) { return 0; }
> > +static inline int stack_depot_early_init(void) { return slab_stack_depot_init(); }
> > #endif
>
> I think the approach should be generic for stackdepot, not tied to a
> function that belongs to slab with 3 different implementations.
> E.g. in stackdepot.h declare a variable e.g. "stack_depot_want_early_init"
> that is checked in stack_depot_early_init() above to call stack_depot_init().
Hmm yeah if we define it in stack depot, that would be nice.
I just didn't want to expose global variable that is specific to slub.
> > depot_stack_handle_t stack_depot_save(unsigned long *entries,
> > diff --git a/mm/slab.c b/mm/slab.c
> > index ddf5737c63d9..c7f929665fbe 100644
> > --- a/mm/slab.c
> > +++ b/mm/slab.c
> > @@ -1196,6 +1196,11 @@ static void __init set_up_node(struct kmem_cache *cachep, int index)
> > }
> > }
> >
> > +int __init slab_stack_depot_init(void)
> > +{
> > + return 0;
> > +}
> > +
> > /*
> > * Initialisation. Called after the page allocator have been initialised and
> > * before smp_init().
> > diff --git a/mm/slob.c b/mm/slob.c
> > index 60c5842215f1..7597c219f061 100644
> > --- a/mm/slob.c
> > +++ b/mm/slob.c
> > @@ -725,3 +725,8 @@ void __init kmem_cache_init_late(void)
> > {
> > slab_state = FULL;
> > }
> > +
> > +int __init slab_stack_depot_init(void)
> > +{
> > + return 0;
> > +}
> > diff --git a/mm/slub.c b/mm/slub.c
> > index a74afe59a403..8f130f917977 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -646,6 +646,16 @@ static slab_flags_t slub_debug;
> >
> > static char *slub_debug_string;
> > static int disable_higher_order_debug;
> > +static bool __initdata init_stack_depot;
> > +
> > +int __init slab_stack_depot_init(void)
> > +{
> > +#ifdef CONFIG_STACKDEPOT
> > + if (init_stack_depot)
> > + stack_depot_init();
> > +#endif
> > + return 0;
> > +}
>
>
>
> > /*
> > * slub is about to manipulate internal object metadata. This memory lies
> > @@ -1531,6 +1541,8 @@ static int __init setup_slub_debug(char *str)
> > global_slub_debug_changed = true;
> > } else {
> > slab_list_specified = true;
> > + if (flags & SLAB_STORE_USER)
> > + init_stack_depot = true;
> > }
> > }
> >
> > @@ -1546,6 +1558,10 @@ static int __init setup_slub_debug(char *str)
> > global_flags = slub_debug;
> > slub_debug_string = saved_str;
> > }
> > +
> > + if (global_flags & SLAB_STORE_USER)
> > + init_stack_depot = true;
>
> This looks good, it would just set the "stack_depot_want_early_init"
> variable instead. But logically should be part of "mm/slub: use
> stackdepot...", not part of patch that introduces the variable. That so if
> you don't mind I would move it there with credit.
>
Oh I don't mind that.
Then I will expect this to be solved in next series...
I'm looking forward to next version. Thanks!
--
Thank you, You are awesome!
Hyeonggon :-)
> > out:
> > slub_debug = global_flags;
> > if (slub_debug != 0 || slub_debug_string)
> > @@ -4221,9 +4237,6 @@ static int kmem_cache_open(struct kmem_cache *s, slab_flags_t flags)
> > s->remote_node_defrag_ratio = 1000;
> > #endif
> >
> > - if (s->flags & SLAB_STORE_USER && IS_ENABLED(CONFIG_STACKDEPOT))
> > - stack_depot_init();
> > -
> > /* Initialize the pre-computed randomized freelist if slab is up */
> > if (slab_state >= UP) {
> > if (init_cache_random_seq(s))
>
© 2016 - 2026 Red Hat, Inc.