On Tuesday 20 December 2016 08:28 AM, Nicholas Piggin wrote:

On Mon, 19 Dec 2016 13:37:08 +0530
Madhavan Srinivasan [off-list ref] wrote:

quoted

Rename the paca->soft_enabled to paca->soft_disabled_mask as
it is no more used as a flag for interrupt state.

This makes it much more readable, thanks. I have a question which
isn't part of this patch but I just notice it now:

quoted

@@ -193,8 +193,8 @@ static inline bool arch_irqs_disabled(void)
  #define hard_irq_disable()	do {			\
  	u8 _was_enabled;				\
  	__hard_irq_disable();				\
-	_was_enabled = local_paca->soft_enabled;	\
-	local_paca->soft_enabled = IRQ_DISABLE_MASK_LINUX;\
+	_was_enabled = local_paca->soft_disabled_mask;	\
+	local_paca->soft_disabled_mask = IRQ_DISABLE_MASK_LINUX;\
  	local_paca->irq_happened |= PACA_IRQ_HARD_DIS;	\
  	if (_was_enabled == IRQ_DISABLE_MASK_NONE)	\
  		trace_hardirqs_off();			\

trace_hardirqs_off() is the Linux interrupt disable, i.e., the MASK_LINUX
bit. So I think the test should be:

if (!(_was_enabled & IRQ_DISABLE_MASK_LINUX))

spot on. nice catch. Infact we should move this as part to
patch 5.

After your rename it should be called _was_masked instead  I guess.

  Too obvious, my bad. Will change it.

Also I suppose the new soft disable mask should include all interrupt
bits, shouldn't it? It would be confusing to get the situation where

Yes That is correct. Condition should include pmu bits also.
Will fix it.

Maddy

hard_irq_disable() strips the PMU bit off the mask. If you agree, then
it would be good to add IRQ_DISABLE_MASK_ALL define where the bits are
defined.

Thanks,
Nick