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

On Wed, 4 Jan 2017 22:21:05 +0530
Madhavan Srinivasan [off-list ref] wrote:

On Wednesday 04 January 2017 06:08 PM, Nicholas Piggin wrote:

quoted

On Wed,  4 Jan 2017 17:19:46 +0530
Madhavan Srinivasan [off-list ref] wrote:
 

quoted

@@ -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