* [PATCH v2] stackdepot: Make max number of pools build-time configurable @ 2025-07-04 12:06 Matt Fleming 2025-07-04 20:35 ` Andrew Morton 2025-07-05 6:00 ` kernel test robot 0 siblings, 2 replies; 7+ messages in thread From: Matt Fleming @ 2025-07-04 12:06 UTC (permalink / raw) To: Andrew Morton Cc: linux-kernel, kernel-team, Marco Elver, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Oscar Salvador, Vlastimil Babka, Matt Fleming From: Matt Fleming <mfleming@cloudflare.com> We're hitting the WARN in depot_init_pool() about reaching the stack depot limit because we have long stacks that don't dedup very well. Introduce a new config to allow users to set the number of maximum stack depot pools at build time. Also, turn the silent capping into a build-time assert to provide more obvious feedback when users set this value too high. Signed-off-by: Matt Fleming <mfleming@cloudflare.com> --- Changes in v2: - Replace BUILD_BUG_ON with static_assert() - Hide STACKDEPOT_MAX_POOLS behind EXPERT lib/Kconfig | 6 ++++++ lib/stackdepot.c | 8 ++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/Kconfig b/lib/Kconfig index b38849af6f13..092a2b69310b 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -720,6 +720,12 @@ config STACKDEPOT_MAX_FRAMES default 64 depends on STACKDEPOT +config STACKDEPOT_MAX_POOLS + int "Maximum number of stack depot pools to store stack traces" if EXPERT + range 1024 131071 + default 8192 + depends on STACKDEPOT + config REF_TRACKER bool depends on STACKTRACE_SUPPORT diff --git a/lib/stackdepot.c b/lib/stackdepot.c index 245d5b416699..1c25c40f31f9 100644 --- a/lib/stackdepot.c +++ b/lib/stackdepot.c @@ -36,11 +36,11 @@ #include <linux/memblock.h> #include <linux/kasan-enabled.h> -#define DEPOT_POOLS_CAP 8192 +#define DEPOT_MAX_POOLS CONFIG_STACKDEPOT_MAX_POOLS + /* The pool_index is offset by 1 so the first record does not have a 0 handle. */ -#define DEPOT_MAX_POOLS \ - (((1LL << (DEPOT_POOL_INDEX_BITS)) - 1 < DEPOT_POOLS_CAP) ? \ - (1LL << (DEPOT_POOL_INDEX_BITS)) - 1 : DEPOT_POOLS_CAP) +static_assert(DEPOT_MAX_POOLS <= (1LL << (DEPOT_POOL_INDEX_BITS)) - 1); + static bool stack_depot_disabled; static bool __stack_depot_early_init_requested __initdata = IS_ENABLED(CONFIG_STACKDEPOT_ALWAYS_INIT); -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] stackdepot: Make max number of pools build-time configurable 2025-07-04 12:06 [PATCH v2] stackdepot: Make max number of pools build-time configurable Matt Fleming @ 2025-07-04 20:35 ` Andrew Morton 2025-07-07 8:12 ` Marco Elver 2025-07-05 6:00 ` kernel test robot 1 sibling, 1 reply; 7+ messages in thread From: Andrew Morton @ 2025-07-04 20:35 UTC (permalink / raw) To: Matt Fleming Cc: linux-kernel, kernel-team, Marco Elver, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Oscar Salvador, Vlastimil Babka, Matt Fleming On Fri, 4 Jul 2025 13:06:04 +0100 Matt Fleming <matt@readmodwrite.com> wrote: > From: Matt Fleming <mfleming@cloudflare.com> > > We're hitting the WARN in depot_init_pool() about reaching the stack > depot limit because we have long stacks that don't dedup very well. > > Introduce a new config to allow users to set the number of maximum stack > depot pools at build time. Also, turn the silent capping into a > build-time assert to provide more obvious feedback when users set this > value too high. > > ... > > --- a/lib/Kconfig > +++ b/lib/Kconfig > @@ -720,6 +720,12 @@ config STACKDEPOT_MAX_FRAMES > default 64 > depends on STACKDEPOT > > +config STACKDEPOT_MAX_POOLS > + int "Maximum number of stack depot pools to store stack traces" if EXPERT > + range 1024 131071 > + default 8192 > + depends on STACKDEPOT Do we need a range? If someone wants values outside the range they'll just edit the Kconfig, shrug. > config REF_TRACKER > bool > depends on STACKTRACE_SUPPORT > diff --git a/lib/stackdepot.c b/lib/stackdepot.c > index 245d5b416699..1c25c40f31f9 100644 > --- a/lib/stackdepot.c > +++ b/lib/stackdepot.c > @@ -36,11 +36,11 @@ > #include <linux/memblock.h> > #include <linux/kasan-enabled.h> > > -#define DEPOT_POOLS_CAP 8192 > +#define DEPOT_MAX_POOLS CONFIG_STACKDEPOT_MAX_POOLS A boot-time option would be much friendlier. Can we dynamically allocate stack_pools[]? Or possibly even runtime, although perhaps not a good effort-to-utility ratio. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] stackdepot: Make max number of pools build-time configurable 2025-07-04 20:35 ` Andrew Morton @ 2025-07-07 8:12 ` Marco Elver 2025-07-07 12:06 ` Matt Fleming 0 siblings, 1 reply; 7+ messages in thread From: Marco Elver @ 2025-07-07 8:12 UTC (permalink / raw) To: Andrew Morton Cc: Matt Fleming, linux-kernel, kernel-team, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Oscar Salvador, Vlastimil Babka, Matt Fleming On Fri, 4 Jul 2025 at 22:35, Andrew Morton <akpm@linux-foundation.org> wrote: > > On Fri, 4 Jul 2025 13:06:04 +0100 Matt Fleming <matt@readmodwrite.com> wrote: > > > From: Matt Fleming <mfleming@cloudflare.com> > > > > We're hitting the WARN in depot_init_pool() about reaching the stack > > depot limit because we have long stacks that don't dedup very well. > > > > Introduce a new config to allow users to set the number of maximum stack > > depot pools at build time. Also, turn the silent capping into a > > build-time assert to provide more obvious feedback when users set this > > value too high. > > > > ... > > > > --- a/lib/Kconfig > > +++ b/lib/Kconfig > > @@ -720,6 +720,12 @@ config STACKDEPOT_MAX_FRAMES > > default 64 > > depends on STACKDEPOT > > > > +config STACKDEPOT_MAX_POOLS > > + int "Maximum number of stack depot pools to store stack traces" if EXPERT > > + range 1024 131071 > > + default 8192 > > + depends on STACKDEPOT > > Do we need a range? If someone wants values outside the range they'll > just edit the Kconfig, shrug. > > > config REF_TRACKER > > bool > > depends on STACKTRACE_SUPPORT > > diff --git a/lib/stackdepot.c b/lib/stackdepot.c > > index 245d5b416699..1c25c40f31f9 100644 > > --- a/lib/stackdepot.c > > +++ b/lib/stackdepot.c > > @@ -36,11 +36,11 @@ > > #include <linux/memblock.h> > > #include <linux/kasan-enabled.h> > > > > -#define DEPOT_POOLS_CAP 8192 > > +#define DEPOT_MAX_POOLS CONFIG_STACKDEPOT_MAX_POOLS > > A boot-time option would be much friendlier. Can we dynamically > allocate stack_pools[]? We could allocate this in stack_depot_init(). I think it wasn't done so far as requiring a very large number of pools is typically indicative of stackdepot misuse. In any case, I agree it's better if this were configurable at boot time. However, need to be careful to sanitize user inputs (reject too small, reject too large). Matt, do you think this would work better for your usecase rather than rebuilding the kernel? > Or possibly even runtime, although perhaps not a good effort-to-utility > ratio. Could make stack_pools an RCU-protected array and resize on demand. But given stackdepot can be used from constrained contexts, the dance around RCU might be brittle (see existing RCU usage in stackdepot). Along with the fact that arbitrary resizing likely indicates a usage bug, it might be safer to just allocate on boot. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] stackdepot: Make max number of pools build-time configurable 2025-07-07 8:12 ` Marco Elver @ 2025-07-07 12:06 ` Matt Fleming 0 siblings, 0 replies; 7+ messages in thread From: Matt Fleming @ 2025-07-07 12:06 UTC (permalink / raw) To: Marco Elver Cc: Andrew Morton, linux-kernel, kernel-team, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Oscar Salvador, Vlastimil Babka, Matt Fleming On Mon, Jul 7, 2025 at 9:13 AM Marco Elver <elver@google.com> wrote: > > We could allocate this in stack_depot_init(). I think it wasn't done > so far as requiring a very large number of pools is typically > indicative of stackdepot misuse. > > In any case, I agree it's better if this were configurable at boot > time. However, need to be careful to sanitize user inputs (reject too > small, reject too large). > > Matt, do you think this would work better for your usecase rather than > rebuilding the kernel? Yep, that's a better option. I'll send a new version. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] stackdepot: Make max number of pools build-time configurable 2025-07-04 12:06 [PATCH v2] stackdepot: Make max number of pools build-time configurable Matt Fleming 2025-07-04 20:35 ` Andrew Morton @ 2025-07-05 6:00 ` kernel test robot 2025-07-07 6:39 ` Marco Elver 1 sibling, 1 reply; 7+ messages in thread From: kernel test robot @ 2025-07-05 6:00 UTC (permalink / raw) To: Matt Fleming, Andrew Morton Cc: oe-kbuild-all, Linux Memory Management List, linux-kernel, kernel-team, Marco Elver, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Oscar Salvador, Vlastimil Babka, Matt Fleming Hi Matt, kernel test robot noticed the following build errors: [auto build test ERROR on akpm-mm/mm-nonmm-unstable] [also build test ERROR on linus/master v6.16-rc4 next-20250704] [cannot apply to akpm-mm/mm-everything] [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#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Matt-Fleming/stackdepot-Make-max-number-of-pools-build-time-configurable/20250704-200804 base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-nonmm-unstable patch link: https://lore.kernel.org/r/20250704120604.2688934-1-matt%40readmodwrite.com patch subject: [PATCH v2] stackdepot: Make max number of pools build-time configurable config: arm64-randconfig-001-20250705 (https://download.01.org/0day-ci/archive/20250705/202507051300.E0JSHxu1-lkp@intel.com/config) compiler: aarch64-linux-gcc (GCC) 10.5.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250705/202507051300.E0JSHxu1-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202507051300.E0JSHxu1-lkp@intel.com/ All errors (new ones prefixed by >>): In file included from include/linux/init.h:5, from include/linux/printk.h:6, from include/asm-generic/bug.h:22, from arch/arm64/include/asm/bug.h:26, from include/linux/bug.h:5, from include/linux/vfsdebug.h:5, from include/linux/fs.h:5, from include/linux/debugfs.h:15, from lib/stackdepot.c:17: >> include/linux/build_bug.h:78:41: error: static assertion failed: "DEPOT_MAX_POOLS <= (1LL << (DEPOT_POOL_INDEX_BITS)) - 1" 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) | ^~~~~~~~~~~~~~ include/linux/build_bug.h:77:34: note: in expansion of macro '__static_assert' 77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr) | ^~~~~~~~~~~~~~~ lib/stackdepot.c:42:1: note: in expansion of macro 'static_assert' 42 | static_assert(DEPOT_MAX_POOLS <= (1LL << (DEPOT_POOL_INDEX_BITS)) - 1); | ^~~~~~~~~~~~~ vim +78 include/linux/build_bug.h bc6245e5efd70c Ian Abbott 2017-07-10 60 6bab69c65013be Rasmus Villemoes 2019-03-07 61 /** 6bab69c65013be Rasmus Villemoes 2019-03-07 62 * static_assert - check integer constant expression at build time 6bab69c65013be Rasmus Villemoes 2019-03-07 63 * 6bab69c65013be Rasmus Villemoes 2019-03-07 64 * static_assert() is a wrapper for the C11 _Static_assert, with a 6bab69c65013be Rasmus Villemoes 2019-03-07 65 * little macro magic to make the message optional (defaulting to the 6bab69c65013be Rasmus Villemoes 2019-03-07 66 * stringification of the tested expression). 6bab69c65013be Rasmus Villemoes 2019-03-07 67 * 6bab69c65013be Rasmus Villemoes 2019-03-07 68 * Contrary to BUILD_BUG_ON(), static_assert() can be used at global 6bab69c65013be Rasmus Villemoes 2019-03-07 69 * scope, but requires the expression to be an integer constant 6bab69c65013be Rasmus Villemoes 2019-03-07 70 * expression (i.e., it is not enough that __builtin_constant_p() is 6bab69c65013be Rasmus Villemoes 2019-03-07 71 * true for expr). 6bab69c65013be Rasmus Villemoes 2019-03-07 72 * 6bab69c65013be Rasmus Villemoes 2019-03-07 73 * Also note that BUILD_BUG_ON() fails the build if the condition is 6bab69c65013be Rasmus Villemoes 2019-03-07 74 * true, while static_assert() fails the build if the expression is 6bab69c65013be Rasmus Villemoes 2019-03-07 75 * false. 6bab69c65013be Rasmus Villemoes 2019-03-07 76 */ 6bab69c65013be Rasmus Villemoes 2019-03-07 77 #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr) 6bab69c65013be Rasmus Villemoes 2019-03-07 @78 #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) 6bab69c65013be Rasmus Villemoes 2019-03-07 79 07a368b3f55a79 Maxim Levitsky 2022-10-25 80 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] stackdepot: Make max number of pools build-time configurable 2025-07-05 6:00 ` kernel test robot @ 2025-07-07 6:39 ` Marco Elver 2025-07-07 12:05 ` Matt Fleming 0 siblings, 1 reply; 7+ messages in thread From: Marco Elver @ 2025-07-07 6:39 UTC (permalink / raw) To: kernel test robot Cc: Matt Fleming, Andrew Morton, oe-kbuild-all, Linux Memory Management List, linux-kernel, kernel-team, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Oscar Salvador, Vlastimil Babka, Matt Fleming On Sat, 5 Jul 2025 at 08:01, kernel test robot <lkp@intel.com> wrote: > > Hi Matt, > > kernel test robot noticed the following build errors: > > [auto build test ERROR on akpm-mm/mm-nonmm-unstable] > [also build test ERROR on linus/master v6.16-rc4 next-20250704] > [cannot apply to akpm-mm/mm-everything] > [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#_base_tree_information] > > url: https://github.com/intel-lab-lkp/linux/commits/Matt-Fleming/stackdepot-Make-max-number-of-pools-build-time-configurable/20250704-200804 > base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-nonmm-unstable > patch link: https://lore.kernel.org/r/20250704120604.2688934-1-matt%40readmodwrite.com > patch subject: [PATCH v2] stackdepot: Make max number of pools build-time configurable > config: arm64-randconfig-001-20250705 (https://download.01.org/0day-ci/archive/20250705/202507051300.E0JSHxu1-lkp@intel.com/config) > compiler: aarch64-linux-gcc (GCC) 10.5.0 > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250705/202507051300.E0JSHxu1-lkp@intel.com/reproduce) > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > the same patch/commit), kindly add following tags > | Reported-by: kernel test robot <lkp@intel.com> > | Closes: https://lore.kernel.org/oe-kbuild-all/202507051300.E0JSHxu1-lkp@intel.com/ > > All errors (new ones prefixed by >>): > > In file included from include/linux/init.h:5, > from include/linux/printk.h:6, > from include/asm-generic/bug.h:22, > from arch/arm64/include/asm/bug.h:26, > from include/linux/bug.h:5, > from include/linux/vfsdebug.h:5, > from include/linux/fs.h:5, > from include/linux/debugfs.h:15, > from lib/stackdepot.c:17: > >> include/linux/build_bug.h:78:41: error: static assertion failed: "DEPOT_MAX_POOLS <= (1LL << (DEPOT_POOL_INDEX_BITS)) - 1" > 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) > | ^~~~~~~~~~~~~~ > include/linux/build_bug.h:77:34: note: in expansion of macro '__static_assert' > 77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr) > | ^~~~~~~~~~~~~~~ > lib/stackdepot.c:42:1: note: in expansion of macro 'static_assert' > 42 | static_assert(DEPOT_MAX_POOLS <= (1LL << (DEPOT_POOL_INDEX_BITS)) - 1); > | ^~~~~~~~~~~~~ This is odd. The randconfig here uses the default: > CONFIG_STACKDEPOT_MAX_POOLS=8192 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] stackdepot: Make max number of pools build-time configurable 2025-07-07 6:39 ` Marco Elver @ 2025-07-07 12:05 ` Matt Fleming 0 siblings, 0 replies; 7+ messages in thread From: Matt Fleming @ 2025-07-07 12:05 UTC (permalink / raw) To: Marco Elver Cc: kernel test robot, Andrew Morton, oe-kbuild-all, Linux Memory Management List, linux-kernel, kernel-team, Alexander Potapenko, Andrey Konovalov, Dmitry Vyukov, Oscar Salvador, Vlastimil Babka, Matt Fleming On Mon, Jul 7, 2025 at 7:40 AM Marco Elver <elver@google.com> wrote: > > On Sat, 5 Jul 2025 at 08:01, kernel test robot <lkp@intel.com> wrote: > > > > Hi Matt, > > > > kernel test robot noticed the following build errors: > > > > [auto build test ERROR on akpm-mm/mm-nonmm-unstable] > > [also build test ERROR on linus/master v6.16-rc4 next-20250704] > > [cannot apply to akpm-mm/mm-everything] > > [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#_base_tree_information] > > > > url: https://github.com/intel-lab-lkp/linux/commits/Matt-Fleming/stackdepot-Make-max-number-of-pools-build-time-configurable/20250704-200804 > > base: https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-nonmm-unstable > > patch link: https://lore.kernel.org/r/20250704120604.2688934-1-matt%40readmodwrite.com > > patch subject: [PATCH v2] stackdepot: Make max number of pools build-time configurable > > config: arm64-randconfig-001-20250705 (https://download.01.org/0day-ci/archive/20250705/202507051300.E0JSHxu1-lkp@intel.com/config) > > compiler: aarch64-linux-gcc (GCC) 10.5.0 > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250705/202507051300.E0JSHxu1-lkp@intel.com/reproduce) > > > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > > the same patch/commit), kindly add following tags > > | Reported-by: kernel test robot <lkp@intel.com> > > | Closes: https://lore.kernel.org/oe-kbuild-all/202507051300.E0JSHxu1-lkp@intel.com/ > > > > All errors (new ones prefixed by >>): > > > > In file included from include/linux/init.h:5, > > from include/linux/printk.h:6, > > from include/asm-generic/bug.h:22, > > from arch/arm64/include/asm/bug.h:26, > > from include/linux/bug.h:5, > > from include/linux/vfsdebug.h:5, > > from include/linux/fs.h:5, > > from include/linux/debugfs.h:15, > > from lib/stackdepot.c:17: > > >> include/linux/build_bug.h:78:41: error: static assertion failed: "DEPOT_MAX_POOLS <= (1LL << (DEPOT_POOL_INDEX_BITS)) - 1" > > 78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg) > > | ^~~~~~~~~~~~~~ > > include/linux/build_bug.h:77:34: note: in expansion of macro '__static_assert' > > 77 | #define static_assert(expr, ...) __static_assert(expr, ##__VA_ARGS__, #expr) > > | ^~~~~~~~~~~~~~~ > > lib/stackdepot.c:42:1: note: in expansion of macro 'static_assert' > > 42 | static_assert(DEPOT_MAX_POOLS <= (1LL << (DEPOT_POOL_INDEX_BITS)) - 1); > > | ^~~~~~~~~~~~~ > > This is odd. The randconfig here uses the default: > > > CONFIG_STACKDEPOT_MAX_POOLS=8192 Ugh, I see what's happened here. For this config, the expression evaluates to static_assert(8192 <= 8191); So the default needs to be 8191. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-07-07 12:06 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-07-04 12:06 [PATCH v2] stackdepot: Make max number of pools build-time configurable Matt Fleming 2025-07-04 20:35 ` Andrew Morton 2025-07-07 8:12 ` Marco Elver 2025-07-07 12:06 ` Matt Fleming 2025-07-05 6:00 ` kernel test robot 2025-07-07 6:39 ` Marco Elver 2025-07-07 12:05 ` Matt Fleming
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).