[PATCH] generic/softirq: Disable softirq stacks on PREEMPT_RT

[PATCH] generic/softirq: Disable softirq stacks on PREEMPT_RT

[Sebastian Andrzej Siewior]

From: Thomas Gleixner <tglx@linutronix.de>

PREEMPT_RT preempts softirqs and the current implementation avoids
do_softirq_own_stack() and only uses __do_softirq().

Disable the unused softirqs stacks on PREEMPT_RT to safe some memory and
ensure that do_softirq_own_stack() is not used which is not expected.

[bigeasy: commit description.]

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 include/asm-generic/softirq_stack.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/asm-generic/softirq_stack.h b/include/asm-generic/softirq_stack.h
index eceeecf6a5bd8..d3e2d81656e04 100644
--- a/include/asm-generic/softirq_stack.h
+++ b/include/asm-generic/softirq_stack.h
@@ -2,7 +2,7 @@
 #ifndef __ASM_GENERIC_SOFTIRQ_STACK_H
 #define __ASM_GENERIC_SOFTIRQ_STACK_H
 
-#ifdef CONFIG_HAVE_SOFTIRQ_ON_OWN_STACK
+#if defined(CONFIG_HAVE_SOFTIRQ_ON_OWN_STACK) && !defined(CONFIG_PREEMPT_RT)
 void do_softirq_own_stack(void);
 #else
 static inline void do_softirq_own_stack(void)
-- 
2.36.1

Re: [PATCH] generic/softirq: Disable softirq stacks on PREEMPT_RT

[Arnd Bergmann]

On Mon, Jun 13, 2022 at 8:27 PM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> From: Thomas Gleixner <tglx@linutronix.de>
>
> PREEMPT_RT preempts softirqs and the current implementation avoids
> do_softirq_own_stack() and only uses __do_softirq().
>
> Disable the unused softirqs stacks on PREEMPT_RT to safe some memory and
> ensure that do_softirq_own_stack() is not used which is not expected.
>
> [bigeasy: commit description.]
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

Acked-by: Arnd Bergmann <arnd@arndb.de>

It's probably best to keep this together with the corresponding architecture
specific changes. I was a bit worried about bisection at first, but
then realized
that this is not a problem for mainline since ARCH_SUPPORTS_RT is not
yet enabled on any architecture.

How are softirqs called on preempt_rt? Does this result in higher stack usage
for the normal task stacks again, or are they run in separate threads anyway?

          Arnd

Re: [PATCH] generic/softirq: Disable softirq stacks on PREEMPT_RT

[Sebastian Andrzej Siewior]

On 2022-06-13 23:13:49 [+0200], Arnd Bergmann wrote:
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> 
> It's probably best to keep this together with the corresponding architecture
> specific changes. I was a bit worried about bisection at first, but
> then realized
> that this is not a problem for mainline since ARCH_SUPPORTS_RT is not
> yet enabled on any architecture.

So where do I put this patch to? If I remember correctly then arm64 is
using this. ARM has its own thing and x86 has this change already.

> How are softirqs called on preempt_rt? Does this result in higher stack usage
> for the normal task stacks again, or are they run in separate threads anyway?

one of:
- ksoftirqd thread.
- in the force-threaded interrupt after the main handler.
- any time after bh-counter hits zero due local_bh_enable().

The last one will cause higher task stacks.

>           Arnd

Sebastian

Re: [PATCH] generic/softirq: Disable softirq stacks on PREEMPT_RT

[Arnd Bergmann]

On Tue, Jun 14, 2022 at 9:11 AM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> On 2022-06-13 23:13:49 [+0200], Arnd Bergmann wrote:
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> >
> > It's probably best to keep this together with the corresponding architecture
> > specific changes. I was a bit worried about bisection at first, but
> > then realized
> > that this is not a problem for mainline since ARCH_SUPPORTS_RT is not
> > yet enabled on any architecture.
>
> So where do I put this patch to? If I remember correctly then arm64 is
> using this. ARM has its own thing and x86 has this change already.

I don't see HAVE_SOFTIRQ_ON_OWN_STACK for arm, parisc, powerpc,
s390, sh, sparc and x86, but not arm64. I would suggest having a single
patch that does the same change across all architectures that don't already do
this, and then merging it either through tip or through my asm-generic tree.

> - ksoftirqd thread.
> - in the force-threaded interrupt after the main handler.
> - any time after bh-counter hits zero due local_bh_enable().
>
> The last one will cause higher task stacks.

Does this mean it only happens when a softirq gets raised from task context
(which would be predictable), or also at an arbitrary time if it gets raised
by a non-threaded hardirq or IPI?

        Arnd

Re: [PATCH] generic/softirq: Disable softirq stacks on PREEMPT_RT

