[Xenomai-core] [PATCH 3/2] kill XN_INFINITE xntimer delays

[Xenomai-core] [PATCH 3/2] kill XN_INFINITE xntimer delays

[Jan Kiszka]

[-- Attachment #1.1: Type: text/plain, Size: 320 bytes --]

[Appendix to the xntimer cleanup series.]

Having to take care for infinite delays in xntimer_start (i.e. to NOT
start the timer...) is a bit suboptimal given that his service might be
used heavily to restart timers. This patch converts the only user
(xnpod_start_timer again) and cleans up the timer code.

Jan

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: xntimer-no-inf-timeout.patch --]
[-- Type: text/x-patch; name="xntimer-no-inf-timeout.patch", Size: 2708 bytes --]

---
 ksrc/nucleus/pod.c   |   10 ++++++----
 ksrc/nucleus/timer.c |   34 ++++++++++++----------------------
 2 files changed, 18 insertions(+), 26 deletions(-)

Index: xenomai/ksrc/nucleus/pod.c
===================================================================
--- xenomai.orig/ksrc/nucleus/pod.c
+++ xenomai/ksrc/nucleus/pod.c
@@ -3095,10 +3095,12 @@ int xnpod_start_timer(u_long nstick, xni
 
 	xntimer_set_sched(&nkpod->htimer, xnpod_sched_slot(XNTIMER_KEEPER_ID));
 
-	xnlock_get_irqsave(&nklock, s);
-	xntimer_start(&nkpod->htimer, delta,
-		      XNARCH_HOST_TICK / nkpod->tickvalue);
-	xnlock_put_irqrestore(&nklock, s);
+	if (delta) {
+		xnlock_get_irqsave(&nklock, s);
+		xntimer_start(&nkpod->htimer, delta,
+			      XNARCH_HOST_TICK / nkpod->tickvalue);
+		xnlock_put_irqrestore(&nklock, s);
+	}
 
 	return 0;
 }
Index: xenomai/ksrc/nucleus/timer.c
===================================================================
--- xenomai.orig/ksrc/nucleus/timer.c
+++ xenomai/ksrc/nucleus/timer.c
@@ -99,20 +99,15 @@ static void xntimer_do_start_aperiodic(x
 	if (!testbits(timer->status, XNTIMER_DEQUEUED))
 		xntimer_dequeue_aperiodic(timer);
 
-	if (value != XN_INFINITE) {
-		xntimerh_date(&timer->aplink) =
-		    xnarch_get_cpu_tsc() + xnarch_ns_to_tsc(value);
-		timer->interval = xnarch_ns_to_tsc(interval);
-		xntimer_enqueue_aperiodic(timer);
-		if (xntimer_heading_p(timer)) {
-			if (xntimer_sched(timer) != xnpod_current_sched())
-				xntimer_next_remote_shot(xntimer_sched(timer));
-			else
-				xntimer_next_local_shot(xntimer_sched(timer));
-		}
-	} else {
-		xntimerh_date(&timer->aplink) = XN_INFINITE;
-		timer->interval = XN_INFINITE;
+	xntimerh_date(&timer->aplink) =
+	    xnarch_get_cpu_tsc() + xnarch_ns_to_tsc(value);
+	timer->interval = xnarch_ns_to_tsc(interval);
+	xntimer_enqueue_aperiodic(timer);
+	if (xntimer_heading_p(timer)) {
+		if (xntimer_sched(timer) != xnpod_current_sched())
+			xntimer_next_remote_shot(xntimer_sched(timer));
+		else
+			xntimer_next_local_shot(xntimer_sched(timer));
 	}
 }
 
@@ -298,14 +293,9 @@ static void xntimer_do_start_periodic(xn
 	if (!testbits(timer->status, XNTIMER_DEQUEUED))
 		xntimer_dequeue_periodic(timer);
 
-	if (value != XN_INFINITE) {
-		xntlholder_date(&timer->plink) = nkpod->jiffies + value;
-		timer->interval = interval;
-		xntimer_enqueue_periodic(timer);
-	} else {
-		xntlholder_date(&timer->plink) = XN_INFINITE;
-		timer->interval = XN_INFINITE;
-	}
+	xntlholder_date(&timer->plink) = nkpod->jiffies + value;
+	timer->interval = interval;
+	xntimer_enqueue_periodic(timer);
 }
 
 static void xntimer_do_stop_periodic(xntimer_t *timer)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]

Re: [Xenomai-core] [PATCH 3/2] kill XN_INFINITE xntimer delays

[Gilles Chanteperdrix]

