[PATCH] stackdepot: Make max number of pools boot-time configurable

[PATCH] stackdepot: Make max number of pools boot-time configurable

[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 start-up parameter to allow users to set the number of
maximum stack depot pools.

Signed-off-by: Matt Fleming <mfleming@cloudflare.com>
---
 .../admin-guide/kernel-parameters.txt         |  5 ++
 lib/stackdepot.c                              | 55 +++++++++++++++++--
 2 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index b5cb36148554..6a6d60de4530 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6542,6 +6542,11 @@
 			consumed by the stack hash table. By default this is set
 			to false.
 
+	stack_depot_max_pools= [KNL,EARLY]
+			Specify the maximum number of pools to use for storing
+			stack traces. Pools are allocated on-demand up to this
+			limit. Default value is 8191 pools.
+
 	stacktrace	[FTRACE]
 			Enabled the stack tracer on boot up.
 
diff --git a/lib/stackdepot.c b/lib/stackdepot.c
index 245d5b416699..f66b1d572efb 100644
--- a/lib/stackdepot.c
+++ b/lib/stackdepot.c
@@ -42,6 +42,8 @@
 	(((1LL << (DEPOT_POOL_INDEX_BITS)) - 1 < DEPOT_POOLS_CAP) ? \
 	 (1LL << (DEPOT_POOL_INDEX_BITS)) - 1 : DEPOT_POOLS_CAP)
 
+static unsigned int stack_max_pools = DEPOT_MAX_POOLS;
+
 static bool stack_depot_disabled;
 static bool __stack_depot_early_init_requested __initdata = IS_ENABLED(CONFIG_STACKDEPOT_ALWAYS_INIT);
 static bool __stack_depot_early_init_passed __initdata;
@@ -62,7 +64,7 @@ static unsigned int stack_bucket_number_order;
 static unsigned int stack_hash_mask;
 
 /* Array of memory regions that store stack records. */
-static void *stack_pools[DEPOT_MAX_POOLS];
+static void **stack_pools;
 /* Newly allocated pool that is not yet added to stack_pools. */
 static void *new_pool;
 /* Number of pools in stack_pools. */
@@ -101,6 +103,33 @@ static int __init disable_stack_depot(char *str)
 }
 early_param("stack_depot_disable", disable_stack_depot);
 
+static int __init parse_max_pools(char *str)
+{
+	const long long limit = (1LL << (DEPOT_POOL_INDEX_BITS)) - 1;
+	unsigned int max_pools;
+	int rv;
+
+	rv = kstrtouint(str, 0, &max_pools);
+	if (rv)
+		return rv;
+
+	if (max_pools < 1024) {
+		pr_err("stack_depot_max_pools too low, using default\n");
+		goto out;
+	}
+
+	if (max_pools > limit) {
+		pr_err("stack_depot_max_pools too high, using default\n");
+		goto out;
+
+	}
+
+	stack_max_pools = max_pools;
+out:
+	return 0;
+}
+early_param("stack_depot_max_pools", parse_max_pools);
+
 void __init stack_depot_request_early_init(void)
 {
 	/* Too late to request early init now. */
@@ -182,6 +211,15 @@ int __init stack_depot_early_init(void)
 	}
 	init_stack_table(entries);
 
+	pr_info("allocating space for %u stack pools via memblock\n", stack_max_pools);
+	stack_pools = memblock_alloc(stack_max_pools * sizeof(void *), PAGE_SIZE);
+	if (!stack_pools) {
+		pr_err("stack pools allocation failed, disabling\n");
+		memblock_free(stack_table, entries * sizeof(struct list_head));
+		stack_depot_disabled = true;
+		return -ENOMEM;
+	}
+
 	return 0;
 }
 
@@ -231,6 +269,15 @@ int stack_depot_init(void)
 	stack_hash_mask = entries - 1;
 	init_stack_table(entries);
 
+	pr_info("allocating space for %u stack pools via kvcalloc\n", stack_max_pools);
+	stack_pools = kvcalloc(stack_max_pools, sizeof(void *), GFP_KERNEL);
+	if (!stack_pools) {
+		pr_err("stack pools allocation failed, disabling\n");
+		kvfree(stack_table);
+		stack_depot_disabled = true;
+		ret = -ENOMEM;
+	}
+
 out_unlock:
 	mutex_unlock(&stack_depot_init_mutex);
 
