Re: [PATCH RT] make hrtimer_nanosleep return immediately if time has passed

[Steven Rostedt <rostedt@goodmis.org>]

On Fri, 2006-01-06 at 14:27 -0800, George Anzinger wrote:
> Steven Rostedt wrote:
> > Thomas,
> ~
> 
> > usecs as was shown in the jitter test.
> > 
> > My patch does the following:
> > 
> > - Changes enqueue_hrtimer from void to int and returns 1 and does 
> >   nothing else in the case of the timer has passed.
> > 
> > - Change hrtimer_start to return 1 if the timer has passed and not when
> >   the timer was active.  I searched the kernel and I could not find one
> >   instance where this hrtimer_start had its return code checked.
> > 
> > - Changed schedule_hrtimer to not go to sleep if the time has passed.
> 
> At this time the posix timer code depends on the call back.  Either you will 
> need to make this an option or change that code to do the right thing.

George,

Thanks for showing me this.  How about the following patch. It leaves
hrtimer_restart queuing the timer by adding an internal function
__hrtimer_restart that adds the option "queue".  Since the only time you
don't want to queue it (that I know of) is internally to nanosleep. But
then again maybe others will want too.  But anyway, this keeps the
current API to hrtimers unchanged.

-- Steve

Index: linux-2.6.15-rt2/kernel/hrtimer.c
===================================================================
--- linux-2.6.15-rt2.orig/kernel/hrtimer.c	2006-01-06 17:49:47.000000000 -0500
+++ linux-2.6.15-rt2/kernel/hrtimer.c	2006-01-06 17:53:25.000000000 -0500
@@ -226,6 +226,10 @@
  * for which the hrt time source was armed.
  *
  * Called with interrupts disabled and base lock held
+ *
+ * Returns:
+ *  0 on success
+ *  1 if time has already past.
  */
 static int hrtimer_reprogram(struct hrtimer *timer, struct hrtimer_base *base)
 {
@@ -239,6 +243,8 @@
 	res = clockevents_set_next_event(expires);
 	if (!res)
 		*expires_next = expires;
+	else
+		res = 1;
 	return res;
 }
 
@@ -381,11 +387,24 @@
 	       smp_processor_id());
 }
 
+/*
+ * kick_off_hrtimer - queue the timer to the expire list and
+ *                    raise the hrtimer softirq.
+ */
+static void
+kick_off_hrtimer(struct hrtimer *timer, struct hrtimer_base *base)
+{
+	list_add_tail(&timer->list, &base->expired);
+	timer->state = HRTIMER_PENDING_CALLBACK;
+	raise_softirq(HRTIMER_SOFTIRQ);
+}
+
 #else /* CONFIG_HIGH_RES_TIMERS */
 
 # define hrtimer_hres_active		0
 # define hres_enqueue_expired(t,b,n)	0
 # define hrtimer_check_clocks()		do { } while (0)
+# define kick_off_hrtimer		do { } while (0)
 
 #endif /* !CONFIG_HIGH_RES_TIMERS */
 
@@ -501,9 +520,14 @@
  *
  * The timer is inserted in expiry order. Insertion into the
  * red black tree is O(log(n)). Must hold the base lock.
+ *
+ * Returns:
+ *  0 on success
+ *  1 if time has already past.
  */
-static void enqueue_hrtimer(struct hrtimer *timer, struct hrtimer_base *base)
+static int enqueue_hrtimer(struct hrtimer *timer, struct hrtimer_base *base)
 {
+
 	struct rb_node **link = &base->active.rb_node;
 	struct rb_node *parent = NULL;
 	struct hrtimer *entry;
@@ -534,12 +558,8 @@
 		 * past we just move it to the expired list
 		 * and schedule the softirq.
 		 */
-		if (hrtimer_hres_active && hrtimer_reprogram(timer, base)) {
-			list_add_tail(&timer->list, &base->expired);
-			timer->state = HRTIMER_PENDING_CALLBACK;
-			raise_softirq(HRTIMER_SOFTIRQ);
-			return;
-		}
+		if (hrtimer_hres_active && hrtimer_reprogram(timer, base))
+			return 1;
 #endif
 		base->first = &timer->node;
 	}
