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

Madhavan Srinivasan maddy at linux.vnet.ibm.com
Wed Jan 4 20:33:03 AEDT 2017



On Tuesday 20 December 2016 08:28 AM, Nicholas Piggin wrote:
> On Mon, 19 Dec 2016 13:37:08 +0530
> Madhavan Srinivasan <maddy at 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))
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
>



More information about the Linuxppc-dev mailing list