Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Marco Elver <elver@google.com>
To: Caleb Kan <calebkan1106@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	kasan-dev@googlegroups.com, Vlastimil Babka <vbabka@kernel.org>,
	Alexander Potapenko <glider@google.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	Andrey Konovalov <andreyknvl@gmail.com>,
	Oscar Salvador <osalvador@suse.de>,
	Caleb Kan <ckan@cloudflare.com>,
	kernel-team@cloudflare.com
Subject: Re: [PATCH RFC v2 02/11] stackdepot: add caller-owned stack trace fetching
Date: Tue, 20 Oct 2026 11:00:52 +0000	[thread overview]
Message-ID: <atdJ5AbOVXS9W7Gg@elver.google.com> (raw)
In-Reply-To: <20260908-stackdepot-trie-v2-2-1996d5cef732@cloudflare.com>

On Tue, Sep 08, 2026 at 02:13PM +0100, Caleb Kan wrote:
> From: Caleb Kan <ckan@cloudflare.com>
> 
> stack_depot_fetch() returns a pointer to contiguous storage owned by stack
> depot. That cannot work for a backend whose frames are not contiguous, so
> callers need an interface that copies the trace before they can support
> both backends.
> 
> Add stack_depot_fetch_into() to copy a complete trace into caller-owned
> storage. Leave the destination unchanged when it is too small, keep a zero
> handle as a no-op, and document that callers must keep the handle valid
> while copying it. Warn if a caller passes a NULL buffer or zero capacity
> for a valid handle instead of treating the stack as missing.
> 
> Unpoison the copied entries before returning them because
> lib/stackdepot.c is not instrumented by KMSAN. Add built-in KUnit tests for
> exact and oversized destinations, zero handles, and undersized buffers.
> Later patches add tests for stack depot internals.
> 
> Signed-off-by: Caleb Kan <ckan@cloudflare.com>
> ---
>  include/linux/stackdepot.h   | 36 ++++++++++++++++++
>  lib/Kconfig.debug            | 16 ++++++++
>  lib/stackdepot.c             | 28 ++++++++++++++
>  lib/tests/Makefile           |  1 +
>  lib/tests/stackdepot_kunit.c | 89 ++++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 170 insertions(+)
> 
> diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
> index 2cc21ffcdaf9..734529767c8a 100644
> --- a/include/linux/stackdepot.h
> +++ b/include/linux/stackdepot.h
> @@ -199,6 +199,42 @@ struct stack_record *__stack_depot_get_stack_record(depot_stack_handle_t handle)
>  unsigned int stack_depot_fetch(depot_stack_handle_t handle,
>  			       unsigned long **entries);
>  
> +/**
> + * stack_depot_fetch_into - Fetch a stack trace into caller-owned storage
> + *
> + * @handle:	Stack depot handle
> + * @entries:	Caller-owned buffer to copy the stack trace into
> + * @max_entries:	Number of frames that fit in @entries
> + *
> + * Copies the stored frames into caller-owned @entries. If fewer frames are
> + * stored than @max_entries, only the stored frames are written and their count
> + * is returned. If more frames are stored than @max_entries, the copy is skipped
> + * entirely and 0 is returned.
> + *
> + * Passing a NULL @entries buffer or zero @max_entries for a valid @handle is
> + * invalid. Callers must provide storage for @max_entries frames.
> + *
> + * Callers should size @entries to match the save-side stack depth cap (for
> + * example, %CONFIG_STACKDEPOT_MAX_FRAMES or the local stack_trace_save() limit)
> + * when losing diagnostics on an undersized buffer would be surprising.
> + *
> + * A non-zero invalid @handle, including a post-put handle, may WARN. Its return
> + * value and copied contents are undefined because the record may have been
> + * reused for another stack.
> + *
> + * Callers must ensure @handle remains valid for the duration of this call.
> + * Persistent handles saved without %STACK_DEPOT_FLAG_GET require no extra
> + * reference; handles saved with %STACK_DEPOT_FLAG_GET require a held reference.
> + * Callers must not call stack_depot_put() on persistent handles.
> + * Racing this helper with stack_depot_put() on the same handle is invalid.
> + *
> + * Return: Number of frames copied, 0 if @handle is 0, stack depot is disabled,
> + * or @max_entries is less than the number of stored frames.
> + */
> +unsigned int stack_depot_fetch_into(depot_stack_handle_t handle,
> +				    unsigned long *entries,
> +				    unsigned int max_entries);
> +
>  /**
>   * stack_depot_print - Print a stack trace from stack depot
>   *
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index 134b15a44625..fcd74edfd93a 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -2785,6 +2785,22 @@ config RESOURCE_KUNIT_TEST
>  
>  	  If unsure, say N.
>  
> +config STACKDEPOT_KUNIT_TEST
> +	bool "KUnit test for stack depot" if !KUNIT_ALL_TESTS
> +	depends on KUNIT=y && STACKDEPOT
> +	depends on STACKDEPOT_MAX_FRAMES >= 3
> +	default KUNIT_ALL_TESTS
> +	help
> +	  Enable this option to test stack depot API behavior at boot.
> +	  This test is built in, so KUNIT must also be built in.
> +
> +	  KUnit tests run during boot and output the results to the debug log
> +	  in TAP format (https://testanything.org/). Only useful for kernel
> +	  developers running the KUnit test harness, and not intended for
> +	  inclusion into a production build.
> +
> +	  If unsure, say N.
> +
>  config SYSCTL_KUNIT_TEST
>  	tristate "KUnit test for sysctl" if !KUNIT_ALL_TESTS
>  	depends on KUNIT
> diff --git a/lib/stackdepot.c b/lib/stackdepot.c
> index 90c52f2e0d3f..4da7279d9f83 100644
> --- a/lib/stackdepot.c
> +++ b/lib/stackdepot.c
> @@ -785,6 +785,34 @@ unsigned int stack_depot_fetch(depot_stack_handle_t handle,
>  }
>  EXPORT_SYMBOL_GPL(stack_depot_fetch);
>  
> +unsigned int stack_depot_fetch_into(depot_stack_handle_t handle,
> +				    unsigned long *entries,
> +				    unsigned int max_entries)
> +{
> +	struct stack_record *stack;
> +	unsigned int nr_entries;
> +
> +	if (!handle)
> +		return 0;
> +	if (stack_depot_disabled)
> +		return 0;
> +	WARN_ON_ONCE(!entries || !max_entries);

If this triggers (e.g. entries == NULL with non-zero max_entries), it
falls through to memcpy(NULL, ...):

    if (WARN_ON_ONCE(!entries || !max_entries))
        return 0;


  reply	other threads:[~2026-10-20 11:01 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-08 13:13 [PATCH RFC v2 00/11] stackdepot: reduce memory use for persistent stack records with a trie Caleb Kan
2026-09-08 13:13 ` [PATCH RFC v2 01/11] stackdepot: stop preallocating after the final pool Caleb Kan
2026-10-16 16:53   ` Marco Elver
2026-09-08 13:13 ` [PATCH RFC v2 02/11] stackdepot: add caller-owned stack trace fetching Caleb Kan
2026-10-20 11:00   ` Marco Elver [this message]
2026-09-08 13:13 ` [PATCH RFC v2 03/11] mm/page_owner: preserve accounting with countable stack depot records Caleb Kan
2026-10-20 11:11   ` Marco Elver
2026-09-08 13:13 ` [PATCH RFC v2 04/11] mm/kmemleak: print trie-backed stack depot traces Caleb Kan
2026-09-08 13:13 ` [PATCH RFC v2 05/11] kmsan: report " Caleb Kan
2026-10-20 14:53   ` Marco Elver
2026-09-08 13:13 ` [PATCH RFC v2 06/11] mm/slub: materialize " Caleb Kan
2026-09-08 13:13 ` [PATCH RFC v2 07/11] drm/locking: preserve deadlock diagnostics for trie-backed stacks Caleb Kan
2026-09-08 13:13 ` [PATCH RFC v2 08/11] scripts/gdb: reject trie-backed stack depot handles Caleb Kan
2026-09-08 13:13 ` [PATCH RFC v2 09/11] stackdepot: add architecture hooks for compact frame storage Caleb Kan
2026-10-20 14:56   ` Marco Elver
2026-09-08 13:13 ` [PATCH RFC v2 10/11] stackdepot: share persistent stack prefixes with trie storage Caleb Kan
2026-10-20 15:04   ` Marco Elver
2026-09-08 13:13 ` [PATCH RFC v2 11/11] stackdepot: add KUnit tests for " Caleb Kan
2026-10-07 10:34 ` [PATCH RFC v2 00/11] stackdepot: reduce memory use for persistent stack records with a trie Matt Fleming
2026-10-20 15:07 ` Marco Elver

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=atdJ5AbOVXS9W7Gg@elver.google.com \
    --to=elver@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=andreyknvl@gmail.com \
    --cc=calebkan1106@gmail.com \
    --cc=ckan@cloudflare.com \
    --cc=dvyukov@google.com \
    --cc=glider@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=kernel-team@cloudflare.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=osalvador@suse.de \
    --cc=vbabka@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox