On Mon, Nov 20, 2023 at 06:47:15PM +0100, andrey.konovalov@xxxxxxxxx wrote:
> From: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
>
> Add stack_depot_put, a function that decrements the 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_put
> 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>
I yet have to review the final bits of this series, but I'd like to
comment on something below
> +void stack_depot_put(depot_stack_handle_t handle)
> +{
> + struct stack_record *stack;
> + 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)) {
> + /* Unlink stack from the hash table. */
> + list_del(&stack->list);
> +
> + /* Free stack. */
> + depot_free_stack(stack);
It would be great if stack_depot_put would also accept a boolean,
which would determine whether we want to erase the stack or not.
For the feature I'm working on page_ower [1], I need to keep track
of how many times we allocated/freed from a certain path, which may
expose a potential leak, and I was using the refcount to do that,
but I don't want the record to be erased, because this new
functionality won't be exclusive with the existing one.
e.g: you can check /sys/kernel/debug/page_owner AND
/sys/kernel/debug/page_owner_stacks
So, while the new functionaliy won't care if a record has been erased,
the old one will, so information will be lost.
[1] https://patchwork.kernel.org/project/linux-mm/cover/20231120084300.4368-1-osalvador@xxxxxxx/
--
Oscar Salvador
SUSE Labs
