On Fri, 22 Nov 2019 at 12:26, <glider@xxxxxxxxxx> wrote:
[...]
> diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
> index 24d49c732341..ac1b5a78d7f6 100644
> --- a/include/linux/stackdepot.h
> +++ b/include/linux/stackdepot.h
> @@ -12,6 +12,11 @@
> #define _LINUX_STACKDEPOT_H
>
> typedef u32 depot_stack_handle_t;
> +/*
> + * Number of bits in the handle that stack depot doesn't use. Users may store
> + * information in them.
> + */
> +#define STACK_DEPOT_EXTRA_BITS 5
>
> depot_stack_handle_t stack_depot_save(unsigned long *entries,
> unsigned int nr_entries, gfp_t gfp_flags);
> @@ -20,5 +25,8 @@ unsigned int stack_depot_fetch(depot_stack_handle_t handle,
> unsigned long **entries);
>
> unsigned int filter_irq_stacks(unsigned long *entries, unsigned int nr_entries);
> +depot_stack_handle_t set_dsh_extra_bits(depot_stack_handle_t handle,
> + unsigned int bits);
The function declaration mismatches its definition ('unsigned bits' vs 'u32').
> +unsigned int get_dsh_extra_bits(depot_stack_handle_t handle);
This also doesn't match the definition.
The abbreviation 'dsh' in these function names is not very readable.
Maybe just '{set,get}_stack_depot_extra' ?
> #endif
> diff --git a/lib/stackdepot.c b/lib/stackdepot.c
> index eb95197b8743..e2f000a9fad8 100644
> --- a/lib/stackdepot.c
> +++ b/lib/stackdepot.c
> @@ -40,8 +40,10 @@
> #define STACK_ALLOC_ALIGN 4
> #define STACK_ALLOC_OFFSET_BITS (STACK_ALLOC_ORDER + PAGE_SHIFT - \
> STACK_ALLOC_ALIGN)
> +
> #define STACK_ALLOC_INDEX_BITS (DEPOT_STACK_BITS - \
> - STACK_ALLOC_NULL_PROTECTION_BITS - STACK_ALLOC_OFFSET_BITS)
> + STACK_ALLOC_NULL_PROTECTION_BITS - \
> + STACK_ALLOC_OFFSET_BITS - STACK_DEPOT_EXTRA_BITS)
> #define STACK_ALLOC_SLABS_CAP 8192
> #define STACK_ALLOC_MAX_SLABS \
> (((1LL << (STACK_ALLOC_INDEX_BITS)) < STACK_ALLOC_SLABS_CAP) ? \
> @@ -54,6 +56,7 @@ union handle_parts {
> u32 slabindex : STACK_ALLOC_INDEX_BITS;
> u32 offset : STACK_ALLOC_OFFSET_BITS;
> u32 valid : STACK_ALLOC_NULL_PROTECTION_BITS;
> + u32 extra : STACK_DEPOT_EXTRA_BITS;
Would a BUILD_BUG_ON somewhere to assert that the total bits do not
exceed DEPOT_STACK_BITS make sense?
> };
> };
>
> @@ -72,6 +75,24 @@ static int next_slab_inited;
> static size_t depot_offset;
> static DEFINE_SPINLOCK(depot_lock);
>
> +depot_stack_handle_t set_dsh_extra_bits(depot_stack_handle_t handle,
> + u32 bits)
> +{
> + union handle_parts parts = { .handle = handle };
> +
> + parts.extra = bits & ((1U << STACK_DEPOT_EXTRA_BITS) - 1);
> + return parts.handle;
> +}
> +EXPORT_SYMBOL_GPL(set_dsh_extra_bits);
> +
> +u32 get_dsh_extra_bits(depot_stack_handle_t handle)
> +{
> + union handle_parts parts = { .handle = handle };
> +
> + return parts.extra;
> +}
> +EXPORT_SYMBOL_GPL(get_dsh_extra_bits);
> +
> static bool init_stack_slab(void **prealloc)
> {
> if (!*prealloc)
> @@ -132,6 +153,7 @@ static struct stack_record *depot_alloc_stack(unsigned long *entries, int size,
> stack->handle.slabindex = depot_index;
> stack->handle.offset = depot_offset >> STACK_ALLOC_ALIGN;
> stack->handle.valid = 1;
> + stack->handle.extra = 0;
> memcpy(stack->entries, entries, size * sizeof(unsigned long));
> depot_offset += required_size;
>
> --
> 2.24.0.432.g9d3f5f5b63-goog
>
