| Message ID | 20231130125045.3080961-1-mpe@ellerman.id.au (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4eb20bf34ea296f648971a8528e32cd80efcbe89 |
| Headers | show |
| Series | powerpc/irq: Allow softirq to hardirq stack transition | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 23 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 6 jobs. |
Le 30/11/2023 à 13:50, Michael Ellerman a écrit : > Allow a transition from the softirq stack to the hardirq stack when > handling a hardirq. Doing so means a hardirq received while deep in > softirq processing is less likely to cause a stack overflow of the > softirq stack. > > Previously it wasn't safe to do so because irq_exit() (which initiates > softirq processing) was called on the hardirq stack. > > That was changed in commit 1b1b6a6f4cc0 ("powerpc: handle irq_enter/ > irq_exit in interrupt handler wrappers") and 1346d00e1bdf ("powerpc: > Don't select HAVE_IRQ_EXIT_ON_IRQ_STACK"). > > The allowed transitions are now: > - process stack -> hardirq stack > - process stack -> softirq stack > - process stack -> softirq stack -> hardirq stack It means you don't like my patch https://patchwork.ozlabs.org/project/linuxppc-dev/patch/6cd9d8bb2258d8b51999c2584eac74423d2b5e29.1657203774.git.christophe.leroy@csgroup.eu/ ? I never got any feedback. > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> > --- > arch/powerpc/kernel/irq.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c > index 6f7d4edaa0bc..7504ceec5c58 100644 > --- a/arch/powerpc/kernel/irq.c > +++ b/arch/powerpc/kernel/irq.c > @@ -284,15 +284,14 @@ static __always_inline void call_do_irq(struct pt_regs *regs, void *sp) > void __do_IRQ(struct pt_regs *regs) > { > struct pt_regs *old_regs = set_irq_regs(regs); > - void *cursp, *irqsp, *sirqsp; > + void *cursp, *irqsp; > > /* Switch to the irq stack to handle this */ > cursp = (void *)(current_stack_pointer & ~(THREAD_SIZE - 1)); > irqsp = hardirq_ctx[raw_smp_processor_id()]; > - sirqsp = softirq_ctx[raw_smp_processor_id()]; > > /* Already there ? If not switch stack and call */ > - if (unlikely(cursp == irqsp || cursp == sirqsp)) > + if (unlikely(cursp == irqsp)) > __do_irq(regs, current_stack_pointer); > else > call_do_irq(regs, irqsp);
Christophe Leroy <christophe.leroy@csgroup.eu> writes: > Le 30/11/2023 à 13:50, Michael Ellerman a écrit : >> Allow a transition from the softirq stack to the hardirq stack when >> handling a hardirq. Doing so means a hardirq received while deep in >> softirq processing is less likely to cause a stack overflow of the >> softirq stack. >> >> Previously it wasn't safe to do so because irq_exit() (which initiates >> softirq processing) was called on the hardirq stack. >> >> That was changed in commit 1b1b6a6f4cc0 ("powerpc: handle irq_enter/ >> irq_exit in interrupt handler wrappers") and 1346d00e1bdf ("powerpc: >> Don't select HAVE_IRQ_EXIT_ON_IRQ_STACK"). >> >> The allowed transitions are now: >> - process stack -> hardirq stack >> - process stack -> softirq stack >> - process stack -> softirq stack -> hardirq stack > > It means you don't like my patch > https://patchwork.ozlabs.org/project/linuxppc-dev/patch/6cd9d8bb2258d8b51999c2584eac74423d2b5e29.1657203774.git.christophe.leroy@csgroup.eu/ > ? I did like your patch :) But then we got reports of folks hitting stack overflow in some distro kernels, and in at least some cases it was a hardirq coming in during softirq handling and overflowing the softirq stack. > I never got any feedback. Sorry, not enough hours in the day. cheers
Le 01/12/2023 à 11:05, Michael Ellerman a écrit : > Christophe Leroy <christophe.leroy@csgroup.eu> writes: >> Le 30/11/2023 à 13:50, Michael Ellerman a écrit : >>> Allow a transition from the softirq stack to the hardirq stack when >>> handling a hardirq. Doing so means a hardirq received while deep in >>> softirq processing is less likely to cause a stack overflow of the >>> softirq stack. >>> >>> Previously it wasn't safe to do so because irq_exit() (which initiates >>> softirq processing) was called on the hardirq stack. >>> >>> That was changed in commit 1b1b6a6f4cc0 ("powerpc: handle irq_enter/ >>> irq_exit in interrupt handler wrappers") and 1346d00e1bdf ("powerpc: >>> Don't select HAVE_IRQ_EXIT_ON_IRQ_STACK"). >>> >>> The allowed transitions are now: >>> - process stack -> hardirq stack >>> - process stack -> softirq stack >>> - process stack -> softirq stack -> hardirq stack >> >> It means you don't like my patch >> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/6cd9d8bb2258d8b51999c2584eac74423d2b5e29.1657203774.git.christophe.leroy@csgroup.eu/ >> ? > > I did like your patch :) > > But then we got reports of folks hitting stack overflow in some distro > kernels, and in at least some cases it was a hardirq coming in during > softirq handling and overflowing the softirq stack. Fair enough, I'll discard it. > >> I never got any feedback. > > Sorry, not enough hours in the day. > Yes same problem here :)
Le 30/11/2023 à 13:50, Michael Ellerman a écrit : > Allow a transition from the softirq stack to the hardirq stack when > handling a hardirq. Doing so means a hardirq received while deep in > softirq processing is less likely to cause a stack overflow of the > softirq stack. > > Previously it wasn't safe to do so because irq_exit() (which initiates > softirq processing) was called on the hardirq stack. > > That was changed in commit 1b1b6a6f4cc0 ("powerpc: handle irq_enter/ > irq_exit in interrupt handler wrappers") and 1346d00e1bdf ("powerpc: > Don't select HAVE_IRQ_EXIT_ON_IRQ_STACK"). > > The allowed transitions are now: > - process stack -> hardirq stack > - process stack -> softirq stack > - process stack -> softirq stack -> hardirq stack > > Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > arch/powerpc/kernel/irq.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c > index 6f7d4edaa0bc..7504ceec5c58 100644 > --- a/arch/powerpc/kernel/irq.c > +++ b/arch/powerpc/kernel/irq.c > @@ -284,15 +284,14 @@ static __always_inline void call_do_irq(struct pt_regs *regs, void *sp) > void __do_IRQ(struct pt_regs *regs) > { > struct pt_regs *old_regs = set_irq_regs(regs); > - void *cursp, *irqsp, *sirqsp; > + void *cursp, *irqsp; > > /* Switch to the irq stack to handle this */ > cursp = (void *)(current_stack_pointer & ~(THREAD_SIZE - 1)); > irqsp = hardirq_ctx[raw_smp_processor_id()]; > - sirqsp = softirq_ctx[raw_smp_processor_id()]; > > /* Already there ? If not switch stack and call */ > - if (unlikely(cursp == irqsp || cursp == sirqsp)) > + if (unlikely(cursp == irqsp)) > __do_irq(regs, current_stack_pointer); > else > call_do_irq(regs, irqsp);
On Thu, 30 Nov 2023 23:50:45 +1100, Michael Ellerman wrote: > Allow a transition from the softirq stack to the hardirq stack when > handling a hardirq. Doing so means a hardirq received while deep in > softirq processing is less likely to cause a stack overflow of the > softirq stack. > > Previously it wasn't safe to do so because irq_exit() (which initiates > softirq processing) was called on the hardirq stack. > > [...] Applied to powerpc/next. [1/1] powerpc/irq: Allow softirq to hardirq stack transition https://git.kernel.org/powerpc/c/4eb20bf34ea296f648971a8528e32cd80efcbe89 cheers
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c index 6f7d4edaa0bc..7504ceec5c58 100644 --- a/arch/powerpc/kernel/irq.c +++ b/arch/powerpc/kernel/irq.c @@ -284,15 +284,14 @@ static __always_inline void call_do_irq(struct pt_regs *regs, void *sp) void __do_IRQ(struct pt_regs *regs) { struct pt_regs *old_regs = set_irq_regs(regs); - void *cursp, *irqsp, *sirqsp; + void *cursp, *irqsp; /* Switch to the irq stack to handle this */ cursp = (void *)(current_stack_pointer & ~(THREAD_SIZE - 1)); irqsp = hardirq_ctx[raw_smp_processor_id()]; - sirqsp = softirq_ctx[raw_smp_processor_id()]; /* Already there ? If not switch stack and call */ - if (unlikely(cursp == irqsp || cursp == sirqsp)) + if (unlikely(cursp == irqsp)) __do_irq(regs, current_stack_pointer); else call_do_irq(regs, irqsp);
Allow a transition from the softirq stack to the hardirq stack when handling a hardirq. Doing so means a hardirq received while deep in softirq processing is less likely to cause a stack overflow of the softirq stack. Previously it wasn't safe to do so because irq_exit() (which initiates softirq processing) was called on the hardirq stack. That was changed in commit 1b1b6a6f4cc0 ("powerpc: handle irq_enter/ irq_exit in interrupt handler wrappers") and 1346d00e1bdf ("powerpc: Don't select HAVE_IRQ_EXIT_ON_IRQ_STACK"). The allowed transitions are now: - process stack -> hardirq stack - process stack -> softirq stack - process stack -> softirq stack -> hardirq stack Signed-off-by: Michael Ellerman <mpe@ellerman.id.au> --- arch/powerpc/kernel/irq.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)