Fix cleanup race in xnpod_disable_timesource

PATCH New Jan Kiszka 1 participant 1 message Jan 27, 2008

Share & tools

Jan Kiszka

author

2008-01-27 15:04 UTC #0 lore raw

PATCH 1/1 Fix cleanup race in xnpod_disable_timesource

CPU n (n!=0)
------------
xnpod_disable_timesource()
for (cpu = 0..x)
rthal_timer_release(cpu=0)
ipipe_release_tickdev(cpu=0)
rthal_timer_set_oneshot(0)
rthal_trigger_irq(RTHAL_HOST_TICK_IRQ)
...
lapic_next_event(...)
xnarch_next_htick_shot(...)
...
rthal_timer_release(cpu=n)
ipipe_release_tickdev(cpu=n)

And dead it is, the lapic on this CPU. Attached patch fixes the issue (and gets rid off multiple RTHAL_BCAST_TICK_IRQ deregistrations).

Jan

---
 ChangeLog                  |    5 +++++
 ksrc/arch/x86/hal-common.c |   18 +++++++++++-------
 2 files changed, 16 insertions(+), 7 deletions(-)

ChangeLog +5 -0

Index: b/ChangeLog
===================================================================
@@ -1,3 +1,8 @@
+2008-01-27  Jan Kiszka  <[email protected]>
+
+	* ksrc/arch/x86/hal-common.c: Fix race when starting the cleanup
+	on CPU > 0. Avoid multiple RTHAL_BCAST_TICK_IRQ releases.
+
 2008-01-26  Gilles Chanteperdrix  <[email protected]>
 
 	* configure.in: Fix --enable-linux-build for latest arch/x86

ksrc/arch/x86/hal-common.c +11 -7

Index: b/ksrc/arch/x86/hal-common.c
===================================================================
@@ -120,6 +120,8 @@ DECLARE_LINUX_IRQ_HANDLER(rthal_broadcas
 	return IRQ_HANDLED;
 }
 
+static int cpu_timers_requested;
+
 #ifdef CONFIG_GENERIC_CLOCKEVENTS
 
 int rthal_timer_request(

@@ -179,7 +181,7 @@ int rthal_timer_request(
 	 * The rest of the initialization should only be performed
 	 * once by a single CPU.
 	 */
-	if (cpu > 0)
+	if (cpu_timers_requested++ > 0)
 		goto out;
 
 	err = rthal_irq_request(RTHAL_APIC_TIMER_IPI,

@@ -219,7 +221,7 @@ int rthal_timer_request(void (*tick_hand
 	 * The rest of the initialization should only be performed
 	 * once by a single CPU.
 	 */
-	if (cpu > 0)
+	if (cpu_timers_requested++ > 0)
 		goto out;
 
 	err = rthal_irq_request(RTHAL_APIC_TIMER_IPI,

@@ -252,18 +254,20 @@ void rthal_timer_release(int cpu)
 {
 #ifdef CONFIG_GENERIC_CLOCKEVENTS
 	ipipe_release_tickdev(cpu);
-#else
-	rthal_irq_host_release(RTHAL_BCAST_TICK_IRQ,
-			       &rthal_broadcast_to_local_timers);
 #endif
 
 	/*
 	 * The rest of the cleanup work should only be performed once
-	 * by a single CPU.
+	 * by the last releasing CPU.
 	 */
-	if (cpu > 0)
+	if (--cpu_timers_requested > 0)
 		return;
 
+#ifndef CONFIG_GENERIC_CLOCKEVENTS
+	rthal_irq_host_release(RTHAL_BCAST_TICK_IRQ,
+			       &rthal_broadcast_to_local_timers);
+#endif
+
 	rthal_nmi_release();
 
 	rthal_irq_release(RTHAL_APIC_TIMER_IPI);