@@ -551,6 +571,7 @@
 	rb_insert_color(&timer->node, &base->active);
 
 	timer->state = HRTIMER_PENDING;
+	return 0;
 }
 
 /*
@@ -598,10 +619,11 @@
  *
  * Returns:
  *  0 on success
- *  1 when the timer was active
+ *  1 if the time has already past
  */
-int
-hrtimer_start(struct hrtimer *timer, ktime_t tim, const enum hrtimer_mode mode)
+static int
+__hrtimer_start(struct hrtimer *timer, ktime_t tim, const enum hrtimer_mode mode,
+		int queue)
 {
 	struct hrtimer_base *base, *new_base;
 	unsigned long flags;
@@ -610,7 +632,7 @@
 	base = lock_hrtimer_base(timer, &flags);
 
 	/* Remove an active timer from the queue: */
-	ret = remove_hrtimer(timer, base);
+	remove_hrtimer(timer, base);
 
 	/* Switch the timer base, if necessary: */
 	new_base = switch_hrtimer_base(timer, base);
@@ -619,13 +641,21 @@
 		tim = ktime_add(tim, new_base->get_time());
 	timer->expires = tim;
 
-	enqueue_hrtimer(timer, new_base);
+	if ((ret = enqueue_hrtimer(timer, new_base)) && queue)
+		kick_off_hrtimer(timer, new_base);
 
 	unlock_hrtimer_base(timer, &flags);
 
 	return ret;
 }
 
+int
+hrtimer_start(struct hrtimer *timer, ktime_t tim, const enum hrtimer_mode mode)
+{
+	return __hrtimer_start(timer, tim, mode, 1);
+}
+
+
 /**
  * hrtimer_try_to_cancel - try to deactivate a timer
  *
@@ -864,9 +894,10 @@
 
 		spin_lock_irq(&base->lock);
 
-		if (restart == HRTIMER_RESTART)
-			enqueue_hrtimer(timer, base);
-		else
+		if (restart == HRTIMER_RESTART) {
+			if (enqueue_hrtimer(timer, base))
+				kick_off_hrtimer(timer, base);
+		} else
 			timer->state = HRTIMER_EXPIRED;
 		set_curr_timer(base, NULL);
 	}
@@ -922,9 +953,10 @@
 
 		spin_lock_irq(&base->lock);
 
-		if (restart == HRTIMER_RESTART)
-			enqueue_hrtimer(timer, base);
-		else
+		if (restart == HRTIMER_RESTART) {
+			if (enqueue_hrtimer(timer, base))
+				kick_off_hrtimer(timer, base);
+		} else
 			timer->state = HRTIMER_EXPIRED;
 		set_curr_timer(base, NULL);
 	}
@@ -983,9 +1015,13 @@
 	/* fn stays NULL, meaning single-shot wakeup: */
 	timer->data = current;
 
-	hrtimer_start(timer, timer->expires, mode);
+	if (__hrtimer_start(timer, timer->expires, mode, 0)) {
+		/* time already past */
+		timer->state = HRTIMER_EXPIRED;
+		set_current_state(TASK_RUNNING);
+	} else
+		schedule();
 
-	schedule();
 	hrtimer_cancel(timer);
 
 	/* Return the remaining time: */
@@ -1128,7 +1164,8 @@
 		timer = rb_entry(node, struct hrtimer, node);
 		__remove_hrtimer(timer, old_base);
 		timer->base = new_base;
-		enqueue_hrtimer(timer, new_base);
+		if (enqueue_hrtimer(timer, new_base))
+			kick_off_hrtimer(timer, base);
 	}
 }
 


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/