[PATCH] powerpc/irq: don't use current_stack_pointer() in do_IRQ()

[PATCH] powerpc/irq: don't use current_stack_pointer() in do_IRQ()

[Christophe Leroy]

Before commit 7306e83ccf5c ("powerpc: Don't use CURRENT_THREAD_INFO to
find the stack"), the current stack base address was obtained by
calling current_thread_info(). That inline function was simply masking
out the value of r1.

In that commit, it was changed to using current_stack_pointer(), which
is an heavier function as it is an outline assembly function which
cannot be inlined and which reads the content of the stack at 0(r1)

Revert to just getting r1 and masking out its value to obtain the base
address of the stack pointer as before.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Fixes: 7306e83ccf5c ("powerpc: Don't use CURRENT_THREAD_INFO to find the stack")
---
 arch/powerpc/kernel/irq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index 240eca12c71d..bb34005ff9d2 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -693,10 +693,11 @@ void __do_irq(struct pt_regs *regs)
 void do_IRQ(struct pt_regs *regs)
 {
 	struct pt_regs *old_regs = set_irq_regs(regs);
+	register unsigned long r1 asm("r1");
 	void *cursp, *irqsp, *sirqsp;
 
 	/* Switch to the irq stack to handle this */
-	cursp = (void *)(current_stack_pointer() & ~(THREAD_SIZE - 1));
+	cursp = (void *)(r1 & ~(THREAD_SIZE - 1));
 	irqsp = hardirq_ctx[raw_smp_processor_id()];
 	sirqsp = softirq_ctx[raw_smp_processor_id()];
 
-- 
2.13.3

Re: [PATCH] powerpc/irq: don't use current_stack_pointer() in do_IRQ()

[Christoph Hellwig]

Why can't current_stack_pointer be turned into an inline function using
inline assembly?  That would reduce the overhead for all callers.

Re: [PATCH] powerpc/irq: don't use current_stack_pointer() in do_IRQ()

[Christophe Leroy]

Le 12/12/2019 à 13:51, Christoph Hellwig a écrit :
> Why can't current_stack_pointer be turned into an inline function using
> inline assembly?  That would reduce the overhead for all callers.
> 

In the old days, it was a macro, and it was changed into an assembly 
function by commit 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bfe9a2cfe91a

It was later renamed from __get_SP() to current_stack_pointer() by 
commit 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=acf620ecf56cfc4edaffaf158250e128539cdd26

But in fact this function is badly named as it doesn't provide the 
current stack pointer but a pointer to the parent's stack frame.

Having it as an extern function forces GCC to set a stack frame in the 
calling function. If inline assembly is used instead, there's a risk of 
not getting a stack frame in the calling function, in which case the 
current_stack_pointer() will return the grandparent's stackframe pointer 
instead of the parent's one.

Christophe