On Thu, Sep 23, 2021 at 12:48 PM Marco Elver <elver@xxxxxxxxxx> wrote:
>
> filter_irq_stacks() has little to do with the stackdepot implementation,
> except that it is usually used by users (such as KASAN) of stackdepot to
> reduce the stack trace.
>
> However, filter_irq_stacks() itself is not useful without a stack trace
> as obtained by stack_trace_save() and friends.
>
> Therefore, move filter_irq_stacks() to kernel/stacktrace.c, so that new
> users of filter_irq_stacks() do not have to start depending on
> STACKDEPOT only for filter_irq_stacks().
>
> Signed-off-by: Marco Elver <elver@xxxxxxxxxx>
> Acked-by: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
Acked-by: Alexander Potapenko <glider@xxxxxxxxxx>
> ---
> v3:
> * Rebase to -next due to conflicting stackdepot changes.
>
> v2:
> * New patch.
> ---
> include/linux/stackdepot.h | 2 --
> include/linux/stacktrace.h | 1 +
> kernel/stacktrace.c | 30 ++++++++++++++++++++++++++++++
> lib/stackdepot.c | 24 ------------------------
> 4 files changed, 31 insertions(+), 26 deletions(-)
>
> diff --git a/include/linux/stackdepot.h b/include/linux/stackdepot.h
> index ee03f11bb51a..c34b55a6e554 100644
> --- a/include/linux/stackdepot.h
> +++ b/include/linux/stackdepot.h
> @@ -30,8 +30,6 @@ int stack_depot_snprint(depot_stack_handle_t handle, char *buf, size_t size,
>
> void stack_depot_print(depot_stack_handle_t stack);
>
> -unsigned int filter_irq_stacks(unsigned long *entries, unsigned int nr_entries);
> -
> #ifdef CONFIG_STACKDEPOT
> int stack_depot_init(void);
> #else
> diff --git a/include/linux/stacktrace.h b/include/linux/stacktrace.h
> index 9edecb494e9e..bef158815e83 100644
> --- a/include/linux/stacktrace.h
> +++ b/include/linux/stacktrace.h
> @@ -21,6 +21,7 @@ unsigned int stack_trace_save_tsk(struct task_struct *task,
> unsigned int stack_trace_save_regs(struct pt_regs *regs, unsigned long *store,
> unsigned int size, unsigned int skipnr);
> unsigned int stack_trace_save_user(unsigned long *store, unsigned int size);
> +unsigned int filter_irq_stacks(unsigned long *entries, unsigned int nr_entries);
>
> /* Internal interfaces. Do not use in generic code */
> #ifdef CONFIG_ARCH_STACKWALK
> diff --git a/kernel/stacktrace.c b/kernel/stacktrace.c
> index 9f8117c7cfdd..9c625257023d 100644
> --- a/kernel/stacktrace.c
> +++ b/kernel/stacktrace.c
> @@ -13,6 +13,7 @@
> #include <linux/export.h>
> #include <linux/kallsyms.h>
> #include <linux/stacktrace.h>
> +#include <linux/interrupt.h>
>
> /**
> * stack_trace_print - Print the entries in the stack trace
> @@ -373,3 +374,32 @@ unsigned int stack_trace_save_user(unsigned long *store, unsigned int size)
> #endif /* CONFIG_USER_STACKTRACE_SUPPORT */
>
> #endif /* !CONFIG_ARCH_STACKWALK */
> +
> +static inline bool in_irqentry_text(unsigned long ptr)
> +{
> + return (ptr >= (unsigned long)&__irqentry_text_start &&
> + ptr < (unsigned long)&__irqentry_text_end) ||
> + (ptr >= (unsigned long)&__softirqentry_text_start &&
> + ptr < (unsigned long)&__softirqentry_text_end);
> +}
> +
> +/**
> + * filter_irq_stacks - Find first IRQ stack entry in trace
> + * @entries: Pointer to stack trace array
> + * @nr_entries: Number of entries in the storage array
> + *
> + * Return: Number of trace entries until IRQ stack starts.
> + */
> +unsigned int filter_irq_stacks(unsigned long *entries, unsigned int nr_entries)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < nr_entries; i++) {
> + if (in_irqentry_text(entries[i])) {
> + /* Include the irqentry function into the stack. */
> + return i + 1;
> + }
> + }
> + return nr_entries;
> +}
> +EXPORT_SYMBOL_GPL(filter_irq_stacks);
> diff --git a/lib/stackdepot.c b/lib/stackdepot.c
> index 69c8c9b0d8d7..b437ae79aca1 100644
> --- a/lib/stackdepot.c
> +++ b/lib/stackdepot.c
> @@ -20,7 +20,6 @@
> */
>
> #include <linux/gfp.h>
> -#include <linux/interrupt.h>
> #include <linux/jhash.h>
> #include <linux/kernel.h>
> #include <linux/mm.h>
> @@ -417,26 +416,3 @@ depot_stack_handle_t stack_depot_save(unsigned long *entries,
> return __stack_depot_save(entries, nr_entries, alloc_flags, true);
> }
> EXPORT_SYMBOL_GPL(stack_depot_save);
> -
> -static inline int in_irqentry_text(unsigned long ptr)
> -{
> - return (ptr >= (unsigned long)&__irqentry_text_start &&
> - ptr < (unsigned long)&__irqentry_text_end) ||
> - (ptr >= (unsigned long)&__softirqentry_text_start &&
> - ptr < (unsigned long)&__softirqentry_text_end);
> -}
> -
> -unsigned int filter_irq_stacks(unsigned long *entries,
> - unsigned int nr_entries)
> -{
> - unsigned int i;
> -
> - for (i = 0; i < nr_entries; i++) {
> - if (in_irqentry_text(entries[i])) {
> - /* Include the irqentry function into the stack. */
> - return i + 1;
> - }
> - }
> - return nr_entries;
> -}
> -EXPORT_SYMBOL_GPL(filter_irq_stacks);
> --
> 2.33.0.464.g1972c5931b-goog
>
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
