On Mon, Aug 08, 2016 at 05:48:28PM +0100, Mark Rutland wrote:

Hi,

[adding Lorenzo]

On Mon, Aug 08, 2016 at 12:03:38PM -0400, Chris Metcalf wrote:

quoted

When doing an nmi backtrace of many cores, most of which are idle,
the output is a little overwhelming and very uninformative.  Suppress
messages for cpus that are idling when they are interrupted and just
emit one line, "NMI backtrace for N skipped: idling at pc 0xNNN".

We do this by grouping all the cpuidle code together into a new
.cpuidle.text section, and then checking the address of the
interrupted PC to see if it lies within that section.

This commit suitably tags x86, arm64, and tile idle routines,
and only adds in the minimal framework for other architectures.

quoted

diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index 659963d40bb4..fe7f93b7b11b 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S

@@ -122,6 +122,7 @@ SECTIONS
 			ENTRY_TEXT
 			TEXT_TEXT
 			SCHED_TEXT
+			CPUIDLE_TEXT
 			LOCK_TEXT
 			KPROBES_TEXT
 			HYPERVISOR_TEXT

diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
index 5bb61de23201..64f088ca3192 100644
--- a/arch/arm64/mm/proc.S
+++ b/arch/arm64/mm/proc.S

@@ -48,11 +48,13 @@
  *
  *	Idle the processor (wait for interrupt).
  */
+	.pushsection ".cpuidle.text","ax"
 ENTRY(cpu_do_idle)
 	dsb	sy				// WFI may enter a low-power mode
 	wfi
 	ret
 ENDPROC(cpu_do_idle)
+	.popsection

From a quick scan it looks like we only call this with interrupts
disabled, and we have no NMI. So shouldn't we be annotating
arch_cpu_idle(), which calls this and subsequently enables interrupts?

I'm also not sure what you need to do for PSCI, which is the preferred
(FW-backed) idle mechanism for arm64. The infrastrucure for that is
spread over a few files:

  arch/arm64/kernel/sleep.S
  arch/arm64/kernel/smccc-call.S
  arch/arm64/kernel/suspend.c
  drivers/cpuidle/cpuidle-arm.c
  drivers/firmware/psci.c

I'm not sure where we'd be an an interruptible state, and therefore I'm
not immediately sure what we should annotate.

I am probably missing something here, but let me add that I am not
sure I understand how this patch can be used on ARM/ARM64 systems
so ARM platform idle back-end code annotation is basically useless
given that it is code that can't be preempted anyway (and even if
it could PC range check can even fail given that we may execute some
code with MMU off so out of physical addresses).

What's the purpose of this cpu idle tracking ? Can't it be implemented
in a simpler way (ie RCU API) ?

Lorenzo