Re: [PATCH v7 4/4] nmi_backtrace: generate one-line reports for idle cpus

[Lorenzo Pieralisi]

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:
> > 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.
> 
> > 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