Re: [PATCH v4 12/12] powerpc: Rename soft_enabled to soft_disabled_mask
From: Nicholas Piggin <npiggin@gmail.com>
Date: 2016-12-20 02:59:08
On Mon, 19 Dec 2016 13:37:08 +0530 Madhavan Srinivasan [off-list ref] 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:
quoted hunk ↗ jump to hunk
@@ -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