[PATCH 00/10] softirq: Consolidate and optimize softirq mask

[PATCH 00/10] softirq: Consolidate and optimize softirq mask

[Frederic Weisbecker]

The softirq mask and its accessors/mutators have many implementations
scattered around many architectures. Most do the same things consisting
in a field in a per-cpu struct (often irq_cpustat_t) accessed through
per-cpu ops. We can provide instead a generic efficient version that
most of them can use. In fact s390 is the only exception because the
field is stored in lowcore.

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
	softirq/mask

HEAD: fb3db1bfa6033202c4b8191e570f39d3777d6a9d

Thanks,
	Frederic
---

Frederic Weisbecker (10):
      ia64: Convert local_softirq_pending() to per-cpu ops
      sparc: Convert local_softirq_pending() to use per-cpu op
      softirq: Turn default irq_cpustat_t to standard per-cpu
      softirq: Consolidate default local_softirq_pending() implementations
      ia64: Switch to generic local_softirq_pending() implementation
      parisc: Switch to generic local_softirq_pending() implementation
      powerpc: Switch to generic local_softirq_pending() implementation
      sparc: Switch to generic local_softirq_pending() implementation
      x86: Switch to generic local_softirq_pending() implementation
      softirq: Remove __ARCH_SET_SOFTIRQ_PENDING


 arch/ia64/include/asm/hardirq.h     |  2 +-
 arch/parisc/include/asm/hardirq.h   |  8 --------
 arch/powerpc/include/asm/hardirq.h  |  7 -------
 arch/sparc/include/asm/hardirq_64.h |  5 +++--
 arch/x86/include/asm/hardirq.h      |  8 --------
 include/linux/interrupt.h           | 16 ++++++++++++++--
 include/linux/irq_cpustat.h         | 10 +++-------
 kernel/softirq.c                    |  4 ++--
 8 files changed, 23 insertions(+), 37 deletions(-)

[PATCH 01/10] ia64: Convert local_softirq_pending() to per-cpu ops

[Frederic Weisbecker]

In order to consolidate and optimize generic softirq mask accesses, we
first need to convert architectures to use per-cpu operations when
possible.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: James E.J. Bottomley <jejb@parisc-linux.org>
Cc: Helge Deller <deller@gmx.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/ia64/include/asm/hardirq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/ia64/include/asm/hardirq.h b/arch/ia64/include/asm/hardirq.h
index bdc4669..22fae71 100644
--- a/arch/ia64/include/asm/hardirq.h
+++ b/arch/ia64/include/asm/hardirq.h
@@ -13,7 +13,7 @@
 
 #define __ARCH_IRQ_STAT	1
 
-#define local_softirq_pending()		(local_cpu_data->softirq_pending)
+#define local_softirq_pending()	(*this_cpu_ptr(&ia64_cpu_info.softirq_pending))
 
 #include <linux/threads.h>
 #include <linux/irq.h>
-- 
2.7.4

[PATCH 02/10] sparc: Convert local_softirq_pending() to use per-cpu op

[Frederic Weisbecker]

In order to consolidate and optimize generic softirq mask accesses, we
first need to convert architectures to use per-cpu operations when
possible.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: James E.J. Bottomley <jejb@parisc-linux.org>
Cc: Helge Deller <deller@gmx.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/sparc/include/asm/hardirq_64.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/sparc/include/asm/hardirq_64.h b/arch/sparc/include/asm/hardirq_64.h
index f565402..6aba904 100644
--- a/arch/sparc/include/asm/hardirq_64.h
+++ b/arch/sparc/include/asm/hardirq_64.h
@@ -11,7 +11,7 @@
 
 #define __ARCH_IRQ_STAT
 #define local_softirq_pending() \
-	(local_cpu_data().__softirq_pending)
+	(*this_cpu_ptr(&__cpu_data.__softirq_pending))
 
 void ack_bad_irq(unsigned int irq);
 
-- 
2.7.4

[PATCH 03/10] softirq: Turn default irq_cpustat_t to standard per-cpu

[Frederic Weisbecker]

