Skip to contentSkip to conversation

nucleus: Prevent rescheduling while in xntbase_tick

RFC PATCH In review Jan Kiszka 3 participants 6 messages Jun 17, 2011

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

RFC PATCH 1/1 nucleus: Prevent rescheduling while in xntbase_tick

Based on code inspection, it looks like a timer handler triggering a reschedule in the path xntbase_tick -> xntimer_tick_aperiodic / xntimer_tick_periodic_inner -> handler can cause problems, e.g. a reschedule before all expired timers were processed. The timer core is usually run atomically from an interrupt handler, so better emulate an IRQ context inside xntbase_tick by setting XNINIRQ.

This patch only affects users of xntbase_tick, ie. psos, vrtx and the vxworks skin.

Signed-off-by Jan Kiszka <[email protected]>

---

If my analysis is correct, this is also 2.5 material.

 ksrc/nucleus/timebase.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)
ksrc/nucleus/timebase.c +9 -0
index 6f7c02f..8f9af04 100644
@@ -477,12 +477,16 @@ EXPORT_SYMBOL_GPL(xntbase_stop);
 
 void xntbase_tick(xntbase_t *base)
 {
+	struct xnsched_t *sched;
 	spl_t s;
 
 	xnlock_get_irqsave(&nklock, s);
 
 	trace_mark(xn_nucleus, tbase_tick, "base %s", base->name);
 
+	++sched->inesting;
+	__setbits(sched->lflags, XNINIRQ);
+
 	if (base == &nktbase)
 		xntimer_tick_aperiodic();
 	else {

@@ -490,6 +494,11 @@ void xntbase_tick(xntbase_t *base)
 		xntimer_tick_periodic_inner(slave);
 	}
 
+	if (--sched->inesting == 0) {
+		__clrbits(sched->lflags, XNINIRQ);
+		xnpod_schedule();
+	}
+
 	xnlock_put_irqrestore(&nklock, s);
 }
 EXPORT_SYMBOL_GPL(xntbase_tick);
Signature · 1 line
1.7.1
#1 lore raw
Quoted 6 lines · Jan Kiszka

Based on code inspection, it looks like a timer handler triggering a reschedule in the path xntbase_tick -> xntimer_tick_aperiodic / xntimer_tick_periodic_inner -> handler can cause problems, e.g. a reschedule before all expired timers were processed. The timer core is usually run atomically from an interrupt handler, so better emulate an IRQ context inside xntbase_tick by setting XNINIRQ.

I do not understand this one either: if we are inside xntimer_tick_aperiodic, XNINIRQ is already set.

Signature · 1 line
                                                                Gilles.
author
#2 lore raw
Quoted 10 lines · Gilles Chanteperdrix
Quoted 6 lines · Jan Kiszka

Based on code inspection, it looks like a timer handler triggering a reschedule in the path xntbase_tick -> xntimer_tick_aperiodic / xntimer_tick_periodic_inner -> handler can cause problems, e.g. a reschedule before all expired timers were processed. The timer core is usually run atomically from an interrupt handler, so better emulate an IRQ context inside xntbase_tick by setting XNINIRQ.

I do not understand this one either: if we are inside xntimer_tick_aperiodic, XNINIRQ is already set.

Not if you come via xntbase_tick, called by the mentioned skins also outside a timer IRQ (at least based on my understanding of that skin APIs). But I might be wrong, I just came across this while checking for potentially invalid cached xnpod_current_sched values.

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
#3 lore raw
Quoted 9 lines · Jan Kiszka
Quoted 8 lines · Gilles Chanteperdrix
Quoted 6 lines · Jan Kiszka

Based on code inspection, it looks like a timer handler triggering a reschedule in the path xntbase_tick -> xntimer_tick_aperiodic / xntimer_tick_periodic_inner -> handler can cause problems, e.g. a reschedule before all expired timers were processed. The timer core is usually run atomically from an interrupt handler, so better emulate an IRQ context inside xntbase_tick by setting XNINIRQ.

Quoted 7 lines
Quoted 2 lines

I do not understand this one either: if we are inside xntimer_tick_aperiodic, XNINIRQ is already set.

Not if you come via xntbase_tick, called by the mentioned skins also outside a timer IRQ (at least based on my understanding of that skin APIs). But I might be wrong, I just came across this while checking for potentially invalid cached xnpod_current_sched values.

That is ok, ui_timer(), tickAnnounce() and tm_tick() are designed by the respective RTOS to be called from IRQ context.

Quoted 3 lines

Jan

Signature · 1 line
Philippe.
author
#4 lore raw
Quoted 13 lines · Philippe Gerum
Quoted 12 lines · Jan Kiszka
Quoted 10 lines · Gilles Chanteperdrix
Quoted 6 lines · Jan Kiszka

Based on code inspection, it looks like a timer handler triggering a reschedule in the path xntbase_tick -> xntimer_tick_aperiodic / xntimer_tick_periodic_inner -> handler can cause problems, e.g. a reschedule before all expired timers were processed. The timer core is usually run atomically from an interrupt handler, so better emulate an IRQ context inside xntbase_tick by setting XNINIRQ.

I do not understand this one either: if we are inside xntimer_tick_aperiodic, XNINIRQ is already set.

Quoted 7 lines
Quoted 4 lines

Not if you come via xntbase_tick, called by the mentioned skins also outside a timer IRQ (at least based on my understanding of that skin APIs). But I might be wrong, I just came across this while checking for potentially invalid cached xnpod_current_sched values.

That is ok, ui_timer(), tickAnnounce() and tm_tick() are designed by the respective RTOS to be called from IRQ context.

Fine. Should we add a XENO_ASSERT to set this in stone and for documentation purposes?

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
#5 lore raw
Quoted 19 lines · Jan Kiszka
Quoted 18 lines · Philippe Gerum
Quoted 16 lines · Jan Kiszka
Quoted 10 lines · Gilles Chanteperdrix
Quoted 6 lines · Jan Kiszka

Based on code inspection, it looks like a timer handler triggering a reschedule in the path xntbase_tick -> xntimer_tick_aperiodic / xntimer_tick_periodic_inner -> handler can cause problems, e.g. a reschedule before all expired timers were processed. The timer core is usually run atomically from an interrupt handler, so better emulate an IRQ context inside xntbase_tick by setting XNINIRQ.

I do not understand this one either: if we are inside xntimer_tick_aperiodic, XNINIRQ is already set.

Not if you come via xntbase_tick, called by the mentioned skins also outside a timer IRQ (at least based on my understanding of that skin APIs). But I might be wrong, I just came across this while checking for potentially invalid cached xnpod_current_sched values.

Quoted 5 lines
Quoted 2 lines

That is ok, ui_timer(), tickAnnounce() and tm_tick() are designed by the respective RTOS to be called from IRQ context.

Fine. Should we add a XENO_ASSERT to set this in stone and for documentation purposes?

I think so.

Quoted 3 lines

Jan

Signature · 1 line
Philippe.