| Message ID | 99cd7ac4a312e86c768b933332364272b9e3fb40.1693328501.git.andreyknvl@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | stackdepot: allow evicting stack traces | expand |
On Tue, Aug 29, 2023 at 07:11PM +0200, andrey.konovalov@linux.dev wrote: > From: Andrey Konovalov <andreyknvl@google.com> > > Add stack_depot_evict, a function that decrements a reference counter > on a stack record and removes it from the stack depot once the counter > reaches 0. > > Internally, when removing a stack record, the function unlinks it from > the hash table bucket and returns to the freelist. > > With this change, the users of stack depot can call stack_depot_evict > when keeping a stack trace in the stack depot is not needed anymore. > This allows avoiding polluting the stack depot with irrelevant stack > traces and thus have more space to store the relevant ones before the > stack depot reaches its capacity. > > Signed-off-by: Andrey Konovalov <andreyknvl@google.com> > --- > include/linux/stackdepot.h | 11 ++++++++++ > lib/stackdepot.c | 43 ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 54 insertions(+) > > diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h > index e58306783d8e..b14da6797714 100644 > --- a/include/linux/stackdepot.h > +++ b/include/linux/stackdepot.h > @@ -121,6 +121,17 @@ depot_stack_handle_t stack_depot_save(unsigned long *entries, > unsigned int stack_depot_fetch(depot_stack_handle_t handle, > unsigned long **entries); > > +/** > + * stack_depot_evict - Drop a reference to a stack trace from stack depot > + * > + * @handle: Stack depot handle returned from stack_depot_save() > + * > + * The stack trace gets fully removed from stack depot once all references "gets fully removed" -> "is evicted" ? > + * to it has been dropped (once the number of stack_depot_evict calls matches "has been" -> "have been" > + * the number of stack_depot_save calls for this stack trace). > + */ > +void stack_depot_evict(depot_stack_handle_t handle); > + > /** > * stack_depot_print - Print a stack trace from stack depot > * > diff --git a/lib/stackdepot.c b/lib/stackdepot.c > index 641db97d8c7c..cf28720b842d 100644 > --- a/lib/stackdepot.c > +++ b/lib/stackdepot.c > @@ -384,6 +384,13 @@ static struct stack_record *depot_fetch_stack(depot_stack_handle_t handle) > return stack; > } > > +/* Frees stack into the freelist. */ > +static void depot_free_stack(struct stack_record *stack) > +{ > + stack->next = next_stack; > + next_stack = stack; > +} > + > /* Calculates the hash for a stack. */ > static inline u32 hash_stack(unsigned long *entries, unsigned int size) > { > @@ -555,6 +562,42 @@ unsigned int stack_depot_fetch(depot_stack_handle_t handle, > } > EXPORT_SYMBOL_GPL(stack_depot_fetch); > > +void stack_depot_evict(depot_stack_handle_t handle) > +{ > + struct stack_record *stack, **bucket; > + unsigned long flags; > + > + if (!handle || stack_depot_disabled) > + return; > + > + write_lock_irqsave(&pool_rwlock, flags); > + > + stack = depot_fetch_stack(handle); > + if (WARN_ON(!stack)) > + goto out; > + > + if (refcount_dec_and_test(&stack->count)) { > + /* Drop stack from the hash table. */ > + if (stack->next) > + stack->next->prev = stack->prev; > + if (stack->prev) > + stack->prev->next = stack->next; > + else { > + bucket = &stack_table[stack->hash & stack_hash_mask]; > + *bucket = stack->next; > + } > + stack->next = NULL; > + stack->prev = NULL; > + > + /* Free stack. */ > + depot_free_stack(stack); > + } > + > +out: > + write_unlock_irqrestore(&pool_rwlock, flags); > +} > +EXPORT_SYMBOL_GPL(stack_depot_evict); > + > void stack_depot_print(depot_stack_handle_t stack) > { > unsigned long *entries; > -- > 2.25.1 >
On Wed, Aug 30, 2023 at 11:20 AM Marco Elver <elver@google.com> wrote: > > > +/** > > + * stack_depot_evict - Drop a reference to a stack trace from stack depot > > + * > > + * @handle: Stack depot handle returned from stack_depot_save() > > + * > > + * The stack trace gets fully removed from stack depot once all references > > "gets fully removed" -> "is evicted" ? > > > + * to it has been dropped (once the number of stack_depot_evict calls matches > > "has been" -> "have been" Will fix both in v2. Thanks!
diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h index e58306783d8e..b14da6797714 100644 --- a/include/linux/stackdepot.h +++ b/include/linux/stackdepot.h @@ -121,6 +121,17 @@ depot_stack_handle_t stack_depot_save(unsigned long *entries, unsigned int stack_depot_fetch(depot_stack_handle_t handle, unsigned long **entries); +/** + * stack_depot_evict - Drop a reference to a stack trace from stack depot + * + * @handle: Stack depot handle returned from stack_depot_save() + * + * The stack trace gets fully removed from stack depot once all references + * to it has been dropped (once the number of stack_depot_evict calls matches + * the number of stack_depot_save calls for this stack trace). + */ +void stack_depot_evict(depot_stack_handle_t handle); + /** * stack_depot_print - Print a stack trace from stack depot * diff --git a/lib/stackdepot.c b/lib/stackdepot.c index 641db97d8c7c..cf28720b842d 100644 --- a/lib/stackdepot.c +++ b/lib/stackdepot.c @@ -384,6 +384,13 @@ static struct stack_record *depot_fetch_stack(depot_stack_handle_t handle) return stack; } +/* Frees stack into the freelist. */ +static void depot_free_stack(struct stack_record *stack) +{ + stack->next = next_stack; + next_stack = stack; +} + /* Calculates the hash for a stack. */ static inline u32 hash_stack(unsigned long *entries, unsigned int size) { @@ -555,6 +562,42 @@ unsigned int stack_depot_fetch(depot_stack_handle_t handle, } EXPORT_SYMBOL_GPL(stack_depot_fetch); +void stack_depot_evict(depot_stack_handle_t handle) +{ + struct stack_record *stack, **bucket; + unsigned long flags; + + if (!handle || stack_depot_disabled) + return; + + write_lock_irqsave(&pool_rwlock, flags); + + stack = depot_fetch_stack(handle); + if (WARN_ON(!stack)) + goto out; + + if (refcount_dec_and_test(&stack->count)) { + /* Drop stack from the hash table. */ + if (stack->next) + stack->next->prev = stack->prev; + if (stack->prev) + stack->prev->next = stack->next; + else { + bucket = &stack_table[stack->hash & stack_hash_mask]; + *bucket = stack->next; + } + stack->next = NULL; + stack->prev = NULL; + + /* Free stack. */ + depot_free_stack(stack); + } + +out: + write_unlock_irqrestore(&pool_rwlock, flags); +} +EXPORT_SYMBOL_GPL(stack_depot_evict); + void stack_depot_print(depot_stack_handle_t stack) { unsigned long *entries;