In order to optimize and consolidate softirq mask accesses, let's
convert the default irq_cpustat_t implementation to per-CPU standard API.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: James E.J. Bottomley <jejb@parisc-linux.org>
Cc: Helge Deller <deller@gmx.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
---
 include/linux/irq_cpustat.h | 4 ++--
 kernel/softirq.c            | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/irq_cpustat.h b/include/linux/irq_cpustat.h
index 4954948..ddea03c 100644
--- a/include/linux/irq_cpustat.h
+++ b/include/linux/irq_cpustat.h
@@ -18,8 +18,8 @@
  */
 
 #ifndef __ARCH_IRQ_STAT
-extern irq_cpustat_t irq_stat[];		/* defined in asm/hardirq.h */
-#define __IRQ_STAT(cpu, member)	(irq_stat[cpu].member)
+DECLARE_PER_CPU_ALIGNED(irq_cpustat_t, irq_stat);	/* defined in asm/hardirq.h */
+#define __IRQ_STAT(cpu, member)	(per_cpu(irq_stat.member, cpu))
 #endif
 
   /* arch independent irq_stat fields */
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 24d243e..fdbb171 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -49,8 +49,8 @@
  */
 
 #ifndef __ARCH_IRQ_STAT
-irq_cpustat_t irq_stat[NR_CPUS] ____cacheline_aligned;
-EXPORT_SYMBOL(irq_stat);
+DEFINE_PER_CPU_ALIGNED(irq_cpustat_t, irq_stat);
+EXPORT_PER_CPU_SYMBOL(irq_stat);
 #endif
 
 static struct softirq_action softirq_vec[NR_SOFTIRQS] __cacheline_aligned_in_smp;
-- 
2.7.4

[PATCH 04/10] softirq: Consolidate default local_softirq_pending() implementations

[Frederic Weisbecker]

Consolidate and optimize default softirq mask API implementations.
Per-CPU operations are expected to be faster and a few architectures
already rely on them to implement local_softirq_pending() and related
accessors/mutators. Those will be migrated to the new generic code.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: James E.J. Bottomley <jejb@parisc-linux.org>
Cc: Helge Deller <deller@gmx.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
---
 include/linux/interrupt.h   | 14 ++++++++++++++
 include/linux/irq_cpustat.h |  6 +-----
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 69c2382..01caeca 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -434,11 +434,25 @@ extern bool force_irqthreads;
 #define force_irqthreads	(0)
 #endif
 
+#ifndef local_softirq_pending
+
+#ifndef local_softirq_pending_ref
+#define local_softirq_pending_ref irq_stat.__softirq_pending
+#endif
+
+#define local_softirq_pending()	(__this_cpu_read(local_softirq_pending_ref))
+#define set_softirq_pending(x)	(__this_cpu_write(local_softirq_pending_ref, (x)))
+#define or_softirq_pending(x)	(__this_cpu_or(local_softirq_pending_ref, (x)))
+
+#else /* local_softirq_pending */
+
 #ifndef __ARCH_SET_SOFTIRQ_PENDING
 #define set_softirq_pending(x) (local_softirq_pending() = (x))
 #define or_softirq_pending(x)  (local_softirq_pending() |= (x))
 #endif
 
+#endif /* local_softirq_pending */
+
 /* Some architectures might implement lazy enabling/disabling of
  * interrupts. In some cases, such as stop_machine, we might want
  * to ensure that after a local_irq_disable(), interrupts have
diff --git a/include/linux/irq_cpustat.h b/include/linux/irq_cpustat.h
index ddea03c..6e8895c 100644
--- a/include/linux/irq_cpustat.h
+++ b/include/linux/irq_cpustat.h
@@ -22,11 +22,7 @@ DECLARE_PER_CPU_ALIGNED(irq_cpustat_t, irq_stat);	/* defined in asm/hardirq.h */
 #define __IRQ_STAT(cpu, member)	(per_cpu(irq_stat.member, cpu))
 #endif
 
-  /* arch independent irq_stat fields */
-#define local_softirq_pending() \
-	__IRQ_STAT(smp_processor_id(), __softirq_pending)
-
-  /* arch dependent irq_stat fields */
+/* arch dependent irq_stat fields */
 #define nmi_count(cpu)		__IRQ_STAT((cpu), __nmi_count)	/* i386 */
 
 #endif	/* __irq_cpustat_h */
-- 
2.7.4

[PATCH 05/10] ia64: Switch to generic local_softirq_pending() implementation