[Sebastian Andrzej Siewior]

On 2022-06-14 12:07:48 [+0200], Arnd Bergmann wrote:
> > So where do I put this patch to? If I remember correctly then arm64 is
> > using this. ARM has its own thing and x86 has this change already.
> 
> I don't see HAVE_SOFTIRQ_ON_OWN_STACK for arm, parisc, powerpc,
> s390, sh, sparc and x86, but not arm64. I would suggest having a single
> patch that does the same change across all architectures that don't already do
> this, and then merging it either through tip or through my asm-generic tree.

Oh. I posted the ARM bits with my other ARM patch as a mini two patch
series a few secs before this.

I could group this softirq change for all architectures in a single
patch. But then powerpc didn't want the "PREEMPT_RT" annotation for the
warning/ stack backtrace and they may not be too keen about this. So for
powerpc I was thinking to present them all at once.
Looking at sparc and parisc, there might be more to it than just this.
Both were never tested while I have the missing bits for arm* and
powerpc in my queue.

Eitherway, if you want I can regroup and send you the softirq bits for
all arches.

> > - ksoftirqd thread.
> > - in the force-threaded interrupt after the main handler.
> > - any time after bh-counter hits zero due local_bh_enable().
> >
> > The last one will cause higher task stacks.
> 
> Does this mean it only happens when a softirq gets raised from task context
> (which would be predictable), or also at an arbitrary time if it gets raised
> by a non-threaded hardirq or IPI?

If the softirq gets raised from non-task context (hardirq or IPI) then
it will be deferred to ksoftirqd (and not invoked on irq-exit path).

>         Arnd

Sebastian

Re: [PATCH] generic/softirq: Disable softirq stacks on PREEMPT_RT

[Arnd Bergmann]

On Tue, Jun 14, 2022 at 12:47 PM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> On 2022-06-14 12:07:48 [+0200], Arnd Bergmann wrote:
> > > So where do I put this patch to? If I remember correctly then arm64 is
> > > using this. ARM has its own thing and x86 has this change already.
> >
> > I don't see HAVE_SOFTIRQ_ON_OWN_STACK for arm, parisc, powerpc,
> > s390, sh, sparc and x86, but not arm64. I would suggest having a single
> > patch that does the same change across all architectures that don't already do
> > this, and then merging it either through tip or through my asm-generic tree.
>
> Oh. I posted the ARM bits with my other ARM patch as a mini two patch
> series a few secs before this.

Yes, I saw these, and that's why I wondered about it, because they looked
like they should not be applied independently.

> I could group this softirq change for all architectures in a single
> patch. But then powerpc didn't want the "PREEMPT_RT" annotation for the
> warning/ stack backtrace and they may not be too keen about this. So for
> powerpc I was thinking to present them all at once.
> Looking at sparc and parisc, there might be more to it than just this.
> Both were never tested while I have the missing bits for arm* and
> powerpc in my queue.
>
> Eitherway, if you want I can regroup and send you the softirq bits for
> all arches.

Yes, I think this would be good. These are still targeted for next, right?

> > > - ksoftirqd thread.
> > > - in the force-threaded interrupt after the main handler.
> > > - any time after bh-counter hits zero due local_bh_enable().
> > >
> > > The last one will cause higher task stacks.
> >
> > Does this mean it only happens when a softirq gets raised from task context
> > (which would be predictable), or also at an arbitrary time if it gets raised
> > by a non-threaded hardirq or IPI?
>
> If the softirq gets raised from non-task context (hardirq or IPI) then
> it will be deferred to ksoftirqd (and not invoked on irq-exit path).

Ok, got it, that means the stack usage is still going to be reproducible.

I wonder how common this case is in practice, but it clearly makes
sense at least from a time accounting perspective.

      Arnd

Re: [PATCH] generic/softirq: Disable softirq stacks on PREEMPT_RT

[Sebastian Andrzej Siewior]

On 2022-06-14 15:32:16 [+0200], Arnd Bergmann wrote:
> Yes, I saw these, and that's why I wondered about it, because they looked
> like they should not be applied independently.
> 
> > I could group this softirq change for all architectures in a single
> > patch. But then powerpc didn't want the "PREEMPT_RT" annotation for the
> > warning/ stack backtrace and they may not be too keen about this. So for
> > powerpc I was thinking to present them all at once.
> > Looking at sparc and parisc, there might be more to it than just this.
> > Both were never tested while I have the missing bits for arm* and
> > powerpc in my queue.
> >
> > Eitherway, if you want I can regroup and send you the softirq bits for
> > all arches.
> 
> Yes, I think this would be good. These are still targeted for next, right?

Yup. Let me do this then.

>       Arnd

Sebastian