remove PURR read from context switch and timer

PATCH New Nicholas Piggin 1 participant 6 messages Mar 28, 2018 PowerPC

lore

Share & tools

lore

Nicholas Piggin

author reviewer

2018-03-28 09:28 UTC #0 lore raw

cover remove PURR read from context switch and timer

And some associated cleanups to the timer code as a result

Nicholas Piggin (5):
powerpc/64: remove start_tb and accum_tb from thread_struct powerpc/pseries: lparcfg calculate PURR on demand
powerpc: clockevents broadcast receiver use
tick_receive_broadcast
powerpc: move timer broadcast code under GENERIC_CLOCKEVENTS_BROADCAST
ifdef
powerpc: move a stray NMI IPI case under NMI_IPI ifdef

 arch/powerpc/include/asm/hw_irq.h        |  1 +
 arch/powerpc/include/asm/processor.h     |  4 --
 arch/powerpc/include/asm/time.h          |  9 ---
 arch/powerpc/kernel/process.c            | 18 ------
 arch/powerpc/kernel/smp.c                | 14 ++++-
 arch/powerpc/kernel/time.c               | 94 ++++++++++++++------------------
 arch/powerpc/platforms/pseries/lparcfg.c | 18 +++---
 7 files changed, 64 insertions(+), 94 deletions(-)

Signature · 1 line

2.16.1

Nicholas Piggin

author reviewer

2018-03-28 09:28 UTC #1 lore raw

PATCH 1/5 powerpc/64: remove start_tb and accum_tb from thread_struct

These fields are only written to.

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/include/asm/processor.h | 4 ----
 arch/powerpc/kernel/process.c        | 6 +-----
 2 files changed, 1 insertion(+), 9 deletions(-)

arch/powerpc/include/asm/processor.h +0 -4

index 01299cdc9806..67eedcc6d803 100644
@@ -251,10 +251,6 @@ struct thread_struct {
 	struct thread_fp_state	*fp_save_area;
 	int		fpexc_mode;	/* floating-point exception mode */
 	unsigned int	align_ctl;	/* alignment handling control */
-#ifdef CONFIG_PPC64
-	unsigned long	start_tb;	/* Start purr when proc switched in */
-	unsigned long	accum_tb;	/* Total accumulated purr for process */
-#endif
 #ifdef CONFIG_HAVE_HW_BREAKPOINT
 	struct perf_event *ptrace_bps[HBP_NUM];
 	/*

arch/powerpc/kernel/process.c +1 -5

index ec4f363ebb89..9626b991da52 100644
@@ -1169,11 +1169,7 @@ struct task_struct *__switch_to(struct task_struct *prev,
 	 */
 	if (firmware_has_feature(FW_FEATURE_SPLPAR)) {
 		struct cpu_usage *cu = this_cpu_ptr(&cpu_usage_array);
-		long unsigned start_tb, current_tb;
-		start_tb = old_thread->start_tb;
-		cu->current_tb = current_tb = mfspr(SPRN_PURR);
-		old_thread->accum_tb += (current_tb - start_tb);
-		new_thread->start_tb = current_tb;
+		cu->current_tb = mfspr(SPRN_PURR);
 	}
 #endif /* CONFIG_PPC64 */

Signature · 1 line

2.16.1

Nicholas Piggin

author reviewer

2018-03-28 09:28 UTC #2 lore raw

PATCH 2/5 powerpc/pseries: lparcfg calculate PURR on demand

In reply to Nicholas Piggin

For SPLPAR, lparcfg provides a sum of PURR registers for all CPUs. Currently this is done by reading PURR in context switch and timer interrupt, and storing that into a per-CPU variable. These are summed to provide the value.

This does not work with all timer schemes (e.g., NO_HZ_FULL), and it is sub-optimal for performance because it reads the PURR register on every context switch, although that's been difficult to distinguish from noise in the contxt_switch microbenchmark.

This patch implements the sum by calling a function on each CPU, to read and add PURR values of each CPU.

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/include/asm/time.h          |  8 --------
 arch/powerpc/kernel/process.c            | 14 --------------
 arch/powerpc/kernel/time.c               |  8 --------
 arch/powerpc/platforms/pseries/lparcfg.c | 18 ++++++++++--------
 4 files changed, 10 insertions(+), 38 deletions(-)

arch/powerpc/include/asm/time.h +0 -8

index db546c034905..c965c79765c4 100644
@@ -196,14 +196,6 @@ extern u64 mulhdu(u64, u64);
 extern void div128_by_32(u64 dividend_high, u64 dividend_low,
 			 unsigned divisor, struct div_result *dr);
 
-/* Used to store Processor Utilization register (purr) values */
-
-struct cpu_usage {
-        u64 current_tb;  /* Holds the current purr register values */
-};
-
-DECLARE_PER_CPU(struct cpu_usage, cpu_usage_array);
-
 extern void secondary_cpu_time_init(void);
 extern void __init time_init(void);

arch/powerpc/kernel/process.c +0 ~~-14~~

index 9626b991da52..48cb033ecd33 100644
@@ -827,10 +827,6 @@ void set_breakpoint(struct arch_hw_breakpoint *brk)
 	preempt_enable();
 }
 
