* [PATCH] powerpc: check_and_cede_processor never cedes
@ 2012-06-27 22:45 Anton Blanchard
2012-06-27 22:51 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 4+ messages in thread
From: Anton Blanchard @ 2012-06-27 22:45 UTC (permalink / raw)
To: benh, paulus; +Cc: linuxppc-dev
Commit f948501b36c6 ("Make hard_irq_disable() actually hard-disable
interrupts") caused check_and_cede_processor to stop working.
->irq_happened will never be zero right after a hard_irq_disable
so the compiler removes the call to cede_processor completely.
The bug was introduced back in the lazy interrupt handling rework
of 3.4 but was hidden until recently because hard_irq_disable did
nothing.
This issue will eventually appear in 3.4 stable since the
hard_irq_disable fix is marked stable, so mark this one for stable
too.
Signed-off-by: Anton Blanchard <anton@samba.org>
Cc: stable@vger.kernel.org
---
Index: linux-build/arch/powerpc/platforms/pseries/processor_idle.c
===================================================================
--- linux-build.orig/arch/powerpc/platforms/pseries/processor_idle.c 2012-06-27 21:20:45.403761715 +1000
+++ linux-build/arch/powerpc/platforms/pseries/processor_idle.c 2012-06-27 21:57:14.796788823 +1000
@@ -106,7 +106,7 @@ static void check_and_cede_processor(voi
* we first hard disable then check.
*/
hard_irq_disable();
- if (get_paca()->irq_happened == 0)
+ if (get_paca()->irq_happened == PACA_IRQ_HARD_DIS)
cede_processor();
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc: check_and_cede_processor never cedes
2012-06-27 22:45 [PATCH] powerpc: check_and_cede_processor never cedes Anton Blanchard
@ 2012-06-27 22:51 ` Benjamin Herrenschmidt
2012-06-27 23:13 ` Anton Blanchard
0 siblings, 1 reply; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2012-06-27 22:51 UTC (permalink / raw)
To: Anton Blanchard; +Cc: linuxppc-dev, paulus
On Thu, 2012-06-28 at 08:45 +1000, Anton Blanchard wrote:
> hard_irq_disable();
> - if (get_paca()->irq_happened == 0)
> + if (get_paca()->irq_happened == PACA_IRQ_HARD_DIS)
> cede_processor();
I'd rather add a helper, something like lazy_irq_pending()
and hide the actual check for the bits in irq_happened, in
case we change the scheme again.
Something like:
static inline bool lazy_irq_pending(void)
{
return !!(get_paca()->irq_happened & ~PACA_IRQ_HARD_DIS);
}
Cheers,
Ben.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] powerpc: check_and_cede_processor never cedes
2012-06-27 22:51 ` Benjamin Herrenschmidt
@ 2012-06-27 23:13 ` Anton Blanchard
2012-06-28 7:10 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 4+ messages in thread
From: Anton Blanchard @ 2012-06-27 23:13 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, paulus
Hi,
> I'd rather add a helper, something like lazy_irq_pending()
> and hide the actual check for the bits in irq_happened, in
> case we change the scheme again.
Good idea. Look ok?
--
Commit f948501b36c6 ("Make hard_irq_disable() actually hard-disable
interrupts") caused check_and_cede_processor to stop working.
->irq_happened will never be zero right after a hard_irq_disable
so the compiler removes the call to cede_processor completely.
The bug was introduced back in the lazy interrupt handling rework
of 3.4 but was hidden until recently because hard_irq_disable did
nothing.
This issue will eventually appear in 3.4 stable since the
hard_irq_disable fix is marked stable, so mark this one for stable
too.
Signed-off-by: Anton Blanchard <anton@samba.org>
Cc: stable@vger.kernel.org
---
v2: create a helper, suggested by Ben.
Index: linux-build/arch/powerpc/platforms/pseries/processor_idle.c
===================================================================
--- linux-build.orig/arch/powerpc/platforms/pseries/processor_idle.c 2012-06-28 08:55:09.422198154 +1000
+++ linux-build/arch/powerpc/platforms/pseries/processor_idle.c 2012-06-28 08:57:36.112591023 +1000
@@ -106,7 +106,7 @@ static void check_and_cede_processor(voi
* we first hard disable then check.
*/
hard_irq_disable();
- if (get_paca()->irq_happened == 0)
+ if (!lazy_irq_pending())
cede_processor();
}
Index: linux-build/arch/powerpc/include/asm/hw_irq.h
===================================================================
--- linux-build.orig/arch/powerpc/include/asm/hw_irq.h 2012-06-21 09:16:26.265354429 +1000
+++ linux-build/arch/powerpc/include/asm/hw_irq.h 2012-06-28 08:59:22.082320381 +1000
@@ -103,6 +103,11 @@ static inline void hard_irq_disable(void
/* include/linux/interrupt.h needs hard_irq_disable to be a macro */
#define hard_irq_disable hard_irq_disable
+static inline bool lazy_irq_pending(void)
+{
+ return !!(get_paca()->irq_happened & ~PACA_IRQ_HARD_DIS);
+}
+
/*
* This is called by asynchronous interrupts to conditionally
* re-enable hard interrupts when soft-disabled after having
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc: check_and_cede_processor never cedes
2012-06-27 23:13 ` Anton Blanchard
@ 2012-06-28 7:10 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2012-06-28 7:10 UTC (permalink / raw)
To: Anton Blanchard; +Cc: linuxppc-dev, paulus
On Thu, 2012-06-28 at 09:13 +1000, Anton Blanchard wrote:
> Hi,
>
> > I'd rather add a helper, something like lazy_irq_pending()
> > and hide the actual check for the bits in irq_happened, in
> > case we change the scheme again.
>
> Good idea. Look ok?
>
> --
>
> Commit f948501b36c6 ("Make hard_irq_disable() actually hard-disable
> interrupts") caused check_and_cede_processor to stop working.
> ->irq_happened will never be zero right after a hard_irq_disable
> so the compiler removes the call to cede_processor completely.
>
> The bug was introduced back in the lazy interrupt handling rework
> of 3.4 but was hidden until recently because hard_irq_disable did
> nothing.
>
> This issue will eventually appear in 3.4 stable since the
> hard_irq_disable fix is marked stable, so mark this one for stable
> too.
Yup, looks good, I'll send to Linus tomorrow.
Cheers,
Ben.
> Signed-off-by: Anton Blanchard <anton@samba.org>
> Cc: stable@vger.kernel.org
> ---
>
> v2: create a helper, suggested by Ben.
>
> Index: linux-build/arch/powerpc/platforms/pseries/processor_idle.c
> ===================================================================
> --- linux-build.orig/arch/powerpc/platforms/pseries/processor_idle.c 2012-06-28 08:55:09.422198154 +1000
> +++ linux-build/arch/powerpc/platforms/pseries/processor_idle.c 2012-06-28 08:57:36.112591023 +1000
> @@ -106,7 +106,7 @@ static void check_and_cede_processor(voi
> * we first hard disable then check.
> */
> hard_irq_disable();
> - if (get_paca()->irq_happened == 0)
> + if (!lazy_irq_pending())
> cede_processor();
> }
>
> Index: linux-build/arch/powerpc/include/asm/hw_irq.h
> ===================================================================
> --- linux-build.orig/arch/powerpc/include/asm/hw_irq.h 2012-06-21 09:16:26.265354429 +1000
> +++ linux-build/arch/powerpc/include/asm/hw_irq.h 2012-06-28 08:59:22.082320381 +1000
> @@ -103,6 +103,11 @@ static inline void hard_irq_disable(void
> /* include/linux/interrupt.h needs hard_irq_disable to be a macro */
> #define hard_irq_disable hard_irq_disable
>
> +static inline bool lazy_irq_pending(void)
> +{
> + return !!(get_paca()->irq_happened & ~PACA_IRQ_HARD_DIS);
> +}
> +
> /*
> * This is called by asynchronous interrupts to conditionally
> * re-enable hard interrupts when soft-disabled after having
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-06-28 7:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-27 22:45 [PATCH] powerpc: check_and_cede_processor never cedes Anton Blanchard
2012-06-27 22:51 ` Benjamin Herrenschmidt
2012-06-27 23:13 ` Anton Blanchard
2012-06-28 7:10 ` Benjamin Herrenschmidt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).