Subject: Re: [PATCH] powerpc: Make hard_irq_disable() do the right thing vs. irq tracing

From: "tiejun.chen" <[email-masked]>
Date: Tue, 7 May 2013 15:15:57 +0800

On 05/07/2013 03:04 PM, Benjamin Herrenschmidt wrote:
> If hard_irq_disable() is called while interrupts are already soft-disabled
> (which is the most common case) all is already well.
>
> However you can (and in some cases want) to call it while everything is
> enabled (to make sure you don't get a lazy even, for example before entry
> into KVM guests) and in this case we need to inform the irq tracer that
> the irqs are going off.
>
> We have to change the inline into a macro to avoid an include circular
> dependency hell hole.
>
> Signed-off-by: Benjamin Herrenschmidt <[email-masked]>
> ---
>
> Tested on pseries, Scott, I don't expect a problem with that patch especially
> since most callers already are soft disabled, so I'll merge it now along with
> my other pending stuff and you can simplify your KVM one.
>
> diff --git a/arch/powerpc/include/asm/hw_irq.h b/arch/powerpc/include/asm/hw_irq.h
> index e45c494..d615b28 100644
> --- a/arch/powerpc/include/asm/hw_irq.h
> +++ b/arch/powerpc/include/asm/hw_irq.h
> @@ -95,15 +95,13 @@ static inline bool arch_irqs_disabled(void)
>   #define __hard_irq_disable()	__mtmsrd(local_paca->kernel_msr, 1)
>   #endif
>
> -static inline void hard_irq_disable(void)
> -{
> -	__hard_irq_disable();
> -	get_paca()->soft_enabled = 0;
> -	get_paca()->irq_happened |= PACA_IRQ_HARD_DIS;
> -}
> -
> -/* include/linux/interrupt.h needs hard_irq_disable to be a macro */
> -#define hard_irq_disable	hard_irq_disable
> +#define hard_irq_disable()	do {			\
> +	__hard_irq_disable();				\
> +	if (local_paca->soft_enabled)			\
> +		trace_hardirqs_off();			\
> +	get_paca()->soft_enabled = 0;			\

Could we simplify this as follows:

+#define hard_irq_disable()	do {			\
+	__hard_irq_disable();				\
+	if (get_paca()->soft_enabled) {			\
+		trace_hardirqs_off();			\
+		get_paca()->soft_enabled = 0;		\
+	}						\
+	get_paca()->irq_happened |= PACA_IRQ_HARD_DIS;	\
+} while(0)

Tiejun

> +	get_paca()->irq_happened |= PACA_IRQ_HARD_DIS;	\
> +} while(0)
>
>   static inline bool lazy_irq_pending(void)
>   {
>
>