Bug 218313

Summary:	stackdepot: reduce memory usage for storing stack traces
Product:	Memory Management	Reporter:	Andrey Konovalov (andreyknvl)
Component:	Sanitizers	Assignee:	MM/Sanitizers virtual assignee (mm_sanitizers)
Status:	RESOLVED CODE_FIX
Severity:	normal	CC:	dvyukov, kasan-dev, melver
Priority:	P3
Hardware:	All
OS:	Linux
Kernel Version:		Subsystem:
Regression:	No	Bisected commit-id:

Description Andrey Konovalov 2023-12-25 16:02:31 UTC

With the "stackdepot: allow evicting stack traces" series (will likely be merged into 6.8), the stack depot starts using fixed-sized slots for storing stack traces. As the result, some memory gets wasted.

This is partially mitigated by using the stack_depot_put API (used by KASAN), which removes unneeded stack traces from the stack depot. However, not all users can use this API (e.g. KMSAN cannot).

We can improve the memory usage by either:

- Introducing size classes for storing stack traces. I.e. store 4/8/16/32/...-sized traces in separate pages.

or

- Allowing each user to dynamically create a stack depot instance with the slot size of their choice. E.g. KMSAN can create a separate instance for storing its 3-frame linking records.

Comment 1 Dmitry Vyukov 2024-01-22 06:27:41 UTC

What are the intended use cases for eviction?
It's not needed for testing/fuzzing scenarios (worked fine before eviction), and the locking/refcounting overhead may be too high for any production uses. At the same time eviction is not guaranteed to bound memory consumption (if lots of stacks are used, they will all stay in memory).
It looks like use cases are pretty narrow, but we pay a high price to support eviction both in terms of memory and speed.
I think eviction should be optional and shouldn't have overhead if not enabled at build time.
If we don't have eviction, we don't need size classes, we can simply allocate exact amount of frames as before. We can also remove refcount_t from stack_record, define STACK_DEPOT_FLAG_GET to 0 to remove branches, etc.

Comment 2 Andrey Konovalov 2024-01-22 15:27:36 UTC

Eviction makes sure that we save fresh stack traces regardless of the uptime of the system. Without eviction, we stop saving them at some point, so we won't have stack traces for bugs that were triggered a while after boot.

Re memory consumption: by itself eviction does not limit the memory consumption indeed, so the idea was to add a command-line parameter on top that would limit the maximum number of pools.

Re overhead: the idea was to use sampling on top to limit the overhead (if required), see https://bugzilla.kernel.org/show_bug.cgi?id=211785.

With that said, I don't mind making eviction an optional feature.

Comment 3 Marco Elver 2024-01-22 17:19:05 UTC

We can keep the evictions feature, and just remove STACK_DEPOT_FLAG_GET/stack_depot_put() where it doesn't make much sense - and this should get previous memory usage again: https://lore.kernel.org/all/20240122171215.319440-2-elver@google.com/T/#u

Is it only KASAN generic where we should remove evictions again?

Comment 4 Dmitry Vyukov 2024-01-22 17:39:13 UTC

We also use SW TAGS during fuzzing on arm64. Probably more use-case- rather than tool-specific.

Comment 5 Marco Elver 2024-01-23 11:54:03 UTC

For generic mode I think it makes most sense to just revert evictions once we have variable-sized records to again save 4+ MiB: https://linux-review.googlesource.com/c/linux/kernel/git/torvalds/linux/+/25002

Comment 6 Andrey Konovalov 2024-04-10 09:05:46 UTC

Marco reintroduced compact stack records for users that don't rely on eviction in [1].

I think we can close this.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=31639fd6cebd4fc3687cceda14814f140c9fd95b