[rebase of https://mail.gna.org/public/xenomai-core/2006-08/msg00024.html] The decision if a periodic timer should be restarted after completion of its handler is currently based on three conditions: timer->interval != 0, timer dequeued, and timer not killed. This approach puts the decision in the hand of the timer handler via its return code, also aligning xntimer a bit more with hrtimer. The idea is both to optimise the timer irq handler a bit and to help us defining some rtdm_timer abstraction that can be mapped also on native (-rt) Linux. Warning: Beyond review, this patch version needs testing, I only compiled it so far. Jan --- include/nucleus/timer.h | 15 +++++++---- ksrc/drivers/testing/timerbench.c | 11 ++++---- ksrc/nucleus/pod.c | 13 ++++------ ksrc/nucleus/thread.c | 9 +++++-- ksrc/nucleus/timer.c | 48 ++++++++++++++++++++++++++++---------- ksrc/skins/native/alarm.c | 6 +++- ksrc/skins/posix/timer.c | 5 +++ ksrc/skins/psos+/tm.c | 4 ++- ksrc/skins/vxworks/wdLib.c | 4 ++- 9 files changed, 80 insertions(+), 35 deletions(-) Index: xenomai/include/nucleus/timer.h =================================================================== --- xenomai.orig/include/nucleus/timer.h +++ xenomai/include/nucleus/timer.h @@ -50,6 +50,9 @@ #define XNTIMER_KEEPER_ID 0 +#define XNTIMER_DONE 0 +#define XNTIMER_FORWARD 1 + typedef struct { xnholder_t link; xnticks_t key; @@ -238,7 +241,7 @@ typedef struct xntimer { struct xnsched *sched; /* !< Sched structure to which the timer is attached. */ - void (*handler)(struct xntimer *timer); /* !< Timeout handler. */ + int (*handler)(struct xntimer *timer); /* !< Timeout handler. */ XNARCH_DECL_DISPLAY_CONTEXT(); @@ -300,7 +303,7 @@ extern xntbops_t nktimer_ops_aperiodic, void xntimer_init(xntimer_t *timer, xntbase_t *base, - void (*handler)(xntimer_t *timer)); + int (*handler)(xntimer_t *timer)); void xntimer_destroy(xntimer_t *timer); @@ -325,8 +328,10 @@ void xntimer_destroy(xntimer_t *timer); * * @param interval The reload value of the timer. It is a periodic * interval value to be used for reprogramming the next timer shot, - * expressed in clock ticks (see note). If @a interval is equal to - * XN_INFINITE, the timer will not be reloaded after it has expired. + * expressed in clock ticks (see note). The associated timer handler must + * return XNTIMER_FORWARD to trigger the reloading after the timer has + * expired. If @a interval is equal to XN_INFINITE or the timer handler + * returns XNTIMER_DONE, the timer will not be reloaded. * * @param mode The timer mode. It can be either XN_RELATIVE or * XN_ABSOLUTE to define if @a value shall be interpreted as a @@ -595,7 +600,7 @@ void xntimer_freeze(void); void xntimer_tick_aperiodic(void); -void xntimer_tick_periodic(xntimer_t *timer); +int xntimer_tick_periodic(xntimer_t *timer); #ifdef CONFIG_SMP int xntimer_migrate(xntimer_t *timer, Index: xenomai/ksrc/drivers/testing/timerbench.c =================================================================== --- xenomai.orig/ksrc/drivers/testing/timerbench.c +++ xenomai/ksrc/drivers/testing/timerbench.c @@ -165,7 +165,7 @@ void timer_task_proc(void *arg) } -void timer_proc(xntimer_t *timer) +int timer_proc(xntimer_t *timer) { struct rt_tmbench_context *ctx = container_of(timer, struct rt_tmbench_context, timer); @@ -178,11 +178,12 @@ void timer_proc(xntimer_t *timer) xntimer_start(&ctx->timer, xntbase_ns2ticks(rtdm_tbase, ctx->date), XN_INFINITE, XN_ABSOLUTE); - if (++ctx->curr.test_loops < ctx->samples_per_sec) - return; + if (++ctx->curr.test_loops >= ctx->samples_per_sec) { + ctx->curr.test_loops = 0; + eval_outer_loop(ctx); + } - ctx->curr.test_loops = 0; - eval_outer_loop(ctx); + return XNTIMER_DONE; } Index: xenomai/ksrc/nucleus/thread.c =================================================================== --- xenomai.orig/ksrc/nucleus/thread.c +++ xenomai/ksrc/nucleus/thread.c @@ -24,14 +24,17 @@ #include #include -static void xnthread_timeout_handler(xntimer_t *timer) +static int xnthread_timeout_handler(xntimer_t *timer) { xnthread_t *thread = container_of(timer, xnthread_t, rtimer); + xnthread_set_info(thread, XNTIMEO); /* Interrupts are off. */ xnpod_resume_thread(thread, XNDELAY); + + return XNTIMER_DONE; } -static void xnthread_periodic_handler(xntimer_t *timer) +static int xnthread_periodic_handler(xntimer_t *timer) { xnthread_t *thread = container_of(timer, xnthread_t, ptimer); @@ -39,6 +42,8 @@ static void xnthread_periodic_handler(xn blocked on a resource. */ if (xnthread_test_state(thread, XNDELAY|XNPEND) == XNDELAY) xnpod_resume_thread(thread, XNDELAY); + + return XNTIMER_FORWARD; } int xnthread_init(xnthread_t *thread, Index: xenomai/ksrc/nucleus/timer.c =================================================================== --- xenomai.orig/ksrc/nucleus/timer.c +++ xenomai/ksrc/nucleus/timer.c @@ -197,19 +197,28 @@ void xntimer_tick_aperiodic(void) if (timer != &nkpod->htimer) { if (!testbits(nktbase.status, XNTBLCK)) { - timer->handler(timer); + if (timer->handler(timer) == XNTIMER_DONE) + continue; now = xnarch_get_cpu_tsc(); +#if defined(CONFIG_XENO_OPT_DEBUG_NUCLEUS) || defined(__XENO_SIM__) if (timer->interval == XN_INFINITE || !testbits(timer->status, XNTIMER_DEQUEUED) - || testbits(timer->status, XNTIMER_KILLED)) + || testbits(timer->status, XNTIMER_KILLED)) { /* The elapsed timer has no reload value, or has been re-enqueued likely as a result of a call to xntimer_start() from the timeout handler, or has been killed by the handler. In all cases, - don't attempt to re-enqueue it for the next - shot. */ - continue; + returning XNTIMER_FORWARD is wrong. */ + xnlogerr("illegal timer restart detected -- " + "status=%08lx, interval=%lld\n", + timer->status, timer->interval); +#ifdef __KERNEL__ + show_stack(NULL, NULL); +#endif + continue; + } +#endif /* CONFIG_XENO_OPT_DEBUG_NUCLEUS || __XENO_SIM__ */ } else if (timer->interval == XN_INFINITE) { xntimerh_date(&timer->aplink) += nkpod->htimer.interval; @@ -315,7 +324,7 @@ static void xntimer_move_periodic(xntime /*! * @internal - * \fn void xntimer_tick_periodic(xntimer_t *mtimer) + * \fn int xntimer_tick_periodic(xntimer_t *mtimer) * * \brief Process a timer tick for a slave periodic time base. * @@ -344,7 +353,7 @@ static void xntimer_move_periodic(xntime * @note Only active timers are inserted into the timer wheel. */ -void xntimer_tick_periodic(xntimer_t *mtimer) +int xntimer_tick_periodic(xntimer_t *mtimer) { xntslave_t *slave = timer2slave(mtimer); xnsched_t *sched = xnpod_current_sched(); @@ -369,12 +378,22 @@ void xntimer_tick_periodic(xntimer_t *mt xntimer_dequeue_periodic(timer); - timer->handler(timer); + if (timer->handler(timer) == XNTIMER_DONE) + continue; +#if defined(CONFIG_XENO_OPT_DEBUG_NUCLEUS) || defined(__XENO_SIM__) if (timer->interval == XN_INFINITE || !testbits(timer->status, XNTIMER_DEQUEUED) - || testbits(timer->status, XNTIMER_KILLED)) + || testbits(timer->status, XNTIMER_KILLED)) { + xnlogerr("illegal timer restart detected -- " + "status=%08lx, interval=%lld\n", + timer->status, timer->interval); +#ifdef __KERNEL__ + show_stack(NULL, NULL); +#endif continue; + } +#endif /* CONFIG_XENO_OPT_DEBUG_NUCLEUS || __XENO_SIM__ */ xntlholder_date(&timer->plink) = base->jiffies + timer->interval; xntimer_enqueue_periodic(timer); @@ -384,6 +403,8 @@ void xntimer_tick_periodic(xntimer_t *mt if (base->hook) base->hook(); + + return XNTIMER_FORWARD; } void xntslave_init(xntslave_t *slave) @@ -484,7 +505,7 @@ xntbops_t nktimer_ops_periodic = { #endif /* CONFIG_XENO_OPT_TIMING_PERIODIC */ /*! - * \fn void xntimer_init(xntimer_t *timer,xntbase_t *base,void (*handler)(xntimer_t *timer)) + * \fn void xntimer_init(xntimer_t *timer,xntbase_t *base,int (*handler)(xntimer_t *timer)) * \brief Initialize a timer object. * * Creates a timer. When created, a timer is left disarmed; it must be @@ -499,7 +520,9 @@ xntbops_t nktimer_ops_periodic = { * depends on. See xntbase_alloc() for detailed explanations about * time bases. * - * @param handler The routine to call upon expiration of the timer. + * @param handler The routine to call upon expiration of the timer. The return + * value of this routine must be either XNTIMER_DONE or XNTIMER_FORWARD, while + * the latter is only valid for periodic timers. See also xntimer_start(). * * There is no limitation on the number of timers which can be * created/active concurrently. @@ -516,7 +539,8 @@ xntbops_t nktimer_ops_periodic = { * Rescheduling: never. */ -void xntimer_init(xntimer_t *timer, xntbase_t *base, void (*handler) (xntimer_t *timer)) +void xntimer_init(xntimer_t *timer, xntbase_t *base, + int (*handler) (xntimer_t *timer)) { /* CAUTION: Setup from xntimer_init() must not depend on the periodic/aperiodic timing mode. */ Index: xenomai/ksrc/skins/native/alarm.c =================================================================== --- xenomai.orig/ksrc/skins/native/alarm.c +++ xenomai/ksrc/skins/native/alarm.c @@ -121,11 +121,15 @@ void __native_alarm_pkg_cleanup(void) __native_alarm_flush_rq(&__native_global_rholder.alarmq); } -static void __alarm_trampoline(xntimer_t *timer) +static int __alarm_trampoline(xntimer_t *timer) { RT_ALARM *alarm = container_of(timer, RT_ALARM, timer_base); + ++alarm->expiries; alarm->handler(alarm, alarm->cookie); + + return xntimer_running_p(&alarm->timer_base) ? + XNTIMER_FORWARD : XNTIMER_DONE; } /** Index: xenomai/ksrc/skins/posix/timer.c =================================================================== --- xenomai.orig/ksrc/skins/posix/timer.c +++ xenomai/ksrc/skins/posix/timer.c @@ -52,7 +52,7 @@ static xnqueue_t timer_freeq; static struct pse51_timer timer_pool[PSE51_TIMER_MAX]; -static void pse51_base_timer_handler(xntimer_t *xntimer) +static int pse51_base_timer_handler(xntimer_t *xntimer) { struct pse51_timer *timer = container_of(xntimer, struct pse51_timer, timerbase); @@ -65,6 +65,9 @@ static void pse51_base_timer_handler(xnt timer->overruns = 0; pse51_sigqueue_inner(timer->owner, &timer->si); } + + return (xntimer_interval(&timer->timerbase) == XN_INFINITE) ? + XNTIMER_DONE : XNTIMER_FORWARD; } /* Must be called with nklock locked, irq off. */ Index: xenomai/ksrc/skins/psos+/tm.c =================================================================== --- xenomai.orig/ksrc/skins/psos+/tm.c +++ xenomai/ksrc/skins/psos+/tm.c @@ -60,7 +60,7 @@ void tm_destroy_internal(psostm_t *tm) xnfree(tm); } -static void tm_evpost_handler(xntimer_t *timer) +static int tm_evpost_handler(xntimer_t *timer) { psostm_t *tm = container_of(timer, psostm_t, timerbase); @@ -68,6 +68,8 @@ static void tm_evpost_handler(xntimer_t if (xntimer_interval(&tm->timerbase) == XN_INFINITE) tm_destroy_internal(tm); + + return XNTIMER_DONE; } static u_long tm_start_event_timer(u_long ticks, Index: xenomai/ksrc/skins/vxworks/wdLib.c =================================================================== --- xenomai.orig/ksrc/skins/vxworks/wdLib.c +++ xenomai/ksrc/skins/vxworks/wdLib.c @@ -90,11 +90,13 @@ static xnpnode_t wd_pnode = { #endif /* CONFIG_XENO_EXPORT_REGISTRY */ -static void wind_wd_trampoline(xntimer_t *timer) +static int wind_wd_trampoline(xntimer_t *timer) { wind_wd_t *wd = container_of(timer, wind_wd_t, timerbase); wd->handler(wd->arg); + + return XNTIMER_DONE; } void wind_wd_init(void) Index: xenomai/ksrc/nucleus/pod.c =================================================================== --- xenomai.orig/ksrc/nucleus/pod.c +++ xenomai/ksrc/nucleus/pod.c @@ -143,7 +143,7 @@ const char *xnpod_fatal_helper(const cha /*! * @internal - * \fn void xnpod_watchdog_handler(xntimer_t *timer) + * \fn int xnpod_watchdog_handler(xntimer_t *timer) * \brief Process watchdog ticks. * * This internal routine handles incoming watchdog ticks to detect @@ -152,23 +152,22 @@ const char *xnpod_fatal_helper(const cha * four seconds. */ -void xnpod_watchdog_handler(xntimer_t *timer) +int xnpod_watchdog_handler(xntimer_t *timer) { xnsched_t *sched = xnpod_current_sched(); xnthread_t *thread = sched->runthread; - if (likely(xnthread_test_state(thread, XNROOT))) { + if (likely(xnthread_test_state(thread, XNROOT))) xnpod_reset_watchdog(sched); - return; - } - - if (unlikely(++sched->wdcount >= 4)) { + else if (unlikely(++sched->wdcount >= 4)) { xnltt_log_event(xeno_ev_watchdog, thread->name); xnprintf("watchdog triggered -- killing runaway thread '%s'\n", thread->name); xnpod_delete_thread(thread); xnpod_reset_watchdog(sched); } + + return XNTIMER_FORWARD; } #endif /* CONFIG_XENO_OPT_WATCHDOG */