@@ -245,9 +292,9 @@ static bool depot_init_pool(void **prealloc)
 {
 	lockdep_assert_held(&pool_lock);
 
-	if (unlikely(pools_num >= DEPOT_MAX_POOLS)) {
+	if (unlikely(pools_num >= stack_max_pools)) {
 		/* Bail out if we reached the pool limit. */
-		WARN_ON_ONCE(pools_num > DEPOT_MAX_POOLS); /* should never happen */
+		WARN_ON_ONCE(pools_num > stack_max_pools); /* should never happen */
 		WARN_ON_ONCE(!new_pool); /* to avoid unnecessary pre-allocation */
 		WARN_ONCE(1, "Stack depot reached limit capacity");
 		return false;
@@ -273,7 +320,7 @@ static bool depot_init_pool(void **prealloc)
 	 * NULL; do not reset to NULL if we have reached the maximum number of
 	 * pools.
 	 */
-	if (pools_num < DEPOT_MAX_POOLS)
+	if (pools_num < stack_max_pools)
 		WRITE_ONCE(new_pool, NULL);
 	else
 		WRITE_ONCE(new_pool, STACK_DEPOT_POISON);
-- 
2.34.1

Re: [PATCH] stackdepot: Make max number of pools boot-time configurable

[Matt Fleming]

On Mon, Jul 14, 2025 at 3:33 PM 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 start-up parameter to allow users to set the number of
> maximum stack depot pools.
>
> Signed-off-by: Matt Fleming <mfleming@cloudflare.com>
> ---

Sorry, this is technically a v3 patch (even if it's very different to
v1 and v2) so I should've included a list of changes:

Changes in v3:
 - Switch from build-time to boot-time parameter

Changes in v2:
 - Replace BUILD_BUG_ON with static_assert()
 - Hide STACKDEPOT_MAX_POOLS behind EXPERT

>  .../admin-guide/kernel-parameters.txt         |  5 ++
>  lib/stackdepot.c                              | 55 +++++++++++++++++--
>  2 files changed, 56 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index b5cb36148554..6a6d60de4530 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -6542,6 +6542,11 @@
>                         consumed by the stack hash table. By default this is set
>                         to false.
>
> +       stack_depot_max_pools= [KNL,EARLY]
> +                       Specify the maximum number of pools to use for storing
> +                       stack traces. Pools are allocated on-demand up to this
> +                       limit. Default value is 8191 pools.
> +

The default value is usually 8192 pools unless you're on arm64 with
64K pages in which case it's 8191. If this discrepancy is too
confusing I'm fine just deleting this final sentence from the help
text.

Re: [PATCH] stackdepot: Make max number of pools boot-time configurable

[Andrew Morton]

On Mon, 14 Jul 2025 15:33:32 +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 start-up parameter to allow users to set the number of
> maximum stack depot pools.
> 
> ...
>
> --- a/lib/stackdepot.c
> +++ b/lib/stackdepot.c
> @@ -42,6 +42,8 @@
>  	(((1LL << (DEPOT_POOL_INDEX_BITS)) - 1 < DEPOT_POOLS_CAP) ? \
>  	 (1LL << (DEPOT_POOL_INDEX_BITS)) - 1 : DEPOT_POOLS_CAP)
>  
> +static unsigned int stack_max_pools = DEPOT_MAX_POOLS;

Geeze that was all quite the mouthful.  Can't we just do this?

--- a/lib/stackdepot.c~a
+++ a/lib/stackdepot.c
@@ -36,13 +36,11 @@
 #include <linux/memblock.h>
 #include <linux/kasan-enabled.h>
 
-#define DEPOT_POOLS_CAP 8192
-/* 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 unsigned int stack_max_pools = DEPOT_MAX_POOLS;
+/*
+ * The pool_index is offset by 1 so the first record does not have a 0 handle.
+ */
+static unsigned int stack_max_pools __read_mostly =
+	MIN((1LL << DEPOT_POOL_INDEX_BITS) - 1, 8192);
 
 static bool stack_depot_disabled;
 static bool __stack_depot_early_init_requested __initdata = IS_ENABLED(CONFIG_STACKDEPOT_ALWAYS_INIT);
_

(please add this to the next version)

(but why do we do this min() at all?  Why not simply use (1<<DEPOT_POOL_INDEX_BITS)?)

(shouldn't that 8192 be 8191?  Seems oddly inconsistent)

> @@ -101,6 +103,33 @@ static int __init disable_stack_depot(char *str)
>  }
>  early_param("stack_depot_disable", disable_stack_depot);
>  
> +static int __init parse_max_pools(char *str)
> +{
> +	const long long limit = (1LL << (DEPOT_POOL_INDEX_BITS)) - 1;

Expands to
limit = (1LL << (((sizeof(depot_stack_handle_t) * 8) - (2 + 12 - 4) - 5))) - 1;
which is 131071.

This seems a reasonable limit.  If we are to have any limit at all. 
Why do we have a limit?

> +	unsigned int max_pools;
> +	int rv;
> +
> +	rv = kstrtouint(str, 0, &max_pools);
> +	if (rv)
> +		return rv;
> +
> +	if (max_pools < 1024) {
> +		pr_err("stack_depot_max_pools too low, using default\n");
> +		goto out;
> +	}
> +
> +	if (max_pools > limit) {
> +		pr_err("stack_depot_max_pools too high, using default\n");

If user hits this they're going to tear hair figuring out the actual
limit.  So how about "stack_depot_max_pools exceeds %d, using default
of %d".

Re: [PATCH] stackdepot: Make max number of pools boot-time configurable

[Matt Fleming]

On Tue, Jul 15, 2025 at 12:38 AM Andrew Morton
<akpm@linux-foundation.org> wrote:
>
>
> Geeze that was all quite the mouthful.  Can't we just do this?
>
> --- a/lib/stackdepot.c~a
> +++ a/lib/stackdepot.c
> @@ -36,13 +36,11 @@
>  #include <linux/memblock.h>
>  #include <linux/kasan-enabled.h>
>
> -#define DEPOT_POOLS_CAP 8192
> -/* 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 unsigned int stack_max_pools = DEPOT_MAX_POOLS;
> +/*
> + * The pool_index is offset by 1 so the first record does not have a 0 handle.
> + */
> +static unsigned int stack_max_pools __read_mostly =
> +       MIN((1LL << DEPOT_POOL_INDEX_BITS) - 1, 8192);
>
>  static bool stack_depot_disabled;
>  static bool __stack_depot_early_init_requested __initdata = IS_ENABLED(CONFIG_STACKDEPOT_ALWAYS_INIT);
> _
>
> (please add this to the next version)

Nice. Will do.

> (but why do we do this min() at all?  Why not simply use (1<<DEPOT_POOL_INDEX_BITS)?)
>
> (shouldn't that 8192 be 8191?  Seems oddly inconsistent)

Yeah, I don't have a good answer here. I was hoping Marco or Alexander
would chime in.

> If user hits this they're going to tear hair figuring out the actual
> limit.  So how about "stack_depot_max_pools exceeds %d, using default
> of %d".

Good point. Will fix.