| Message ID | 658f5f34d4f94721844ad8ba41452d54b4f8ace5.1694625260.git.andreyknvl@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | stackdepot: allow evicting stack traces | expand |
On Wed, 13 Sept 2023 at 19:14, <andrey.konovalov@linux.dev> wrote: > > From: Andrey Konovalov <andreyknvl@google.com> > > Instead of storing stack records in stack depot pools one right after > another, use fixed-sized slots. > > Add a new Kconfig option STACKDEPOT_MAX_FRAMES that allows to select > the size of the slot in frames. Use 64 as the default value, which is > the maximum stack trace size both KASAN and KMSAN use right now. > > Also add descriptions for other stack depot Kconfig options. > > This is preparatory patch for implementing the eviction of stack records > from the stack depot. > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > > --- > > Changes v1->v2: > - Add and use STACKDEPOT_MAX_FRAMES Kconfig option. > --- > lib/Kconfig | 10 ++++++++-- > lib/stackdepot.c | 13 +++++++++---- > 2 files changed, 17 insertions(+), 6 deletions(-) > > diff --git a/lib/Kconfig b/lib/Kconfig > index c686f4adc124..7c32f424a6f3 100644 > --- a/lib/Kconfig > +++ b/lib/Kconfig > @@ -708,13 +708,19 @@ config ARCH_STACKWALK > bool > > config STACKDEPOT > - bool > + bool "Stack depot: stack trace storage that avoids duplication" > select STACKTRACE > > config STACKDEPOT_ALWAYS_INIT > - bool > + bool "Always initialize stack depot during early boot" > select STACKDEPOT This makes both STACKDEPOT and STACKDEPOT_ALWAYS_INIT configurable by users: https://www.kernel.org/doc/html/next/kbuild/kconfig-language.html#menu-attributes Usually the way to add documentation for non-user-configurable options is to add text in the "help" section of the config. I think the change here is not what was intended. > +config STACKDEPOT_MAX_FRAMES > + int "Maximum number of frames in trace saved in stack depot" > + range 1 256 > + default 64 > + depends on STACKDEPOT > + > config REF_TRACKER > bool > depends on STACKTRACE_SUPPORT > diff --git a/lib/stackdepot.c b/lib/stackdepot.c > index 9a004f15f59d..128ece21afe9 100644 > --- a/lib/stackdepot.c > +++ b/lib/stackdepot.c > @@ -58,9 +58,12 @@ struct stack_record { > u32 hash; /* Hash in the hash table */ > u32 size; /* Number of stored frames */ > union handle_parts handle; > - unsigned long entries[]; /* Variable-sized array of frames */ > + unsigned long entries[CONFIG_STACKDEPOT_MAX_FRAMES]; /* Frames */ > }; > > +#define DEPOT_STACK_RECORD_SIZE \ > + ALIGN(sizeof(struct stack_record), 1 << DEPOT_STACK_ALIGN) > + > 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; > @@ -258,9 +261,7 @@ static struct stack_record * > depot_alloc_stack(unsigned long *entries, int size, u32 hash, void **prealloc) > { > struct stack_record *stack; > - size_t required_size = struct_size(stack, entries, size); > - > - required_size = ALIGN(required_size, 1 << DEPOT_STACK_ALIGN); > + size_t required_size = DEPOT_STACK_RECORD_SIZE; > > /* Check if there is not enough space in the current pool. */ > if (unlikely(pool_offset + required_size > DEPOT_POOL_SIZE)) { > @@ -295,6 +296,10 @@ depot_alloc_stack(unsigned long *entries, int size, u32 hash, void **prealloc) > if (stack_pools[pool_index] == NULL) > return NULL; > > + /* Limit number of saved frames to CONFIG_STACKDEPOT_MAX_FRAMES. */ > + if (size > CONFIG_STACKDEPOT_MAX_FRAMES) > + size = CONFIG_STACKDEPOT_MAX_FRAMES; > + > /* Save the stack trace. */ > stack = stack_pools[pool_index] + pool_offset; > stack->hash = hash; > -- > 2.25.1 >
On Fri, Sep 15, 2023 at 10:56 AM Marco Elver <elver@google.com> wrote: > > --- a/lib/Kconfig > > +++ b/lib/Kconfig > > @@ -708,13 +708,19 @@ config ARCH_STACKWALK > > bool > > > > config STACKDEPOT > > - bool > > + bool "Stack depot: stack trace storage that avoids duplication" > > select STACKTRACE > > > > config STACKDEPOT_ALWAYS_INIT > > - bool > > + bool "Always initialize stack depot during early boot" > > select STACKDEPOT > > This makes both STACKDEPOT and STACKDEPOT_ALWAYS_INIT configurable by > users: https://www.kernel.org/doc/html/next/kbuild/kconfig-language.html#menu-attributes > > Usually the way to add documentation for non-user-configurable options > is to add text in the "help" section of the config. > > I think the change here is not what was intended. Ah, didn't know about that. Will fix in v3. Thanks!
diff --git a/lib/Kconfig b/lib/Kconfig index c686f4adc124..7c32f424a6f3 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -708,13 +708,19 @@ config ARCH_STACKWALK bool config STACKDEPOT - bool + bool "Stack depot: stack trace storage that avoids duplication" select STACKTRACE config STACKDEPOT_ALWAYS_INIT - bool + bool "Always initialize stack depot during early boot" select STACKDEPOT +config STACKDEPOT_MAX_FRAMES + int "Maximum number of frames in trace saved in stack depot" + range 1 256 + default 64 + depends on STACKDEPOT + config REF_TRACKER bool depends on STACKTRACE_SUPPORT diff --git a/lib/stackdepot.c b/lib/stackdepot.c index 9a004f15f59d..128ece21afe9 100644 --- a/lib/stackdepot.c +++ b/lib/stackdepot.c @@ -58,9 +58,12 @@ struct stack_record { u32 hash; /* Hash in the hash table */ u32 size; /* Number of stored frames */ union handle_parts handle; - unsigned long entries[]; /* Variable-sized array of frames */ + unsigned long entries[CONFIG_STACKDEPOT_MAX_FRAMES]; /* Frames */ }; +#define DEPOT_STACK_RECORD_SIZE \ + ALIGN(sizeof(struct stack_record), 1 << DEPOT_STACK_ALIGN) + 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; @@ -258,9 +261,7 @@ static struct stack_record * depot_alloc_stack(unsigned long *entries, int size, u32 hash, void **prealloc) { struct stack_record *stack; - size_t required_size = struct_size(stack, entries, size); - - required_size = ALIGN(required_size, 1 << DEPOT_STACK_ALIGN); + size_t required_size = DEPOT_STACK_RECORD_SIZE; /* Check if there is not enough space in the current pool. */ if (unlikely(pool_offset + required_size > DEPOT_POOL_SIZE)) { @@ -295,6 +296,10 @@ depot_alloc_stack(unsigned long *entries, int size, u32 hash, void **prealloc) if (stack_pools[pool_index] == NULL) return NULL; + /* Limit number of saved frames to CONFIG_STACKDEPOT_MAX_FRAMES. */ + if (size > CONFIG_STACKDEPOT_MAX_FRAMES) + size = CONFIG_STACKDEPOT_MAX_FRAMES; + /* Save the stack trace. */ stack = stack_pools[pool_index] + pool_offset; stack->hash = hash;