Re: [PATCH v4 12/12] powerpc: Rename soft_enabled to soft_disabled_mask

[Nicholas Piggin <npiggin@gmail.com>]

On Mon, 19 Dec 2016 13:37:08 +0530
Madhavan Srinivasan <maddy@linux.vnet.ibm.com> wrote:

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


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

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

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