[PATCH v5 08/12] powerpc: Add support to mask perf interrupts and replay them

[Nicholas Piggin]

On Wed, 4 Jan 2017 22:21:05 +0530
Madhavan Srinivasan <maddy at linux.vnet.ibm.com> wrote:

> On Wednesday 04 January 2017 06:08 PM, Nicholas Piggin wrote:
> > On Wed,  4 Jan 2017 17:19:46 +0530
> > Madhavan Srinivasan <maddy at linux.vnet.ibm.com> wrote:
> >  
> >> @@ -134,7 +137,7 @@ static inline bool arch_irqs_disabled(void)
> >>   	_was_enabled = local_paca->soft_enabled;	\
> >>   	local_paca->soft_enabled = IRQ_DISABLE_MASK_LINUX;\
> >>   	local_paca->irq_happened |= PACA_IRQ_HARD_DIS;	\
> >> -	if (!(_was_enabled & IRQ_DISABLE_MASK_LINUX))	\
> >> +	if (!(_was_enabled & IRQ_DISABLE_MASK_ALL))	\
> >>   		trace_hardirqs_off();			\
> >>   } while(0)  
> > Hang on, maybe there's some confusion about this. trace_hardirqs_off() is
> > for Linux irqs (i.e., local_irq_disable()), so that should continue to
> > test just the LINUX mask I think. Otherwise this
> >
> >      powerpc_local_pmu_disable();
> >      hard_irq_disable();  
> 
> Currently we set both bits for pmu soft disable
> 
>                  flags = 
> soft_disabled_mask_or_return(IRQ_DISABLE_MASK_LINUX | \
> IRQ_DISABLE_MASK_PMU);                  \
> 
> So yes in the above seq, we will miss the pmu bit. But since 
> trace_hardirqs_off()
> is for _LINUX, instead will it not be safer to OR it?
> 
>          local_paca->soft_disabled_mask |= IRQ_DISABLE_MASK_LINUX;\

I would just say set all unconditionally. Despite "Linux" IRQs being the
special case, I would much prefer it to be written as much as possible
with all mask bits being equal, and then cases where LINUX bits need to
be handled differently built on that.

We might end up adding more bits, or even splitting the current single
LINUX bit into several others.

hard disable effectively masks all the interrupts, so then it seems we
should set them all in the disabled mask rather than just one. The
special case would then be just the test for the LINUX bit when deciding
whether to call trace_hardirqs_off().

Thanks,
Nick