| Message ID | 20200220115141.2707-1-mpe@ellerman.id.au (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 3d13e839e801e081bdece0127c2affa33d0f77cf |
| Headers | show |
| Series | [v3,1/5] powerpc: Rename current_stack_pointer() to current_stack_frame() | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (65b2623f395a4e25ab3ff4cff1c9c7623619a22d) |
| snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 0 warnings, 2 checks, 73 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
Le 20/02/2020 à 12:51, Michael Ellerman a écrit : > current_stack_pointer(), which was called __get_SP(), used to just > return the value in r1. > > But that caused problems in some cases, so it was turned into a > function in commit bfe9a2cfe91a ("powerpc: Reimplement __get_SP() as a > function not a define"). > > Because it's a function in a separate compilation unit to all its > callers, it has the effect of causing a stack frame to be created, and > then returns the address of that frame. This is good in some cases > like those described in the above commit, but in other cases it's > overkill, we just need to know what stack page we're on. > > On some other arches current_stack_pointer is just a register global > giving the stack pointer, and we'd like to do that too. So rename our > current_stack_pointer() to current_stack_frame() to make that > possible. > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> LGTM I was afraid to do that and risk invisible conflicts hence bugs by reusing the same name for different purpose, but that's the best solution for sure. Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr> > --- > arch/powerpc/include/asm/perf_event.h | 2 +- > arch/powerpc/include/asm/reg.h | 2 +- > arch/powerpc/kernel/irq.c | 4 ++-- > arch/powerpc/kernel/misc.S | 4 ++-- > arch/powerpc/kernel/process.c | 2 +- > arch/powerpc/kernel/stacktrace.c | 6 +++--- > 6 files changed, 10 insertions(+), 10 deletions(-) > > v3: New. > > diff --git a/arch/powerpc/include/asm/perf_event.h b/arch/powerpc/include/asm/perf_event.h > index 7426d7a90e1e..eed3954082fa 100644 > --- a/arch/powerpc/include/asm/perf_event.h > +++ b/arch/powerpc/include/asm/perf_event.h > @@ -32,7 +32,7 @@ > do { \ > (regs)->result = 0; \ > (regs)->nip = __ip; \ > - (regs)->gpr[1] = current_stack_pointer(); \ > + (regs)->gpr[1] = current_stack_frame(); \ > asm volatile("mfmsr %0" : "=r" ((regs)->msr)); \ > } while (0) > > diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h > index 1aa46dff0957..1b1ffdba6097 100644 > --- a/arch/powerpc/include/asm/reg.h > +++ b/arch/powerpc/include/asm/reg.h > @@ -1448,7 +1448,7 @@ static inline void mtsrin(u32 val, u32 idx) > > #define proc_trap() asm volatile("trap") > > -extern unsigned long current_stack_pointer(void); > +extern unsigned long current_stack_frame(void); > > extern unsigned long scom970_read(unsigned int address); > extern void scom970_write(unsigned int address, unsigned long value); > diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c > index 5c9b11878555..02118c18434d 100644 > --- a/arch/powerpc/kernel/irq.c > +++ b/arch/powerpc/kernel/irq.c > @@ -602,7 +602,7 @@ static inline void check_stack_overflow(void) > #ifdef CONFIG_DEBUG_STACKOVERFLOW > long sp; > > - sp = current_stack_pointer() & (THREAD_SIZE-1); > + sp = current_stack_frame() & (THREAD_SIZE-1); > > /* check for stack overflow: is there less than 2KB free? */ > if (unlikely(sp < 2048)) { > @@ -647,7 +647,7 @@ void do_IRQ(struct pt_regs *regs) > void *cursp, *irqsp, *sirqsp; > > /* Switch to the irq stack to handle this */ > - cursp = (void *)(current_stack_pointer() & ~(THREAD_SIZE - 1)); > + cursp = (void *)(current_stack_frame() & ~(THREAD_SIZE - 1)); > irqsp = hardirq_ctx[raw_smp_processor_id()]; > sirqsp = softirq_ctx[raw_smp_processor_id()]; > > diff --git a/arch/powerpc/kernel/misc.S b/arch/powerpc/kernel/misc.S > index 974f65f79a8e..65f9f731c229 100644 > --- a/arch/powerpc/kernel/misc.S > +++ b/arch/powerpc/kernel/misc.S > @@ -110,7 +110,7 @@ _GLOBAL(longjmp) > li r3, 1 > blr > > -_GLOBAL(current_stack_pointer) > +_GLOBAL(current_stack_frame) > PPC_LL r3,0(r1) > blr > -EXPORT_SYMBOL(current_stack_pointer) > +EXPORT_SYMBOL(current_stack_frame) > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c > index e730b8e522b0..110db94cdf3c 100644 > --- a/arch/powerpc/kernel/process.c > +++ b/arch/powerpc/kernel/process.c > @@ -2051,7 +2051,7 @@ void show_stack(struct task_struct *tsk, unsigned long *stack) > sp = (unsigned long) stack; > if (sp == 0) { > if (tsk == current) > - sp = current_stack_pointer(); > + sp = current_stack_frame(); > else > sp = tsk->thread.ksp; > } > diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c > index e2a46cfed5fd..c477b8585a29 100644 > --- a/arch/powerpc/kernel/stacktrace.c > +++ b/arch/powerpc/kernel/stacktrace.c > @@ -57,7 +57,7 @@ void save_stack_trace(struct stack_trace *trace) > { > unsigned long sp; > > - sp = current_stack_pointer(); > + sp = current_stack_frame(); > > save_context_stack(trace, sp, current, 1); > } > @@ -71,7 +71,7 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace) > return; > > if (tsk == current) > - sp = current_stack_pointer(); > + sp = current_stack_frame(); > else > sp = tsk->thread.ksp; > > @@ -131,7 +131,7 @@ static int __save_stack_trace_tsk_reliable(struct task_struct *tsk, > } > > if (tsk == current) > - sp = current_stack_pointer(); > + sp = current_stack_frame(); > else > sp = tsk->thread.ksp; > >
On Thu, 2020-02-20 at 11:51:37 UTC, Michael Ellerman wrote: > current_stack_pointer(), which was called __get_SP(), used to just > return the value in r1. > > But that caused problems in some cases, so it was turned into a > function in commit bfe9a2cfe91a ("powerpc: Reimplement __get_SP() as a > function not a define"). > > Because it's a function in a separate compilation unit to all its > callers, it has the effect of causing a stack frame to be created, and > then returns the address of that frame. This is good in some cases > like those described in the above commit, but in other cases it's > overkill, we just need to know what stack page we're on. > > On some other arches current_stack_pointer is just a register global > giving the stack pointer, and we'd like to do that too. So rename our > current_stack_pointer() to current_stack_frame() to make that > possible. > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> Series applied to powerpc next. https://git.kernel.org/powerpc/c/3d13e839e801e081bdece0127c2affa33d0f77cf cheers
diff --git a/arch/powerpc/include/asm/perf_event.h b/arch/powerpc/include/asm/perf_event.h index 7426d7a90e1e..eed3954082fa 100644 --- a/arch/powerpc/include/asm/perf_event.h +++ b/arch/powerpc/include/asm/perf_event.h @@ -32,7 +32,7 @@ do { \ (regs)->result = 0; \ (regs)->nip = __ip; \ - (regs)->gpr[1] = current_stack_pointer(); \ + (regs)->gpr[1] = current_stack_frame(); \ asm volatile("mfmsr %0" : "=r" ((regs)->msr)); \ } while (0) diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h index 1aa46dff0957..1b1ffdba6097 100644 --- a/arch/powerpc/include/asm/reg.h +++ b/arch/powerpc/include/asm/reg.h @@ -1448,7 +1448,7 @@ static inline void mtsrin(u32 val, u32 idx) #define proc_trap() asm volatile("trap") -extern unsigned long current_stack_pointer(void); +extern unsigned long current_stack_frame(void); extern unsigned long scom970_read(unsigned int address); extern void scom970_write(unsigned int address, unsigned long value); diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c index 5c9b11878555..02118c18434d 100644 --- a/arch/powerpc/kernel/irq.c +++ b/arch/powerpc/kernel/irq.c @@ -602,7 +602,7 @@ static inline void check_stack_overflow(void) #ifdef CONFIG_DEBUG_STACKOVERFLOW long sp; - sp = current_stack_pointer() & (THREAD_SIZE-1); + sp = current_stack_frame() & (THREAD_SIZE-1); /* check for stack overflow: is there less than 2KB free? */ if (unlikely(sp < 2048)) { @@ -647,7 +647,7 @@ void do_IRQ(struct pt_regs *regs) void *cursp, *irqsp, *sirqsp; /* Switch to the irq stack to handle this */ - cursp = (void *)(current_stack_pointer() & ~(THREAD_SIZE - 1)); + cursp = (void *)(current_stack_frame() & ~(THREAD_SIZE - 1)); irqsp = hardirq_ctx[raw_smp_processor_id()]; sirqsp = softirq_ctx[raw_smp_processor_id()]; diff --git a/arch/powerpc/kernel/misc.S b/arch/powerpc/kernel/misc.S index 974f65f79a8e..65f9f731c229 100644 --- a/arch/powerpc/kernel/misc.S +++ b/arch/powerpc/kernel/misc.S @@ -110,7 +110,7 @@ _GLOBAL(longjmp) li r3, 1 blr -_GLOBAL(current_stack_pointer) +_GLOBAL(current_stack_frame) PPC_LL r3,0(r1) blr -EXPORT_SYMBOL(current_stack_pointer) +EXPORT_SYMBOL(current_stack_frame) diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c index e730b8e522b0..110db94cdf3c 100644 --- a/arch/powerpc/kernel/process.c +++ b/arch/powerpc/kernel/process.c @@ -2051,7 +2051,7 @@ void show_stack(struct task_struct *tsk, unsigned long *stack) sp = (unsigned long) stack; if (sp == 0) { if (tsk == current) - sp = current_stack_pointer(); + sp = current_stack_frame(); else sp = tsk->thread.ksp; } diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c index e2a46cfed5fd..c477b8585a29 100644 --- a/arch/powerpc/kernel/stacktrace.c +++ b/arch/powerpc/kernel/stacktrace.c @@ -57,7 +57,7 @@ void save_stack_trace(struct stack_trace *trace) { unsigned long sp; - sp = current_stack_pointer(); + sp = current_stack_frame(); save_context_stack(trace, sp, current, 1); } @@ -71,7 +71,7 @@ void save_stack_trace_tsk(struct task_struct *tsk, struct stack_trace *trace) return; if (tsk == current) - sp = current_stack_pointer(); + sp = current_stack_frame(); else sp = tsk->thread.ksp; @@ -131,7 +131,7 @@ static int __save_stack_trace_tsk_reliable(struct task_struct *tsk, } if (tsk == current) - sp = current_stack_pointer(); + sp = current_stack_frame(); else sp = tsk->thread.ksp;
current_stack_pointer(), which was called __get_SP(), used to just return the value in r1. But that caused problems in some cases, so it was turned into a function in commit bfe9a2cfe91a ("powerpc: Reimplement __get_SP() as a function not a define"). Because it's a function in a separate compilation unit to all its callers, it has the effect of causing a stack frame to be created, and then returns the address of that frame. This is good in some cases like those described in the above commit, but in other cases it's overkill, we just need to know what stack page we're on. On some other arches current_stack_pointer is just a register global giving the stack pointer, and we'd like to do that too. So rename our current_stack_pointer() to current_stack_frame() to make that possible. Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> --- arch/powerpc/include/asm/perf_event.h | 2 +- arch/powerpc/include/asm/reg.h | 2 +- arch/powerpc/kernel/irq.c | 4 ++-- arch/powerpc/kernel/misc.S | 4 ++-- arch/powerpc/kernel/process.c | 2 +- arch/powerpc/kernel/stacktrace.c | 6 +++--- 6 files changed, 10 insertions(+), 10 deletions(-) v3: New.