Jan Kiszka wrote:
 > [Appendix to the xntimer cleanup series.]
 > 
 > Having to take care for infinite delays in xntimer_start (i.e. to NOT
 > start the timer...) is a bit suboptimal given that his service might be
 > used heavily to restart timers. This patch converts the only user
 > (xnpod_start_timer again) and cleans up the timer code.
 > 
 > Jan
 > ---
 >  ksrc/nucleus/pod.c   |   10 ++++++----
 >  ksrc/nucleus/timer.c |   34 ++++++++++++----------------------
 >  2 files changed, 18 insertions(+), 26 deletions(-)
 > 
 > Index: xenomai/ksrc/nucleus/pod.c
 > ===================================================================
 > --- xenomai.orig/ksrc/nucleus/pod.c
 > +++ xenomai/ksrc/nucleus/pod.c
 > @@ -3095,10 +3095,12 @@ int xnpod_start_timer(u_long nstick, xni
 >  
 >  	xntimer_set_sched(&nkpod->htimer, xnpod_sched_slot(XNTIMER_KEEPER_ID));
 >  
 > -	xnlock_get_irqsave(&nklock, s);
 > -	xntimer_start(&nkpod->htimer, delta,
 > -		      XNARCH_HOST_TICK / nkpod->tickvalue);
 > -	xnlock_put_irqrestore(&nklock, s);
 > +	if (delta) {

Should not this rather be if (XNARCH_HOST_TICK) ?

 > +		xnlock_get_irqsave(&nklock, s);
 > +		xntimer_start(&nkpod->htimer, delta,
 > +			      XNARCH_HOST_TICK / nkpod->tickvalue);
 > +		xnlock_put_irqrestore(&nklock, s);
 > +	}

-- 


					    Gilles Chanteperdrix.

Re: [Xenomai-core] [PATCH 3/2] kill XN_INFINITE xntimer delays

[Philippe Gerum]

On Wed, 2006-07-19 at 12:20 +0200, Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>  > [Appendix to the xntimer cleanup series.]
>  > 
>  > Having to take care for infinite delays in xntimer_start (i.e. to NOT
>  > start the timer...) is a bit suboptimal given that his service might be
>  > used heavily to restart timers. This patch converts the only user
>  > (xnpod_start_timer again) and cleans up the timer code.
>  > 
>  > Jan
>  > ---
>  >  ksrc/nucleus/pod.c   |   10 ++++++----
>  >  ksrc/nucleus/timer.c |   34 ++++++++++++----------------------
>  >  2 files changed, 18 insertions(+), 26 deletions(-)
>  > 
>  > Index: xenomai/ksrc/nucleus/pod.c
>  > ===================================================================
>  > --- xenomai.orig/ksrc/nucleus/pod.c
>  > +++ xenomai/ksrc/nucleus/pod.c
>  > @@ -3095,10 +3095,12 @@ int xnpod_start_timer(u_long nstick, xni
>  >  
>  >  	xntimer_set_sched(&nkpod->htimer, xnpod_sched_slot(XNTIMER_KEEPER_ID));
>  >  
>  > -	xnlock_get_irqsave(&nklock, s);
>  > -	xntimer_start(&nkpod->htimer, delta,
>  > -		      XNARCH_HOST_TICK / nkpod->tickvalue);
>  > -	xnlock_put_irqrestore(&nklock, s);
>  > +	if (delta) {
> 
> Should not this rather be if (XNARCH_HOST_TICK) ?

Both work, even if using the macro would get the unused code optimized
away. The bottom line is that we need to refrain from starting the timer
whenever the delta value is zero, since it's a clear sign that
XNARCH_HOST_TICK is zero in the first place. In all other cases, the
delta value must be non-zero, even if the elapsed portion of the current
tick is null.

> 
>  > +		xnlock_get_irqsave(&nklock, s);
>  > +		xntimer_start(&nkpod->htimer, delta,
>  > +			      XNARCH_HOST_TICK / nkpod->tickvalue);
>  > +		xnlock_put_irqrestore(&nklock, s);
>  > +	}
> 
-- 
Philippe.

Re: [Xenomai-core] [PATCH 3/2] kill XN_INFINITE xntimer delays

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2108 bytes --]

Philippe Gerum wrote:
> On Wed, 2006-07-19 at 12:20 +0200, Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>  > [Appendix to the xntimer cleanup series.]
>>  > 
>>  > Having to take care for infinite delays in xntimer_start (i.e. to NOT
>>  > start the timer...) is a bit suboptimal given that his service might be
>>  > used heavily to restart timers. This patch converts the only user
>>  > (xnpod_start_timer again) and cleans up the timer code.
>>  > 
>>  > Jan
>>  > ---
>>  >  ksrc/nucleus/pod.c   |   10 ++++++----
>>  >  ksrc/nucleus/timer.c |   34 ++++++++++++----------------------
>>  >  2 files changed, 18 insertions(+), 26 deletions(-)
>>  > 
>>  > Index: xenomai/ksrc/nucleus/pod.c
>>  > ===================================================================
>>  > --- xenomai.orig/ksrc/nucleus/pod.c
>>  > +++ xenomai/ksrc/nucleus/pod.c
>>  > @@ -3095,10 +3095,12 @@ int xnpod_start_timer(u_long nstick, xni
>>  >  
>>  >  	xntimer_set_sched(&nkpod->htimer, xnpod_sched_slot(XNTIMER_KEEPER_ID));
>>  >  
>>  > -	xnlock_get_irqsave(&nklock, s);
>>  > -	xntimer_start(&nkpod->htimer, delta,
>>  > -		      XNARCH_HOST_TICK / nkpod->tickvalue);
>>  > -	xnlock_put_irqrestore(&nklock, s);
>>  > +	if (delta) {
>>
>> Should not this rather be if (XNARCH_HOST_TICK) ?
> 
> Both work, even if using the macro would get the unused code optimized
> away. The bottom line is that we need to refrain from starting the timer

Optimising away sounds good to me :). As we are already on it, I would
second Gilles' suggestion. It also makes the code more readable ("we do
not need the following when XNARCH_HOST_TICK is 0").

> whenever the delta value is zero, since it's a clear sign that
> XNARCH_HOST_TICK is zero in the first place. In all other cases, the
> delta value must be non-zero, even if the elapsed portion of the current
> tick is null.
> 
>>  > +		xnlock_get_irqsave(&nklock, s);
>>  > +		xntimer_start(&nkpod->htimer, delta,
>>  > +			      XNARCH_HOST_TICK / nkpod->tickvalue);
>>  > +		xnlock_put_irqrestore(&nklock, s);
>>  > +	}
>>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]