2021-02-05 02:02:01

by Thomas Gleixner

[permalink] [raw]
Subject: [patch 11/12] softirq: Allow inlining do_softirq_own_stack()

The function to switch to the irq stack on x86 is now minimal and there is
only a single caller. Allow the stack switch to be inlined.

Signed-off-by: Thomas Gleixner <[email protected]>
---
include/linux/interrupt.h | 2 ++
kernel/softirq.c | 4 ++++
2 files changed, 6 insertions(+)

--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -570,7 +570,9 @@ asmlinkage void do_softirq(void);
asmlinkage void __do_softirq(void);

#ifdef __ARCH_HAS_DO_SOFTIRQ
+# ifndef __ARCH_HAS_DO_SOFTIRQ_INLINE
void do_softirq_own_stack(void);
+# endif
#else
static inline void do_softirq_own_stack(void)
{
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -26,6 +26,10 @@
#include <linux/tick.h>
#include <linux/irq.h>

+#ifdef __ARCH_HAS_DO_SOFTIRQ_INLINE
+# include <asm/irq_stack.h>
+#endif
+
#define CREATE_TRACE_POINTS
#include <trace/events/irq.h>



2021-02-05 10:20:23

by Lai Jiangshan

[permalink] [raw]
Subject: Re: [patch 11/12] softirq: Allow inlining do_softirq_own_stack()

On Fri, Feb 5, 2021 at 10:04 AM Thomas Gleixner <[email protected]> wrote:
>
> The function to switch to the irq stack on x86 is now minimal and there is
> only a single caller. Allow the stack switch to be inlined.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> include/linux/interrupt.h | 2 ++
> kernel/softirq.c | 4 ++++
> 2 files changed, 6 insertions(+)
>
> --- a/include/linux/interrupt.h
> +++ b/include/linux/interrupt.h
> @@ -570,7 +570,9 @@ asmlinkage void do_softirq(void);
> asmlinkage void __do_softirq(void);
>
> #ifdef __ARCH_HAS_DO_SOFTIRQ
> +# ifndef __ARCH_HAS_DO_SOFTIRQ_INLINE
> void do_softirq_own_stack(void);
> +# endif
> #else
> static inline void do_softirq_own_stack(void)
> {

Hello

This patch and the next patch have three "#if[n]def" with
__ARCH_HAS_DO_SOFTIRQ_INLINE and this one is nested in
__ARCH_HAS_DO_SOFTIRQ.

I wonder if we can use __ARCH_HAS_DO_SOFTIRQ only.

For example, we can move "void do_softirq_own_stack(void);" to around
the code where __ARCH_HAS_DO_SOFTIRQ are defined in very ARCHs.
(And for x86, do_softirq_own_stack() is a macro instead of function
declaration as next patch shows)

Thanks
Lai

> --- a/kernel/softirq.c
> +++ b/kernel/softirq.c
> @@ -26,6 +26,10 @@
> #include <linux/tick.h>
> #include <linux/irq.h>
>
> +#ifdef __ARCH_HAS_DO_SOFTIRQ_INLINE
> +# include <asm/irq_stack.h>
> +#endif
> +
> #define CREATE_TRACE_POINTS
> #include <trace/events/irq.h>
>
>

2021-02-05 11:46:05

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 11/12] softirq: Allow inlining do_softirq_own_stack()

On Fri, Feb 05 2021 at 18:14, Lai Jiangshan wrote:
> On Fri, Feb 5, 2021 at 10:04 AM Thomas Gleixner <[email protected]> wrote:
>> static inline void do_softirq_own_stack(void)
>> {
>
> Hello
>
> This patch and the next patch have three "#if[n]def" with
> __ARCH_HAS_DO_SOFTIRQ_INLINE and this one is nested in
> __ARCH_HAS_DO_SOFTIRQ.
>
> I wonder if we can use __ARCH_HAS_DO_SOFTIRQ only.
>
> For example, we can move "void do_softirq_own_stack(void);" to around
> the code where __ARCH_HAS_DO_SOFTIRQ are defined in very ARCHs.
> (And for x86, do_softirq_own_stack() is a macro instead of function
> declaration as next patch shows)

We can do that as well. No strong preference.

Thanks,

tglx

2021-02-10 00:16:40

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 11/12] softirq: Allow inlining do_softirq_own_stack()

Lai,

On Fri, Feb 05 2021 at 12:38, Thomas Gleixner wrote:
> On Fri, Feb 05 2021 at 18:14, Lai Jiangshan wrote:
>> On Fri, Feb 5, 2021 at 10:04 AM Thomas Gleixner <[email protected]> wrote:
>>> static inline void do_softirq_own_stack(void)
>>> {
>>
>> Hello
>>
>> This patch and the next patch have three "#if[n]def" with
>> __ARCH_HAS_DO_SOFTIRQ_INLINE and this one is nested in
>> __ARCH_HAS_DO_SOFTIRQ.
>>
>> I wonder if we can use __ARCH_HAS_DO_SOFTIRQ only.
>>
>> For example, we can move "void do_softirq_own_stack(void);" to around
>> the code where __ARCH_HAS_DO_SOFTIRQ are defined in very ARCHs.
>> (And for x86, do_softirq_own_stack() is a macro instead of function
>> declaration as next patch shows)
>
> We can do that as well. No strong preference.

actually it's not that trivial. It ends up in include hell and then
pulls the whole irq stack macro mess into every file which includes
interrupt.h.

I've moved the default function prototype and the stub into
asm-generic/softirq_stack.h and let x86 override it.

Thanks,

tglx