[Frederic Weisbecker]

Benefit from the generic softirq mask implementation that rely on per-CPU
mutators instead of working with raw operators on top of this_cpu_ptr().

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: James E.J. Bottomley <jejb@parisc-linux.org>
Cc: Helge Deller <deller@gmx.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/ia64/include/asm/hardirq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/ia64/include/asm/hardirq.h b/arch/ia64/include/asm/hardirq.h
index 22fae71..ccde7c2 100644
--- a/arch/ia64/include/asm/hardirq.h
+++ b/arch/ia64/include/asm/hardirq.h
@@ -13,7 +13,7 @@
 
 #define __ARCH_IRQ_STAT	1
 
-#define local_softirq_pending()	(*this_cpu_ptr(&ia64_cpu_info.softirq_pending))
+#define local_softirq_pending_ref	ia64_cpu_info.softirq_pending
 
 #include <linux/threads.h>
 #include <linux/irq.h>
-- 
2.7.4

[PATCH 06/10] parisc: Switch to generic local_softirq_pending() implementation

[Frederic Weisbecker]

Remove the ad-hoc implementation, the generic code now allows us not to
reinvent the wheel.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: James E.J. Bottomley <jejb@parisc-linux.org>
Cc: Helge Deller <deller@gmx.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/parisc/include/asm/hardirq.h | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/arch/parisc/include/asm/hardirq.h b/arch/parisc/include/asm/hardirq.h
index 0778151..1a1235a 100644
--- a/arch/parisc/include/asm/hardirq.h
+++ b/arch/parisc/include/asm/hardirq.h
@@ -34,14 +34,6 @@ DECLARE_PER_CPU_SHARED_ALIGNED(irq_cpustat_t, irq_stat);
 #define __IRQ_STAT(cpu, member) (irq_stat[cpu].member)
 #define inc_irq_stat(member)	this_cpu_inc(irq_stat.member)
 #define __inc_irq_stat(member)	__this_cpu_inc(irq_stat.member)
-#define local_softirq_pending()	this_cpu_read(irq_stat.__softirq_pending)
-
-#define __ARCH_SET_SOFTIRQ_PENDING
-
-#define set_softirq_pending(x)	\
-		this_cpu_write(irq_stat.__softirq_pending, (x))
-#define or_softirq_pending(x)	this_cpu_or(irq_stat.__softirq_pending, (x))
-
 #define ack_bad_irq(irq) WARN(1, "unexpected IRQ trap at vector %02x\n", irq)
 
 #endif /* _PARISC_HARDIRQ_H */
-- 
2.7.4

[PATCH 07/10] powerpc: Switch to generic local_softirq_pending() implementation

[Frederic Weisbecker]

