Message ID | 10b188af-a03b-3acc-8a06-cf31a46f735a@huawei.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | arm64: stacktrace: Factor out some common code info on_stack() | expand |
On Thu, May 07, 2020 at 05:28:19PM +0800, Yunfeng Ye wrote: > diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h > index fdb913cc0bcb..b92bef2fb6cd 100644 > --- a/arch/arm64/include/asm/stacktrace.h > +++ b/arch/arm64/include/asm/stacktrace.h > @@ -69,27 +69,40 @@ extern void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk, > > DECLARE_PER_CPU(unsigned long *, irq_stack_ptr); > > -static inline bool on_irq_stack(unsigned long sp, > +static inline bool on_stack(unsigned long sp, unsigned long low, > + unsigned long high, enum stack_type type, > struct stack_info *info) > { > - unsigned long low = (unsigned long)raw_cpu_read(irq_stack_ptr); > - unsigned long high = low + IRQ_STACK_SIZE; > - > - if (!low) > - return false; > - > if (sp < low || sp >= high) > return false; > > if (info) { > info->low = low; > info->high = high; > - info->type = STACK_TYPE_IRQ; > + info->type = type; > } > - > return true; > } > > +static inline bool on_valid_stack(unsigned long sp, unsigned long low, > + unsigned long high, enum stack_type type, > + struct stack_info *info) > +{ > + if (!low) > + return false; > + > + return on_stack(sp, low, high, type, info); > +} Do we need this as distinct from on_stack()? Afaict, 'low' is never going to be NULL for the on_stack() callers, so I suggest just having on_stack() check 'low' and getting everybody to call that instead. Make sense? Will
On 2020/5/7 21:52, Will Deacon wrote: > On Thu, May 07, 2020 at 05:28:19PM +0800, Yunfeng Ye wrote: >> diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h >> index fdb913cc0bcb..b92bef2fb6cd 100644 >> --- a/arch/arm64/include/asm/stacktrace.h >> +++ b/arch/arm64/include/asm/stacktrace.h >> @@ -69,27 +69,40 @@ extern void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk, >> >> DECLARE_PER_CPU(unsigned long *, irq_stack_ptr); >> >> -static inline bool on_irq_stack(unsigned long sp, >> +static inline bool on_stack(unsigned long sp, unsigned long low, >> + unsigned long high, enum stack_type type, >> struct stack_info *info) >> { >> - unsigned long low = (unsigned long)raw_cpu_read(irq_stack_ptr); >> - unsigned long high = low + IRQ_STACK_SIZE; >> - >> - if (!low) >> - return false; >> - >> if (sp < low || sp >= high) >> return false; >> >> if (info) { >> info->low = low; >> info->high = high; >> - info->type = STACK_TYPE_IRQ; >> + info->type = type; >> } >> - >> return true; >> } >> >> +static inline bool on_valid_stack(unsigned long sp, unsigned long low, >> + unsigned long high, enum stack_type type, >> + struct stack_info *info) >> +{ >> + if (!low) >> + return false; >> + >> + return on_stack(sp, low, high, type, info); >> +} > > Do we need this as distinct from on_stack()? Afaict, 'low' is never > going to be NULL for the on_stack() callers, so I suggest just having > on_stack() check 'low' and getting everybody to call that instead. > > Make sense? > ok, I will modify and send the patch v2, thanks. > Will > > . >
diff --git a/arch/arm64/include/asm/stacktrace.h b/arch/arm64/include/asm/stacktrace.h index fdb913cc0bcb..b92bef2fb6cd 100644 --- a/arch/arm64/include/asm/stacktrace.h +++ b/arch/arm64/include/asm/stacktrace.h @@ -69,27 +69,40 @@ extern void dump_backtrace(struct pt_regs *regs, struct task_struct *tsk, DECLARE_PER_CPU(unsigned long *, irq_stack_ptr); -static inline bool on_irq_stack(unsigned long sp, +static inline bool on_stack(unsigned long sp, unsigned long low, + unsigned long high, enum stack_type type, struct stack_info *info) { - unsigned long low = (unsigned long)raw_cpu_read(irq_stack_ptr); - unsigned long high = low + IRQ_STACK_SIZE; - - if (!low) - return false; - if (sp < low || sp >= high) return false; if (info) { info->low = low; info->high = high; - info->type = STACK_TYPE_IRQ; + info->type = type; } - return true; } +static inline bool on_valid_stack(unsigned long sp, unsigned long low, + unsigned long high, enum stack_type type, + struct stack_info *info) +{ + if (!low) + return false; + + return on_stack(sp, low, high, type, info); +} + +static inline bool on_irq_stack(unsigned long sp, + struct stack_info *info) +{ + unsigned long low = (unsigned long)raw_cpu_read(irq_stack_ptr); + unsigned long high = low + IRQ_STACK_SIZE; + + return on_valid_stack(sp, low, high, STACK_TYPE_IRQ, info); +} + static inline bool on_task_stack(const struct task_struct *tsk, unsigned long sp, struct stack_info *info) @@ -97,16 +110,7 @@ static inline bool on_task_stack(const struct task_struct *tsk, unsigned long low = (unsigned long)task_stack_page(tsk); unsigned long high = low + THREAD_SIZE; - if (sp < low || sp >= high) - return false; - - if (info) { - info->low = low; - info->high = high; - info->type = STACK_TYPE_TASK; - } - - return true; + return on_stack(sp, low, high, STACK_TYPE_TASK, info); } #ifdef CONFIG_VMAP_STACK @@ -118,16 +122,7 @@ static inline bool on_overflow_stack(unsigned long sp, unsigned long low = (unsigned long)raw_cpu_ptr(overflow_stack); unsigned long high = low + OVERFLOW_STACK_SIZE; - if (sp < low || sp >= high) - return false; - - if (info) { - info->low = low; - info->high = high; - info->type = STACK_TYPE_OVERFLOW; - } - - return true; + return on_stack(sp, low, high, STACK_TYPE_OVERFLOW, info); } #else static inline bool on_overflow_stack(unsigned long sp, diff --git a/arch/arm64/kernel/sdei.c b/arch/arm64/kernel/sdei.c index d6259dac62b6..1758fcaa45dd 100644 --- a/arch/arm64/kernel/sdei.c +++ b/arch/arm64/kernel/sdei.c @@ -95,19 +95,7 @@ static bool on_sdei_normal_stack(unsigned long sp, struct stack_info *info) unsigned long low = (unsigned long)raw_cpu_read(sdei_stack_normal_ptr); unsigned long high = low + SDEI_STACK_SIZE; - if (!low) - return false; - - if (sp < low || sp >= high) - return false; - - if (info) { - info->low = low; - info->high = high; - info->type = STACK_TYPE_SDEI_NORMAL; - } - - return true; + return on_valid_stack(sp, low, high, STACK_TYPE_SDEI_NORMAL, info); } static bool on_sdei_critical_stack(unsigned long sp, struct stack_info *info) @@ -115,19 +103,7 @@ static bool on_sdei_critical_stack(unsigned long sp, struct stack_info *info) unsigned long low = (unsigned long)raw_cpu_read(sdei_stack_critical_ptr); unsigned long high = low + SDEI_STACK_SIZE; - if (!low) - return false; - - if (sp < low || sp >= high) - return false; - - if (info) { - info->low = low; - info->high = high; - info->type = STACK_TYPE_SDEI_CRITICAL; - } - - return true; + return on_valid_stack(sp, low, high, STACK_TYPE_SDEI_CRITICAL, info); } bool _on_sdei_stack(unsigned long sp, struct stack_info *info)
There are some common codes for stack checking, so factors it out into the function on_stack() and on_valid_stack(). No functional change. Signed-off-by: Yunfeng Ye <yeyunfeng@huawei.com> --- arch/arm64/include/asm/stacktrace.h | 53 +++++++++++++++++-------------------- arch/arm64/kernel/sdei.c | 28 ++------------------ 2 files changed, 26 insertions(+), 55 deletions(-)