-#ifdef CONFIG_PPC64
-DEFINE_PER_CPU(struct cpu_usage, cpu_usage_array);
-#endif
-
 static inline bool hw_brk_match(struct arch_hw_breakpoint *a,
 			      struct arch_hw_breakpoint *b)
 {

@@ -1163,16 +1159,6 @@ struct task_struct *__switch_to(struct task_struct *prev,
 
 	WARN_ON(!irqs_disabled());
 
-#ifdef CONFIG_PPC64
-	/*
-	 * Collect processor utilization data per process
-	 */
-	if (firmware_has_feature(FW_FEATURE_SPLPAR)) {
-		struct cpu_usage *cu = this_cpu_ptr(&cpu_usage_array);
-		cu->current_tb = mfspr(SPRN_PURR);
-	}
-#endif /* CONFIG_PPC64 */
-
 #ifdef CONFIG_PPC_BOOK3S_64
 	batch = this_cpu_ptr(&ppc64_tlb_batch);
 	if (batch->active) {

arch/powerpc/kernel/time.c +0 -8

index f7d96a68ecaa..3e531ab4e836 100644
@@ -565,14 +565,6 @@ static void __timer_interrupt(void)
 		__this_cpu_inc(irq_stat.timer_irqs_others);
 	}
 
-#ifdef CONFIG_PPC64
-	/* collect purr register values often, for accurate calculations */
-	if (firmware_has_feature(FW_FEATURE_SPLPAR)) {
-		struct cpu_usage *cu = this_cpu_ptr(&cpu_usage_array);
-		cu->current_tb = mfspr(SPRN_PURR);
-	}
-#endif
-
 	trace_timer_interrupt_exit(regs);
 }

arch/powerpc/platforms/pseries/lparcfg.c +10 -8

index c508c938dc71..7c872dc01bdb 100644
@@ -52,18 +52,20 @@
  * Track sum of all purrs across all processors. This is used to further
  * calculate usage values by different applications
  */
+static void cpu_get_purr(void *arg)
+{
+	atomic64_t *sum = arg;
+
+	atomic64_add(mfspr(SPRN_PURR), sum);
+}
+
 static unsigned long get_purr(void)
 {
-	unsigned long sum_purr = 0;
-	int cpu;
+	atomic64_t purr = ATOMIC64_INIT(0);
 
-	for_each_possible_cpu(cpu) {
-		struct cpu_usage *cu;
+	on_each_cpu(cpu_get_purr, &purr, 1);
 
-		cu = &per_cpu(cpu_usage_array, cpu);
-		sum_purr += cu->current_tb;
-	}
-	return sum_purr;
+	return atomic64_read(&purr);
 }
 
 /*

Signature · 1 line

2.16.1

Nicholas Piggin

author reviewer

2018-03-28 09:28 UTC #3 lore raw

PATCH 3/5 powerpc: clockevents broadcast receiver use tick_receive_broadcast

In reply to Nicholas Piggin

The broadcast tick recipient can call tick_receive_broadcast rather than re-running the full timer interrupt.

It does not not have to check for the next event time, because the sender already determined the timer has expired. It does not have to test irq_work_pending, because that's a direct decrementer interrupt and does not go through the clock events subsystem.

This results in no code size change, but both the decrementer and broadcast path lengths are reduced.

Cc Srivatsa S. Bhat <[email protected]>

Cc Preeti U Murthy <[email protected]>

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/include/asm/hw_irq.h |  1 +
 arch/powerpc/include/asm/time.h   |  1 -
 arch/powerpc/kernel/smp.c         |  4 +-
 arch/powerpc/kernel/time.c        | 84 ++++++++++++++++++---------------------
 4 files changed, 42 insertions(+), 48 deletions(-)

arch/powerpc/include/asm/hw_irq.h +1 -0

index 855e17d158b1..dcdb95a13c47 100644
@@ -55,6 +55,7 @@ extern void replay_system_reset(void);
 extern void __replay_interrupt(unsigned int vector);
 
 extern void timer_interrupt(struct pt_regs *);
+extern void timer_broadcast_interrupt(void);
 extern void performance_monitor_exception(struct pt_regs *regs);
 extern void WatchdogException(struct pt_regs *regs);
 extern void unknown_exception(struct pt_regs *regs);

arch/powerpc/include/asm/time.h +0 -1

index c965c79765c4..69b89f941252 100644
@@ -28,7 +28,6 @@ extern struct clock_event_device decrementer_clockevent;
 
 struct rtc_time;
 extern void to_tm(int tim, struct rtc_time * tm);
-extern void tick_broadcast_ipi_handler(void);
 
 extern void generic_calibrate_decr(void);
 extern void hdec_interrupt(struct pt_regs *regs);

arch/powerpc/kernel/smp.c +2 -2

index bbe7634b3a43..de5ac63b4ad6 100644
@@ -157,7 +157,7 @@ static irqreturn_t reschedule_action(int irq, void *data)
 
 static irqreturn_t tick_broadcast_ipi_action(int irq, void *data)
 {
-	tick_broadcast_ipi_handler();
+	timer_broadcast_interrupt();
 	return IRQ_HANDLED;
 }

@@ -278,7 +278,7 @@ irqreturn_t smp_ipi_demux_relaxed(void)
 		if (all & IPI_MESSAGE(PPC_MSG_RESCHEDULE))
 			scheduler_ipi();
 		if (all & IPI_MESSAGE(PPC_MSG_TICK_BROADCAST))
-			tick_broadcast_ipi_handler();
+			timer_broadcast_interrupt();
 #ifdef CONFIG_NMI_IPI
 		if (all & IPI_MESSAGE(PPC_MSG_NMI_IPI))
 			nmi_ipi_action(0, NULL);

arch/powerpc/kernel/time.c +39 ~~-45~~

114 lines not on this page · raw message

Signature · 1 line

2.16.1

Nicholas Piggin

author reviewer

2018-03-28 09:28 UTC #4 lore raw

PATCH 4/5 powerpc: move timer broadcast code under GENERIC_CLOCKEVENTS_BROADCAST ifdef

In reply to Nicholas Piggin

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/kernel/smp.c  | 8 ++++++++
 arch/powerpc/kernel/time.c | 2 ++
 2 files changed, 10 insertions(+)

arch/powerpc/kernel/smp.c +8 -0

index de5ac63b4ad6..17c6abed3def 100644
@@ -155,11 +155,13 @@ static irqreturn_t reschedule_action(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 static irqreturn_t tick_broadcast_ipi_action(int irq, void *data)
 {
 	timer_broadcast_interrupt();
 	return IRQ_HANDLED;
 }
+#endif
 
 #ifdef CONFIG_NMI_IPI
 static irqreturn_t nmi_ipi_action(int irq, void *data)

@@ -172,7 +174,9 @@ static irqreturn_t nmi_ipi_action(int irq, void *data)
 static irq_handler_t smp_ipi_action[] = {
 	[PPC_MSG_CALL_FUNCTION] =  call_function_action,
 	[PPC_MSG_RESCHEDULE] = reschedule_action,
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 	[PPC_MSG_TICK_BROADCAST] = tick_broadcast_ipi_action,
+#endif
 #ifdef CONFIG_NMI_IPI
 	[PPC_MSG_NMI_IPI] = nmi_ipi_action,
 #endif

@@ -186,7 +190,9 @@ static irq_handler_t smp_ipi_action[] = {
 const char *smp_ipi_name[] = {
 	[PPC_MSG_CALL_FUNCTION] =  "ipi call function",
 	[PPC_MSG_RESCHEDULE] = "ipi reschedule",
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 	[PPC_MSG_TICK_BROADCAST] = "ipi tick-broadcast",
+#endif
 	[PPC_MSG_NMI_IPI] = "nmi ipi",
 };

@@ -277,8 +283,10 @@ irqreturn_t smp_ipi_demux_relaxed(void)
 			generic_smp_call_function_interrupt();
 		if (all & IPI_MESSAGE(PPC_MSG_RESCHEDULE))
 			scheduler_ipi();
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 		if (all & IPI_MESSAGE(PPC_MSG_TICK_BROADCAST))
 			timer_broadcast_interrupt();
+#endif
 #ifdef CONFIG_NMI_IPI
 		if (all & IPI_MESSAGE(PPC_MSG_NMI_IPI))
 			nmi_ipi_action(0, NULL);

arch/powerpc/kernel/time.c +2 -0

index 199f330e8f01..4c02b6725844 100644
@@ -604,6 +604,7 @@ void timer_interrupt(struct pt_regs *regs)
 }
 EXPORT_SYMBOL(timer_interrupt);
 
+#ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 void timer_broadcast_interrupt(void)
 {
 	u64 *next_tb = this_cpu_ptr(&decrementers_next_tb);

@@ -615,6 +616,7 @@ void timer_broadcast_interrupt(void)
 	__this_cpu_inc(irq_stat.timer_irqs_event);
 	trace_timer_interrupt_exit(regs);
 }
+#endif
 
 /*
  * Hypervisor decrementer interrupts shouldn't occur but are sometimes

Signature · 1 line

2.16.1

Nicholas Piggin

author reviewer

2018-03-28 09:28 UTC #5 lore raw

PATCH 5/5 powerpc: move a stray NMI IPI case under NMI_IPI ifdef

In reply to Nicholas Piggin

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/kernel/smp.c | 2 ++
 1 file changed, 2 insertions(+)

arch/powerpc/kernel/smp.c +2 -0

index 17c6abed3def..6185b7245e9e 100644
@@ -193,7 +193,9 @@ const char *smp_ipi_name[] = {
 #ifdef CONFIG_GENERIC_CLOCKEVENTS_BROADCAST
 	[PPC_MSG_TICK_BROADCAST] = "ipi tick-broadcast",
 #endif
+#ifdef CONFIG_NMI_IPI
 	[PPC_MSG_NMI_IPI] = "nmi ipi",
+#endif
 };
 
 /* optional function to request ipi, for controllers with >= 4 ipis */

Signature · 1 line

2.16.1