Re: [PATCH 14/15] stackdepot: allow users to evict stack traces

[Marco Elver]

On Tue, Aug 29, 2023 at 07:11PM +0200, andrey.konovalov@xxxxxxxxx wrote:
> From: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>
> ---
>  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
>