Remove the ad-hoc implementation, the generic code now allows us not to
reinvent the wheel.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: James E.J. Bottomley <jejb@parisc-linux.org>
Cc: Helge Deller <deller@gmx.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/powerpc/include/asm/hardirq.h | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/arch/powerpc/include/asm/hardirq.h b/arch/powerpc/include/asm/hardirq.h
index 5986d47..383f628 100644
--- a/arch/powerpc/include/asm/hardirq.h
+++ b/arch/powerpc/include/asm/hardirq.h
@@ -25,15 +25,8 @@ typedef struct {
 DECLARE_PER_CPU_SHARED_ALIGNED(irq_cpustat_t, irq_stat);
 
 #define __ARCH_IRQ_STAT
-
-#define local_softirq_pending()	__this_cpu_read(irq_stat.__softirq_pending)
-
-#define __ARCH_SET_SOFTIRQ_PENDING
 #define __ARCH_IRQ_EXIT_IRQS_DISABLED
 
-#define set_softirq_pending(x) __this_cpu_write(irq_stat.__softirq_pending, (x))
-#define or_softirq_pending(x) __this_cpu_or(irq_stat.__softirq_pending, (x))
-
 static inline void ack_bad_irq(unsigned int irq)
 {
 	printk(KERN_CRIT "unexpected IRQ trap at vector %02x\n", irq);
-- 
2.7.4

[PATCH 08/10] sparc: Switch to generic local_softirq_pending() implementation

[Frederic Weisbecker]

Benefit from the generic softirq mask implementation that rely on per-CPU
mutators instead of working with raw operators on top of this_cpu_ptr().

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: James E.J. Bottomley <jejb@parisc-linux.org>
Cc: Helge Deller <deller@gmx.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/sparc/include/asm/hardirq_64.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/sparc/include/asm/hardirq_64.h b/arch/sparc/include/asm/hardirq_64.h
index 6aba904..75b92bf 100644
--- a/arch/sparc/include/asm/hardirq_64.h
+++ b/arch/sparc/include/asm/hardirq_64.h
@@ -10,8 +10,9 @@
 #include <asm/cpudata.h>
 
 #define __ARCH_IRQ_STAT
-#define local_softirq_pending() \
-	(*this_cpu_ptr(&__cpu_data.__softirq_pending))
+
+#define local_softirq_pending_ref \
+	__cpu_data.__softirq_pending
 
 void ack_bad_irq(unsigned int irq);
 
-- 
2.7.4

[PATCH 09/10] x86: Switch to generic local_softirq_pending() implementation

[Frederic Weisbecker]

Remove the ad-hoc implementation, the generic code now allows us not to
reinvent the wheel.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: James E.J. Bottomley <jejb@parisc-linux.org>
Cc: Helge Deller <deller@gmx.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
---
 arch/x86/include/asm/hardirq.h | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h
index 7c341a7..fd73beb 100644
--- a/arch/x86/include/asm/hardirq.h
+++ b/arch/x86/include/asm/hardirq.h
@@ -49,14 +49,6 @@ DECLARE_PER_CPU_SHARED_ALIGNED(irq_cpustat_t, irq_stat);
 
 #define inc_irq_stat(member)	this_cpu_inc(irq_stat.member)
 
-#define local_softirq_pending()	this_cpu_read(irq_stat.__softirq_pending)
-
-#define __ARCH_SET_SOFTIRQ_PENDING
-
-#define set_softirq_pending(x)	\
-		this_cpu_write(irq_stat.__softirq_pending, (x))
-#define or_softirq_pending(x)	this_cpu_or(irq_stat.__softirq_pending, (x))
-
 extern void ack_bad_irq(unsigned int irq);
 
 extern u64 arch_irq_stat_cpu(unsigned int cpu);
-- 
2.7.4

[PATCH 10/10] softirq: Remove __ARCH_SET_SOFTIRQ_PENDING

[Frederic Weisbecker]

The last user of __ARCH_SET_SOFTIRQ_PENDING has been converted to generic
per-cpu softirq mask. We can now remove this conditional.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: David S. Miller <davem@davemloft.net>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: James E.J. Bottomley <jejb@parisc-linux.org>
Cc: Helge Deller <deller@gmx.de>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
---
 include/linux/interrupt.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 01caeca..4237618 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -446,10 +446,8 @@ extern bool force_irqthreads;
 
 #else /* local_softirq_pending */
 
-#ifndef __ARCH_SET_SOFTIRQ_PENDING
 #define set_softirq_pending(x) (local_softirq_pending() = (x))
 #define or_softirq_pending(x)  (local_softirq_pending() |= (x))
-#endif
 
 #endif /* local_softirq_pending */
 
-- 
2.7.4

Re: [PATCH 10/10] softirq: Remove __ARCH_SET_SOFTIRQ_PENDING

[Peter Zijlstra]

On Thu, Mar 29, 2018 at 04:27:05AM +0200, Frederic Weisbecker wrote:
> The last user of __ARCH_SET_SOFTIRQ_PENDING has been converted to generic
> per-cpu softirq mask. We can now remove this conditional.

This seems like half a cleanup; who still has local_softirq_pending()
after this?

>  
>  #else /* local_softirq_pending */
>  
> -#ifndef __ARCH_SET_SOFTIRQ_PENDING
>  #define set_softirq_pending(x) (local_softirq_pending() = (x))
>  #define or_softirq_pending(x)  (local_softirq_pending() |= (x))
> -#endif
>  
>  #endif /* local_softirq_pending */

Re: [PATCH 10/10] softirq: Remove __ARCH_SET_SOFTIRQ_PENDING

[Frederic Weisbecker]

On Thu, Mar 29, 2018 at 09:16:19AM +0200, Peter Zijlstra wrote:
> On Thu, Mar 29, 2018 at 04:27:05AM +0200, Frederic Weisbecker wrote:
> > The last user of __ARCH_SET_SOFTIRQ_PENDING has been converted to generic
> > per-cpu softirq mask. We can now remove this conditional.
> 
> This seems like half a cleanup; who still has local_softirq_pending()
> after this?

Only s390 because it uses lowcore to store such cpu data.

Re: [PATCH 10/10] softirq: Remove __ARCH_SET_SOFTIRQ_PENDING

[Peter Zijlstra]

On Thu, Mar 29, 2018 at 03:47:46PM +0200, Frederic Weisbecker wrote:
> On Thu, Mar 29, 2018 at 09:16:19AM +0200, Peter Zijlstra wrote:
> > On Thu, Mar 29, 2018 at 04:27:05AM +0200, Frederic Weisbecker wrote:
> > > The last user of __ARCH_SET_SOFTIRQ_PENDING has been converted to generic
> > > per-cpu softirq mask. We can now remove this conditional.
> > 
> > This seems like half a cleanup; who still has local_softirq_pending()
> > after this?
> 
> Only s390 because it uses lowcore to store such cpu data.

Is it worth keeping it there? It seems an aweful shame to keep this
stuff special cased for just the one arch. At the very least this
should've mentioned s390 is special and why.

And I don't see s390 people on Cc either.

Re: [PATCH 10/10] softirq: Remove __ARCH_SET_SOFTIRQ_PENDING

[Frederic Weisbecker]

On Thu, Mar 29, 2018 at 04:01:11PM +0200, Peter Zijlstra wrote:
> On Thu, Mar 29, 2018 at 03:47:46PM +0200, Frederic Weisbecker wrote:
> > On Thu, Mar 29, 2018 at 09:16:19AM +0200, Peter Zijlstra wrote:
> > > On Thu, Mar 29, 2018 at 04:27:05AM +0200, Frederic Weisbecker wrote:
> > > > The last user of __ARCH_SET_SOFTIRQ_PENDING has been converted to generic
> > > > per-cpu softirq mask. We can now remove this conditional.
> > > 
> > > This seems like half a cleanup; who still has local_softirq_pending()
> > > after this?
> > 
> > Only s390 because it uses lowcore to store such cpu data.
> 
> Is it worth keeping it there? It seems an aweful shame to keep this
> stuff special cased for just the one arch. At the very least this
> should've mentioned s390 is special and why.

Right, I thought well about moving that special case to s390. I can
do that in v2.

> 
> And I don't see s390 people on Cc either.

Oops, forgot to add them.

Thanks.

Re: [PATCH 10/10] softirq: Remove __ARCH_SET_SOFTIRQ_PENDING

[Martin Schwidefsky]

On Thu, 29 Mar 2018 16:07:49 +0200
Frederic Weisbecker <frederic@kernel.org> wrote:

> On Thu, Mar 29, 2018 at 04:01:11PM +0200, Peter Zijlstra wrote:
> > On Thu, Mar 29, 2018 at 03:47:46PM +0200, Frederic Weisbecker wrote:  
> > > On Thu, Mar 29, 2018 at 09:16:19AM +0200, Peter Zijlstra wrote:  
> > > > On Thu, Mar 29, 2018 at 04:27:05AM +0200, Frederic Weisbecker wrote:  
> > > > > The last user of __ARCH_SET_SOFTIRQ_PENDING has been converted to generic
> > > > > per-cpu softirq mask. We can now remove this conditional.  
> > > > 
> > > > This seems like half a cleanup; who still has local_softirq_pending()
> > > > after this?  
> > > 
> > > Only s390 because it uses lowcore to store such cpu data.  
> > 
> > Is it worth keeping it there? It seems an aweful shame to keep this
> > stuff special cased for just the one arch. At the very least this
> > should've mentioned s390 is special and why.  
> 
> Right, I thought well about moving that special case to s390. I can
> do that in v2.

The lowcore optimization for softirq_pending field is not really needed,
just nice to have. But if there is a strong reason to make a common
definition for it we can certainly do that.

First try has hit the wall due to include file hell though.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH 10/10] softirq: Remove __ARCH_SET_SOFTIRQ_PENDING

[Frederic Weisbecker]

On Thu, Mar 29, 2018 at 04:53:43PM +0200, Martin Schwidefsky wrote:
> On Thu, 29 Mar 2018 16:07:49 +0200
> Frederic Weisbecker <frederic@kernel.org> wrote:
> 
> > On Thu, Mar 29, 2018 at 04:01:11PM +0200, Peter Zijlstra wrote:
> > > On Thu, Mar 29, 2018 at 03:47:46PM +0200, Frederic Weisbecker wrote:  
> > > > On Thu, Mar 29, 2018 at 09:16:19AM +0200, Peter Zijlstra wrote:  
> > > > > On Thu, Mar 29, 2018 at 04:27:05AM +0200, Frederic Weisbecker wrote:  
> > > > > > The last user of __ARCH_SET_SOFTIRQ_PENDING has been converted to generic
> > > > > > per-cpu softirq mask. We can now remove this conditional.  
> > > > > 
> > > > > This seems like half a cleanup; who still has local_softirq_pending()
> > > > > after this?  
> > > > 
> > > > Only s390 because it uses lowcore to store such cpu data.  
> > > 
> > > Is it worth keeping it there? It seems an aweful shame to keep this
> > > stuff special cased for just the one arch. At the very least this
> > > should've mentioned s390 is special and why.  
> > 
> > Right, I thought well about moving that special case to s390. I can
> > do that in v2.
> 
> The lowcore optimization for softirq_pending field is not really needed,
> just nice to have. But if there is a strong reason to make a common
> definition for it we can certainly do that.

I think there is no need to. Lowcore is faster to access than per-cpu on s390
and we are dealing with a frequently accessed field. Plus lowcore is expected to
be often cache-hot.

But Peter is right that I should move the default implementation of
or_softirq_pending() and set_softirq_pending() to s390 as it's the last
user of these after this patchset.

Thanks.

Re: [PATCH 10/10] softirq: Remove __ARCH_SET_SOFTIRQ_PENDING

[Peter Zijlstra]

On Thu, Mar 29, 2018 at 04:53:43PM +0200, Martin Schwidefsky wrote:
> The lowcore optimization for softirq_pending field is not really needed,
> just nice to have. But if there is a strong reason to make a common
> definition for it we can certainly do that.

A slightly related question; would it make sense to move all kernel
static per-cpu stuff into lowcore, or is that asking for too much
trickery?

Re: [PATCH 10/10] softirq: Remove __ARCH_SET_SOFTIRQ_PENDING

[Martin Schwidefsky]

On Thu, 29 Mar 2018 20:08:36 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Thu, Mar 29, 2018 at 04:53:43PM +0200, Martin Schwidefsky wrote:
> > The lowcore optimization for softirq_pending field is not really needed,
> > just nice to have. But if there is a strong reason to make a common
> > definition for it we can certainly do that.  
> 
> A slightly related question; would it make sense to move all kernel
> static per-cpu stuff into lowcore, or is that asking for too much
> trickery?
 
The space in lowcore is quite limited, for zArch the structure is 8K with
many pre-defined fields. I fear that putting all of the static per-cpu
stuff in there is too much. 

So far I used the lowcore as optimization for selected per-cpu fields
which are performance relevant.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [PATCH 10/10] softirq: Remove __ARCH_SET_SOFTIRQ_PENDING

[Peter Zijlstra]

On Tue, Apr 03, 2018 at 07:52:25AM +0200, Martin Schwidefsky wrote:
> On Thu, 29 Mar 2018 20:08:36 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Thu, Mar 29, 2018 at 04:53:43PM +0200, Martin Schwidefsky wrote:
> > > The lowcore optimization for softirq_pending field is not really needed,
> > > just nice to have. But if there is a strong reason to make a common
> > > definition for it we can certainly do that.  
> > 
> > A slightly related question; would it make sense to move all kernel
> > static per-cpu stuff into lowcore, or is that asking for too much
> > trickery?
>  
> The space in lowcore is quite limited, for zArch the structure is 8K with
> many pre-defined fields. I fear that putting all of the static per-cpu
> stuff in there is too much. 
> 
> So far I used the lowcore as optimization for selected per-cpu fields
> which are performance relevant.

Fair enough; and yes 8k isn't much. Thanks for the info.