linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] hrtimers updates
@ 2006-01-20  2:55 Thomas Gleixner
  2006-01-20  2:55 ` [PATCH 1/7] [hrtimers] Fixup itimer conversion Thomas Gleixner
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Thomas Gleixner @ 2006-01-20  2:55 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: LKML, Ingo Molnar, George Anzinger, Steven Rostedt, Andrew Morton

Linus,

please pull from

master.kernel.org:/pub/scm/linux/kernel/git/tglx/hrtimer-2.6.git

This is an update on following issues:

- itimer locking
- NULL pointer usage
- oldvalue return in setitimer bug#5617
- posix-timer requeue race
- hrtimer cleanups and simplifications
- correct initial value for relative SIGEV_NONE timers

	tglx

diff-tree a1f15939b7af18c5abcd4810ccd512467c77a6b1 (from 4b2719dcb1143a18de16162f2562045e40487e49)
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Fri Jan 20 02:19:44 2006 +0100

    [hrtimers] Set correct initial expiry time for relative SIGEV_NONE timers
    
    The expiry time for relative timers with SIGEV_NONE set was never
    updated to the correct value.
    
    Pointed out by George Anzinger.
    
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

:100644 100644 28e72fd0029fa466e1768d40bcb10b28a2505450 e2fa4c03c2589784a031036916a603e7fe0ac18d M	kernel/posix-timers.c

diff-tree 4b2719dcb1143a18de16162f2562045e40487e49 (from 0ea0b28ad86d611745b0a55b472f46d27c38e7a2)
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Fri Jan 20 01:29:11 2006 +0100

    [hrtimers] Add back lost credit lines
    
    At some point we added credits to people who actively helped
    to bring k/hr-timers along. This was lost in the big code
    revamp. Add it back.
    
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Signed-off-by: Ingo Molnar <mingo@elte.hu>

:100644 100644 1bd6552cc34134c4e19d5565e992fc91b9785910 6aca67a569a2ff907830ef64e6b69dafc154996c M	include/linux/ktime.h
:100644 100644 efff9496b2fae67de84bf2d044a901f8a546ad2d 2b6e1757aeddf118f43c31111f46224b00f5d25e M	kernel/hrtimer.c

diff-tree 0ea0b28ad86d611745b0a55b472f46d27c38e7a2 (from 7a42511f275d3c895be54f4e578921fc35e25dd2)
Author: George Anzinger <george@wildturkeyranch.net>
Date:   Thu Jan 19 23:55:54 2006 +0100

    [hrtimers] Cleanups and simplifications
    
    This patch cleans up the interface to hrtimers by changing the init code
    to pass the mode as well as the clock.  This allow the init code to
    select the correct base and eliminates extra timer re-init code in
    posix-timers.  We also simplify the restart interface nanosleep use.
    
    Signed-off-by: George Anzinger <george@mvista.com>
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

:100644 100644 c657f3d4924a14e5714b818a1788c28859872a12 6361544bb6ae5fef3ee1dcd84dc8788262bd7917 M	include/linux/hrtimer.h
:100644 100644 4ae8cfc1c89cffdee486c2a3b28316e583a40747 7f0ab5ee948c633a8065e685a5147e0df3d439ad M	kernel/fork.c
:100644 100644 f580dd9db2863bee71e16129fce63956b7344b72 efff9496b2fae67de84bf2d044a901f8a546ad2d M	kernel/hrtimer.c
:100644 100644 3b606d361b529dfda6097ba08e60bbdfd3be62aa 28e72fd0029fa466e1768d40bcb10b28a2505450 M	kernel/posix-timers.c

diff-tree 7a42511f275d3c895be54f4e578921fc35e25dd2 (from 3f59dd20898d805781b3eac7ed0807e7a0b30f2f)
Author: Steven Rostedtrostedt@goodmis.org <rostedt@goodmis.org>
Date:   Thu Jan 19 23:52:29 2006 +0100

    [hrtimers] Fix posix-timer requeue race
    
    CPU0 expires a posix-timer and runs the callback function.
    The signal is queued.
    After releasing the posix-timer lock and before returning to
    hrtimer_run_queue CPU0 gets interrupted.
    CPU1 delivers the queued signal and rearms the timer.
    CPU0 comes back to hrtimer_run_queue and sets the timer state to expired.
    The next modification of the timer can result in an oops, because the state
    information is wrong.
    
    Keep track of state = RUNNING and check if the state has been in the return
    path of hrtimer_run_queue. In case the state has been changed, ignore a
    restart request and do not touch the state variable.
    
    Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

:100644 100644 089bfb1fa01a771d7c11ee7de4227deb88d29b00 c657f3d4924a14e5714b818a1788c28859872a12 M	include/linux/hrtimer.h
:100644 100644 f1c4155b49ac140051538f12046cb553674657f9 f580dd9db2863bee71e16129fce63956b7344b72 M	kernel/hrtimer.c

diff-tree 3f59dd20898d805781b3eac7ed0807e7a0b30f2f (from a42e5a139db5c1759bc98729152618ed5b80410e)
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Thu Jan 19 15:36:22 2006 +0100

    [hrtimers] Fix oldvalue return in setitimer
    
    This resolves bugzilla bug#5617. The oldvalue of the
    timer was read after the timer was cancelled, so the
    remaining time was always zero.
    
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

:100644 100644 6433d06855063d477c4c037e00ad660aea4319fd 379be2f8c84c33445b9cea549fe2c7215d3d6cc4 M	kernel/itimer.c

diff-tree a42e5a139db5c1759bc98729152618ed5b80410e (from 52ae41e3d11d6f1828c5827861b7b83b7e854222)
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Tue Jan 17 22:50:51 2006 +0100

    [hrtimers] Fix possible use of NULL pointer in posix-timers
    
    Fixup the conversion of posix-timers to hrtimers.
    
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

:100644 100644 197208b3aa2ad837517d27662e5bacaafd75258b 3b606d361b529dfda6097ba08e60bbdfd3be62aa M	kernel/posix-timers.c

diff-tree 52ae41e3d11d6f1828c5827861b7b83b7e854222 (from 2664b25051f7ab96b22b199aa2f5ef6a949a4296)
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Tue Jan 17 20:03:14 2006 +0100

    [hrtimers] Fixup itimer conversion
    
    The itimer conversion removed the locking which protects
    the timer and variables in the shared signal structure.
    Steven Rostedt found the problem in the latest -rt patches.
    
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

:100644 100644 c2c05c4ff28d5bd7bd32cf8ca1eea1dc768b71c2 6433d06855063d477c4c037e00ad660aea4319fd M	kernel/itimer.c

--


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/7] [hrtimers] Fixup itimer conversion
  2006-01-20  2:55 [PATCH 0/7] hrtimers updates Thomas Gleixner
@ 2006-01-20  2:55 ` Thomas Gleixner
  2006-01-20  2:55 ` [PATCH 2/7] [hrtimers] Fix possible use of NULL pointer in posix-timers Thomas Gleixner
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Thomas Gleixner @ 2006-01-20  2:55 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: LKML, Ingo Molnar, George Anzinger, Steven Rostedt, Andrew Morton

[-- Attachment #1: 0001-hrtimers-Fixup-itimer-conversion.txt --]
[-- Type: text/plain, Size: 1664 bytes --]


The itimer conversion removed the locking which protects
the timer and variables in the shared signal structure.
Steven Rostedt found the problem in the latest -rt patches.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---

 kernel/itimer.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

52ae41e3d11d6f1828c5827861b7b83b7e854222
diff --git a/kernel/itimer.c b/kernel/itimer.c
index c2c05c4..6433d06 100644
--- a/kernel/itimer.c
+++ b/kernel/itimer.c
@@ -49,9 +49,11 @@ int do_getitimer(int which, struct itime
 
 	switch (which) {
 	case ITIMER_REAL:
+		spin_lock_irq(&tsk->sighand->siglock);
 		value->it_value = itimer_get_remtime(&tsk->signal->real_timer);
 		value->it_interval =
 			ktime_to_timeval(tsk->signal->it_real_incr);
+		spin_unlock_irq(&tsk->sighand->siglock);
 		break;
 	case ITIMER_VIRTUAL:
 		read_lock(&tasklist_lock);
@@ -150,8 +152,14 @@ int do_setitimer(int which, struct itime
 
 	switch (which) {
 	case ITIMER_REAL:
+again:
+		spin_lock_irq(&tsk->sighand->siglock);
 		timer = &tsk->signal->real_timer;
-		hrtimer_cancel(timer);
+		/* We are sharing ->siglock with it_real_fn() */
+		if (hrtimer_try_to_cancel(timer) < 0) {
+			spin_unlock_irq(&tsk->sighand->siglock);
+			goto again;
+		}
 		if (ovalue) {
 			ovalue->it_value = itimer_get_remtime(timer);
 			ovalue->it_interval
@@ -162,6 +170,7 @@ int do_setitimer(int which, struct itime
 		expires = timeval_to_ktime(value->it_value);
 		if (expires.tv64 != 0)
 			hrtimer_start(timer, expires, HRTIMER_REL);
+		spin_unlock_irq(&tsk->sighand->siglock);
 		break;
 	case ITIMER_VIRTUAL:
 		nval = timeval_to_cputime(&value->it_value);
-- 
1.0.8

--


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/7] [hrtimers] Fix possible use of NULL pointer in posix-timers
  2006-01-20  2:55 [PATCH 0/7] hrtimers updates Thomas Gleixner
  2006-01-20  2:55 ` [PATCH 1/7] [hrtimers] Fixup itimer conversion Thomas Gleixner
@ 2006-01-20  2:55 ` Thomas Gleixner
  2006-01-20  2:55 ` [PATCH 3/7] [hrtimers] Fix oldvalue return in setitimer Thomas Gleixner
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Thomas Gleixner @ 2006-01-20  2:55 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: LKML, Ingo Molnar, George Anzinger, Steven Rostedt, Andrew Morton

[-- Attachment #1: 0002-hrtimers-Fix-possible-use-of-NULL-pointer-in-posix-timers.txt --]
[-- Type: text/plain, Size: 645 bytes --]


Fixup the conversion of posix-timers to hrtimers.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---

 kernel/posix-timers.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

a42e5a139db5c1759bc98729152618ed5b80410e
diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
index 197208b..3b606d3 100644
--- a/kernel/posix-timers.c
+++ b/kernel/posix-timers.c
@@ -290,7 +290,8 @@ void do_schedule_next_timer(struct sigin
 		info->si_overrun = timr->it_overrun_last;
 	}
 
-	unlock_timer(timr, flags);
+	if (timr)
+		unlock_timer(timr, flags);
 }
 
 int posix_timer_event(struct k_itimer *timr,int si_private)
-- 
1.0.8

--


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/7] [hrtimers] Fix oldvalue return in setitimer
  2006-01-20  2:55 [PATCH 0/7] hrtimers updates Thomas Gleixner
  2006-01-20  2:55 ` [PATCH 1/7] [hrtimers] Fixup itimer conversion Thomas Gleixner
  2006-01-20  2:55 ` [PATCH 2/7] [hrtimers] Fix possible use of NULL pointer in posix-timers Thomas Gleixner
@ 2006-01-20  2:55 ` Thomas Gleixner
  2006-01-24 21:56   ` Orion Poplawski
  2006-01-20  2:55 ` [PATCH 4/7] [hrtimers] Fix posix-timer requeue race Thomas Gleixner
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Thomas Gleixner @ 2006-01-20  2:55 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: LKML, Ingo Molnar, George Anzinger, Steven Rostedt, Andrew Morton

[-- Attachment #1: 0003-hrtimers-Fix-oldvalue-return-in-setitimer.txt --]
[-- Type: text/plain, Size: 1202 bytes --]


This resolves bugzilla bug#5617. The oldvalue of the
timer was read after the timer was cancelled, so the
remaining time was always zero.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---

 kernel/itimer.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

3f59dd20898d805781b3eac7ed0807e7a0b30f2f
diff --git a/kernel/itimer.c b/kernel/itimer.c
index 6433d06..379be2f 100644
--- a/kernel/itimer.c
+++ b/kernel/itimer.c
@@ -155,16 +155,16 @@ int do_setitimer(int which, struct itime
 again:
 		spin_lock_irq(&tsk->sighand->siglock);
 		timer = &tsk->signal->real_timer;
-		/* We are sharing ->siglock with it_real_fn() */
-		if (hrtimer_try_to_cancel(timer) < 0) {
-			spin_unlock_irq(&tsk->sighand->siglock);
-			goto again;
-		}
 		if (ovalue) {
 			ovalue->it_value = itimer_get_remtime(timer);
 			ovalue->it_interval
 				= ktime_to_timeval(tsk->signal->it_real_incr);
 		}
+		/* We are sharing ->siglock with it_real_fn() */
+		if (hrtimer_try_to_cancel(timer) < 0) {
+			spin_unlock_irq(&tsk->sighand->siglock);
+			goto again;
+		}
 		tsk->signal->it_real_incr =
 			timeval_to_ktime(value->it_interval);
 		expires = timeval_to_ktime(value->it_value);
-- 
1.0.8

--


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/7] [hrtimers] Fix posix-timer requeue race
  2006-01-20  2:55 [PATCH 0/7] hrtimers updates Thomas Gleixner
                   ` (2 preceding siblings ...)
  2006-01-20  2:55 ` [PATCH 3/7] [hrtimers] Fix oldvalue return in setitimer Thomas Gleixner
@ 2006-01-20  2:55 ` Thomas Gleixner
  2006-01-20 11:36   ` Roman Zippel
  2006-01-20  2:55 ` [PATCH 5/7] [hrtimers] Cleanups and simplifications Thomas Gleixner
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Thomas Gleixner @ 2006-01-20  2:55 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: LKML, Ingo Molnar, George Anzinger, Steven Rostedt, Andrew Morton

[-- Attachment #1: 0004-hrtimers-Fix-posix-timer-requeue-race.txt --]
[-- Type: text/plain, Size: 2051 bytes --]


From: Steven Rostedtrostedt@goodmis.org <rostedt@goodmis.org>
Date: 1137711149 +0100

CPU0 expires a posix-timer and runs the callback function.
The signal is queued.
After releasing the posix-timer lock and before returning to
hrtimer_run_queue CPU0 gets interrupted.
CPU1 delivers the queued signal and rearms the timer.
CPU0 comes back to hrtimer_run_queue and sets the timer state to expired.
The next modification of the timer can result in an oops, because the state
information is wrong.

Keep track of state = RUNNING and check if the state has been in the return
path of hrtimer_run_queue. In case the state has been changed, ignore a
restart request and do not touch the state variable.

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---

 include/linux/hrtimer.h |    1 +
 kernel/hrtimer.c        |    5 +++++
 2 files changed, 6 insertions(+), 0 deletions(-)

7a42511f275d3c895be54f4e578921fc35e25dd2
diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index 089bfb1..c657f3d 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -40,6 +40,7 @@ enum hrtimer_restart {
 enum hrtimer_state {
 	HRTIMER_INACTIVE,	/* Timer is inactive */
 	HRTIMER_EXPIRED,		/* Timer is expired */
+	HRTIMER_RUNNING,		/* Timer is running the callback function */
 	HRTIMER_PENDING,		/* Timer is pending */
 };
 
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index f1c4155..f580dd9 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -550,6 +550,7 @@ static inline void run_hrtimer_queue(str
 		fn = timer->function;
 		data = timer->data;
 		set_curr_timer(base, timer);
+		timer->state = HRTIMER_RUNNING;
 		__remove_hrtimer(timer, base);
 		spin_unlock_irq(&base->lock);
 
@@ -565,6 +566,10 @@ static inline void run_hrtimer_queue(str
 
 		spin_lock_irq(&base->lock);
 
+		/* Another CPU has added back the timer */
+		if (timer->state != HRTIMER_RUNNING)
+			continue;
+
 		if (restart == HRTIMER_RESTART)
 			enqueue_hrtimer(timer, base);
 		else
-- 
1.0.8

--


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/7] [hrtimers] Cleanups and simplifications
  2006-01-20  2:55 [PATCH 0/7] hrtimers updates Thomas Gleixner
                   ` (3 preceding siblings ...)
  2006-01-20  2:55 ` [PATCH 4/7] [hrtimers] Fix posix-timer requeue race Thomas Gleixner
@ 2006-01-20  2:55 ` Thomas Gleixner
  2006-01-20  2:55 ` [PATCH 6/7] [hrtimers] Add back lost credit lines Thomas Gleixner
  2006-01-20  2:55 ` [PATCH 7/7] [hrtimers] Set correct initial expiry time for relative SIGEV_NONE timers Thomas Gleixner
  6 siblings, 0 replies; 15+ messages in thread
From: Thomas Gleixner @ 2006-01-20  2:55 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: LKML, Ingo Molnar, George Anzinger, Steven Rostedt, Andrew Morton

[-- Attachment #1: 0005-hrtimers-Cleanups-and-simplifications.txt --]
[-- Type: text/plain, Size: 8539 bytes --]


From: George Anzinger <george@wildturkeyranch.net>
Date: 1137711354 +0100

This patch cleans up the interface to hrtimers by changing the init code
to pass the mode as well as the clock.  This allow the init code to
select the correct base and eliminates extra timer re-init code in
posix-timers.  We also simplify the restart interface nanosleep use.

Signed-off-by: George Anzinger <george@mvista.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---

 include/linux/hrtimer.h |    5 ++--
 kernel/fork.c           |    2 +-
 kernel/hrtimer.c        |   59 +++++++++++++++++++----------------------------
 kernel/posix-timers.c   |   37 +++++++----------------------
 4 files changed, 36 insertions(+), 67 deletions(-)

0ea0b28ad86d611745b0a55b472f46d27c38e7a2
diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index c657f3d..6361544 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -101,9 +101,8 @@ struct hrtimer_base {
 /* Exported timer functions: */
 
 /* Initialize timers: */
-extern void hrtimer_init(struct hrtimer *timer, const clockid_t which_clock);
-extern void hrtimer_rebase(struct hrtimer *timer, const clockid_t which_clock);
-
+extern void hrtimer_init(struct hrtimer *timer, clockid_t which_clock,
+			 enum hrtimer_mode mode);
 
 /* Basic timer operations: */
 extern int hrtimer_start(struct hrtimer *timer, ktime_t tim,
diff --git a/kernel/fork.c b/kernel/fork.c
index 4ae8cfc..7f0ab5e 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -802,7 +802,7 @@ static inline int copy_signal(unsigned l
 	init_sigpending(&sig->shared_pending);
 	INIT_LIST_HEAD(&sig->posix_timers);
 
-	hrtimer_init(&sig->real_timer, CLOCK_MONOTONIC);
+	hrtimer_init(&sig->real_timer, CLOCK_MONOTONIC, HRTIMER_REL);
 	sig->it_real_incr.tv64 = 0;
 	sig->real_timer.function = it_real_fn;
 	sig->real_timer.data = tsk;
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index f580dd9..efff949 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -66,6 +66,12 @@ EXPORT_SYMBOL_GPL(ktime_get_real);
 
 /*
  * The timer bases:
+ *
+ * Note: If we want to add new timer bases, we have to skip the two
+ * clock ids captured by the cpu-timers. We do this by holding empty
+ * entries rather than doing math adjustment of the clock ids.
+ * This ensures that we capture erroneous accesses to these clock ids
+ * rather than moving them into the range of valid clock id's.
  */
 
 #define MAX_HRTIMER_BASES 2
@@ -483,29 +489,25 @@ ktime_t hrtimer_get_remaining(const stru
 }
 
 /**
- * hrtimer_rebase - rebase an initialized hrtimer to a different base
+ * hrtimer_init - initialize a timer to the given clock
  *
- * @timer:	the timer to be rebased
+ * @timer:	the timer to be initialized
  * @clock_id:	the clock to be used
+ * @mode:	timer mode abs/rel
  */
-void hrtimer_rebase(struct hrtimer *timer, const clockid_t clock_id)
+void hrtimer_init(struct hrtimer *timer, clockid_t clock_id,
+		  enum hrtimer_mode mode)
 {
 	struct hrtimer_base *bases;
 
+	memset(timer, 0, sizeof(struct hrtimer));
+
 	bases = per_cpu(hrtimer_bases, raw_smp_processor_id());
-	timer->base = &bases[clock_id];
-}
 
-/**
- * hrtimer_init - initialize a timer to the given clock
- *
- * @timer:	the timer to be initialized
- * @clock_id:	the clock to be used
- */
-void hrtimer_init(struct hrtimer *timer, const clockid_t clock_id)
-{
-	memset(timer, 0, sizeof(struct hrtimer));
-	hrtimer_rebase(timer, clock_id);
+	if (clock_id == CLOCK_REALTIME && mode != HRTIMER_ABS)
+		clock_id = CLOCK_MONOTONIC;
+
+	timer->base = &bases[clock_id];
 }
 
 /**
@@ -643,8 +645,7 @@ schedule_hrtimer_interruptible(struct hr
 	return schedule_hrtimer(timer, mode);
 }
 
-static long __sched
-nanosleep_restart(struct restart_block *restart, clockid_t clockid)
+static long __sched nanosleep_restart(struct restart_block *restart)
 {
 	struct timespec __user *rmtp;
 	struct timespec tu;
@@ -654,7 +655,7 @@ nanosleep_restart(struct restart_block *
 
 	restart->fn = do_no_restart_syscall;
 
-	hrtimer_init(&timer, clockid);
+	hrtimer_init(&timer, (clockid_t) restart->arg3, HRTIMER_ABS);
 
 	timer.expires.tv64 = ((u64)restart->arg1 << 32) | (u64) restart->arg0;
 
@@ -674,16 +675,6 @@ nanosleep_restart(struct restart_block *
 	return -ERESTART_RESTARTBLOCK;
 }
 
-static long __sched nanosleep_restart_mono(struct restart_block *restart)
-{
-	return nanosleep_restart(restart, CLOCK_MONOTONIC);
-}
-
-static long __sched nanosleep_restart_real(struct restart_block *restart)
-{
-	return nanosleep_restart(restart, CLOCK_REALTIME);
-}
-
 long hrtimer_nanosleep(struct timespec *rqtp, struct timespec __user *rmtp,
 		       const enum hrtimer_mode mode, const clockid_t clockid)
 {
@@ -692,7 +683,7 @@ long hrtimer_nanosleep(struct timespec *
 	struct timespec tu;
 	ktime_t rem;
 
-	hrtimer_init(&timer, clockid);
+	hrtimer_init(&timer, clockid, mode);
 
 	timer.expires = timespec_to_ktime(*rqtp);
 
@@ -700,7 +691,7 @@ long hrtimer_nanosleep(struct timespec *
 	if (rem.tv64 <= 0)
 		return 0;
 
-	/* Absolute timers do not update the rmtp value: */
+	/* Absolute timers do not update the rmtp value and restart: */
 	if (mode == HRTIMER_ABS)
 		return -ERESTARTNOHAND;
 
@@ -710,11 +701,11 @@ long hrtimer_nanosleep(struct timespec *
 		return -EFAULT;
 
 	restart = &current_thread_info()->restart_block;
-	restart->fn = (clockid == CLOCK_MONOTONIC) ?
-		nanosleep_restart_mono : nanosleep_restart_real;
+	restart->fn = nanosleep_restart;
 	restart->arg0 = timer.expires.tv64 & 0xFFFFFFFF;
 	restart->arg1 = timer.expires.tv64 >> 32;
 	restart->arg2 = (unsigned long) rmtp;
+	restart->arg3 = (unsigned long) timer.base->index;
 
 	return -ERESTART_RESTARTBLOCK;
 }
@@ -741,10 +732,8 @@ static void __devinit init_hrtimers_cpu(
 	struct hrtimer_base *base = per_cpu(hrtimer_bases, cpu);
 	int i;
 
-	for (i = 0; i < MAX_HRTIMER_BASES; i++) {
+	for (i = 0; i < MAX_HRTIMER_BASES; i++, base++)
 		spin_lock_init(&base->lock);
-		base++;
-	}
 }
 
 #ifdef CONFIG_HOTPLUG_CPU
diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
index 3b606d3..28e72fd 100644
--- a/kernel/posix-timers.c
+++ b/kernel/posix-timers.c
@@ -194,9 +194,7 @@ static inline int common_clock_set(const
 
 static int common_timer_create(struct k_itimer *new_timer)
 {
-	hrtimer_init(&new_timer->it.real.timer, new_timer->it_clock);
-	new_timer->it.real.timer.data = new_timer;
-	new_timer->it.real.timer.function = posix_timer_fn;
+	hrtimer_init(&new_timer->it.real.timer, new_timer->it_clock, 0);
 	return 0;
 }
 
@@ -693,6 +691,7 @@ common_timer_set(struct k_itimer *timr, 
 		 struct itimerspec *new_setting, struct itimerspec *old_setting)
 {
 	struct hrtimer *timer = &timr->it.real.timer;
+	enum hrtimer_mode mode;
 
 	if (old_setting)
 		common_timer_get(timr, old_setting);
@@ -714,14 +713,10 @@ common_timer_set(struct k_itimer *timr, 
 	if (!new_setting->it_value.tv_sec && !new_setting->it_value.tv_nsec)
 		return 0;
 
-	/* Posix madness. Only absolute CLOCK_REALTIME timers
-	 * are affected by clock sets. So we must reiniatilize
-	 * the timer.
-	 */
-	if (timr->it_clock == CLOCK_REALTIME && (flags & TIMER_ABSTIME))
-		hrtimer_rebase(timer, CLOCK_REALTIME);
-	else
-		hrtimer_rebase(timer, CLOCK_MONOTONIC);
+	mode = flags & TIMER_ABSTIME ? HRTIMER_ABS : HRTIMER_REL;
+	hrtimer_init(&timr->it.real.timer, timr->it_clock, mode);
+	timr->it.real.timer.data = timr;
+	timr->it.real.timer.function = posix_timer_fn;
 
 	timer->expires = timespec_to_ktime(new_setting->it_value);
 
@@ -732,8 +727,7 @@ common_timer_set(struct k_itimer *timr, 
 	if (((timr->it_sigev_notify & ~SIGEV_THREAD_ID) == SIGEV_NONE))
 		return 0;
 
-	hrtimer_start(timer, timer->expires, (flags & TIMER_ABSTIME) ?
-		      HRTIMER_ABS : HRTIMER_REL);
+	hrtimer_start(timer, timer->expires, mode);
 	return 0;
 }
 
@@ -948,21 +942,8 @@ sys_clock_getres(const clockid_t which_c
 static int common_nsleep(const clockid_t which_clock, int flags,
 			 struct timespec *tsave, struct timespec __user *rmtp)
 {
-	int mode = flags & TIMER_ABSTIME ? HRTIMER_ABS : HRTIMER_REL;
-	int clockid = which_clock;
-
-	switch (which_clock) {
-	case CLOCK_REALTIME:
-		/* Posix madness. Only absolute timers on clock realtime
-		   are affected by clock set. */
-		if (mode != HRTIMER_ABS)
-			clockid = CLOCK_MONOTONIC;
-	case CLOCK_MONOTONIC:
-		break;
-	default:
-		return -EINVAL;
-	}
-	return hrtimer_nanosleep(tsave, rmtp, mode, clockid);
+	return hrtimer_nanosleep(tsave, rmtp, flags & TIMER_ABSTIME ?
+				 HRTIMER_ABS : HRTIMER_REL, which_clock);
 }
 
 asmlinkage long
-- 
1.0.8

--


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 6/7] [hrtimers] Add back lost credit lines
  2006-01-20  2:55 [PATCH 0/7] hrtimers updates Thomas Gleixner
                   ` (4 preceding siblings ...)
  2006-01-20  2:55 ` [PATCH 5/7] [hrtimers] Cleanups and simplifications Thomas Gleixner
@ 2006-01-20  2:55 ` Thomas Gleixner
  2006-01-20  2:55 ` [PATCH 7/7] [hrtimers] Set correct initial expiry time for relative SIGEV_NONE timers Thomas Gleixner
  6 siblings, 0 replies; 15+ messages in thread
From: Thomas Gleixner @ 2006-01-20  2:55 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: LKML, Ingo Molnar, George Anzinger, Steven Rostedt, Andrew Morton

[-- Attachment #1: 0006-hrtimers-Add-back-lost-credit-lines.txt --]
[-- Type: text/plain, Size: 1305 bytes --]


At some point we added credits to people who actively helped
to bring k/hr-timers along. This was lost in the big code
revamp. Add it back.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@elte.hu>

---

 include/linux/ktime.h |    6 ++++++
 kernel/hrtimer.c      |    6 ++++++
 2 files changed, 12 insertions(+), 0 deletions(-)

4b2719dcb1143a18de16162f2562045e40487e49
diff --git a/include/linux/ktime.h b/include/linux/ktime.h
index 1bd6552..6aca67a 100644
--- a/include/linux/ktime.h
+++ b/include/linux/ktime.h
@@ -10,6 +10,12 @@
  *
  *  Started by: Thomas Gleixner and Ingo Molnar
  *
+ *  Credits:
+ *
+ *  	Roman Zippel provided the ideas and primary code snippets of
+ *  	the ktime_t union and further simplifications of the original
+ *  	code.
+ *
  *  For licencing details see kernel-base/COPYING
  */
 #ifndef _LINUX_KTIME_H
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index efff949..2b6e175 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -21,6 +21,12 @@
  *  Credits:
  *	based on kernel/timer.c
  *
+ *	Help, testing, suggestions, bugfixes, improvements were
+ *	provided by:
+ *
+ *	George Anzinger, Andrew Morton, Steven Rostedt, Roman Zippel
+ *	et. al.
+ *
  *  For licencing details see kernel-base/COPYING
  */
 
-- 
1.0.8

--


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 7/7] [hrtimers] Set correct initial expiry time for relative SIGEV_NONE timers
  2006-01-20  2:55 [PATCH 0/7] hrtimers updates Thomas Gleixner
                   ` (5 preceding siblings ...)
  2006-01-20  2:55 ` [PATCH 6/7] [hrtimers] Add back lost credit lines Thomas Gleixner
@ 2006-01-20  2:55 ` Thomas Gleixner
  2006-01-20  5:11   ` Andrew Morton
  2006-01-20 16:33   ` George Anzinger
  6 siblings, 2 replies; 15+ messages in thread
From: Thomas Gleixner @ 2006-01-20  2:55 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: LKML, Ingo Molnar, George Anzinger, Steven Rostedt, Andrew Morton

[-- Attachment #1: 0007-hrtimers-Set-correct-initial-expiry-time-for-relative-SIGEV_NONE-timers.txt --]
[-- Type: text/plain, Size: 1055 bytes --]


The expiry time for relative timers with SIGEV_NONE set was never
updated to the correct value.

Pointed out by George Anzinger.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

---

 kernel/posix-timers.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

a1f15939b7af18c5abcd4810ccd512467c77a6b1
diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
index 28e72fd..e2fa4c0 100644
--- a/kernel/posix-timers.c
+++ b/kernel/posix-timers.c
@@ -724,8 +724,13 @@ common_timer_set(struct k_itimer *timr, 
 	timr->it.real.interval = timespec_to_ktime(new_setting->it_interval);
 
 	/* SIGEV_NONE timers are not queued ! See common_timer_get */
-	if (((timr->it_sigev_notify & ~SIGEV_THREAD_ID) == SIGEV_NONE))
+	if (((timr->it_sigev_notify & ~SIGEV_THREAD_ID) == SIGEV_NONE)) {
+		/* Setup correct expiry time for relative timers */
+		if (mode == HRTIMER_REL)
+			timer->expires = ktime_add(timer-expires,
+						   timer->base->get_time());
 		return 0;
+	}
 
 	hrtimer_start(timer, timer->expires, mode);
 	return 0;
-- 
1.0.8

--


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 7/7] [hrtimers] Set correct initial expiry time for relative SIGEV_NONE timers
  2006-01-20  2:55 ` [PATCH 7/7] [hrtimers] Set correct initial expiry time for relative SIGEV_NONE timers Thomas Gleixner
@ 2006-01-20  5:11   ` Andrew Morton
  2006-01-20  9:49     ` Thomas Gleixner
  2006-01-20 16:33   ` George Anzinger
  1 sibling, 1 reply; 15+ messages in thread
From: Andrew Morton @ 2006-01-20  5:11 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: torvalds, linux-kernel, mingo, george, rostedt

Thomas Gleixner <tglx@linutronix.de> wrote:
>
> 
> The expiry time for relative timers with SIGEV_NONE set was never
> updated to the correct value.
> 

Ahem.

> +		if (mode == HRTIMER_REL)
> +			timer->expires = ktime_add(timer-expires,
> +						   timer->base->get_time());

doesn't compile, hence isn't tested.

Please confirm that with the below fix we get something which works OK?

--- devel/kernel/posix-timers.c~hrtimers-set-correct-initial-expiry-time-for-relative-fix	2006-01-19 21:08:44.000000000 -0800
+++ devel-akpm/kernel/posix-timers.c	2006-01-19 21:08:44.000000000 -0800
@@ -727,7 +727,7 @@ common_timer_set(struct k_itimer *timr, 
 	if (((timr->it_sigev_notify & ~SIGEV_THREAD_ID) == SIGEV_NONE)) {
 		/* Setup correct expiry time for relative timers */
 		if (mode == HRTIMER_REL)
-			timer->expires = ktime_add(timer-expires,
+			timer->expires = ktime_add(timer->expires,
 						   timer->base->get_time());
 		return 0;
 	}
_

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 7/7] [hrtimers] Set correct initial expiry time for relative SIGEV_NONE timers
  2006-01-20  5:11   ` Andrew Morton
@ 2006-01-20  9:49     ` Thomas Gleixner
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Gleixner @ 2006-01-20  9:49 UTC (permalink / raw)
  To: Andrew Morton; +Cc: torvalds, linux-kernel, mingo, george, rostedt

On Thu, 2006-01-19 at 21:11 -0800, Andrew Morton wrote:
> Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> > 
> > The expiry time for relative timers with SIGEV_NONE set was never
> > updated to the correct value.
> > 
> 
> Ahem.
> 
> > +		if (mode == HRTIMER_REL)
> > +			timer->expires = ktime_add(timer-expires,
> > +						   timer->base->get_time());
> 
> doesn't compile, hence isn't tested.

Oh well, I wanted to add quilt autorefresh mode since long :(

	tglx



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/7] [hrtimers] Fix posix-timer requeue race
  2006-01-20  2:55 ` [PATCH 4/7] [hrtimers] Fix posix-timer requeue race Thomas Gleixner
@ 2006-01-20 11:36   ` Roman Zippel
  0 siblings, 0 replies; 15+ messages in thread
From: Roman Zippel @ 2006-01-20 11:36 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Linus Torvalds, LKML, Ingo Molnar, George Anzinger,
	Steven Rostedt, Andrew Morton

Hi,

On Fri, 20 Jan 2006, Thomas Gleixner wrote:

> CPU0 expires a posix-timer and runs the callback function.
> The signal is queued.
> After releasing the posix-timer lock and before returning to
> hrtimer_run_queue CPU0 gets interrupted.
> CPU1 delivers the queued signal and rearms the timer.
> CPU0 comes back to hrtimer_run_queue and sets the timer state to expired.
> The next modification of the timer can result in an oops, because the state
> information is wrong.

Thomas, this is exactly what I meant with overloaded state machines,
enumerated states are very hard to get right with concurrent processes.
Please get rid of the state field and at least use a flag field instead.

bye, Roman

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 7/7] [hrtimers] Set correct initial expiry time for relative SIGEV_NONE timers
  2006-01-20  2:55 ` [PATCH 7/7] [hrtimers] Set correct initial expiry time for relative SIGEV_NONE timers Thomas Gleixner
  2006-01-20  5:11   ` Andrew Morton
@ 2006-01-20 16:33   ` George Anzinger
  2006-01-20 16:58     ` Thomas Gleixner
  1 sibling, 1 reply; 15+ messages in thread
From: George Anzinger @ 2006-01-20 16:33 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Linus Torvalds, LKML, Ingo Molnar, Steven Rostedt, Andrew Morton

Thomas Gleixner wrote:
> The expiry time for relative timers with SIGEV_NONE set was never
> updated to the correct value.
> 
> Pointed out by George Anzinger.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> 
> ---
> 
>  kernel/posix-timers.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
> 
> a1f15939b7af18c5abcd4810ccd512467c77a6b1
> diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
> index 28e72fd..e2fa4c0 100644
> --- a/kernel/posix-timers.c
> +++ b/kernel/posix-timers.c
> @@ -724,8 +724,13 @@ common_timer_set(struct k_itimer *timr, 
>  	timr->it.real.interval = timespec_to_ktime(new_setting->it_interval);
>  
>  	/* SIGEV_NONE timers are not queued ! See common_timer_get */
> -	if (((timr->it_sigev_notify & ~SIGEV_THREAD_ID) == SIGEV_NONE))
> +	if (((timr->it_sigev_notify & ~SIGEV_THREAD_ID) == SIGEV_NONE)) {
> +		/* Setup correct expiry time for relative timers */
> +		if (mode == HRTIMER_REL)
> +			timer->expires = ktime_add(timer-expires,
> +						   timer->base->get_time());
This is only part of the problem.  When the user does a timer_gettime() the 
expiry time is taken from the hrtimer field and NOT the posix-timer part. 
Somewhere this value needs to be copied to the hrtimer sub structure.

George
-- 

>  		return 0;
> +	}
>  
>  	hrtimer_start(timer, timer->expires, mode);
>  	return 0;

-- 
George Anzinger   george@wildturkeyranch.net
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 7/7] [hrtimers] Set correct initial expiry time for relative SIGEV_NONE timers
  2006-01-20 16:33   ` George Anzinger
@ 2006-01-20 16:58     ` Thomas Gleixner
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Gleixner @ 2006-01-20 16:58 UTC (permalink / raw)
  To: george; +Cc: Linus Torvalds, LKML, Ingo Molnar, Steven Rostedt, Andrew Morton

On Fri, 2006-01-20 at 08:33 -0800, George Anzinger wrote:
> > a1f15939b7af18c5abcd4810ccd512467c77a6b1
> > diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c
> > index 28e72fd..e2fa4c0 100644
> > --- a/kernel/posix-timers.c
> > +++ b/kernel/posix-timers.c
> > @@ -724,8 +724,13 @@ common_timer_set(struct k_itimer *timr, 
> >  	timr->it.real.interval = timespec_to_ktime(new_setting->it_interval);
> >  
> >  	/* SIGEV_NONE timers are not queued ! See common_timer_get */
> > -	if (((timr->it_sigev_notify & ~SIGEV_THREAD_ID) == SIGEV_NONE))
> > +	if (((timr->it_sigev_notify & ~SIGEV_THREAD_ID) == SIGEV_NONE)) {
> > +		/* Setup correct expiry time for relative timers */
> > +		if (mode == HRTIMER_REL)
> > +			timer->expires = ktime_add(timer-expires,
> > +						   timer->base->get_time());
> This is only part of the problem.  When the user does a timer_gettime() the 
> expiry time is taken from the hrtimer field and NOT the posix-timer part. 
> Somewhere this value needs to be copied to the hrtimer sub structure.

Well timer->expires is the hrtimer struct itself.

	tglx





^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/7] [hrtimers] Fix oldvalue return in setitimer
  2006-01-20  2:55 ` [PATCH 3/7] [hrtimers] Fix oldvalue return in setitimer Thomas Gleixner
@ 2006-01-24 21:56   ` Orion Poplawski
  2006-01-25  6:56     ` Thomas Gleixner
  0 siblings, 1 reply; 15+ messages in thread
From: Orion Poplawski @ 2006-01-24 21:56 UTC (permalink / raw)
  To: linux-kernel

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Thomas Gleixner wrote:
> This resolves bugzilla bug#5617. The oldvalue of the
> timer was read after the timer was cancelled, so the
> remaining time was always zero.
> 

I'm seeing this problem on recent Fedore development kernels.
Interestingly, it causes the IDL 7 minute timed demo to exit immediately
upon trying to plot since it resets the timer and expects the old value
to be returned.

- - Orion
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFD1qJyORnzrtFC2/sRAnp4AJ9ZQ/E0huj2sk8TOxF/1QF7OnrtQQCdHwsT
g0YGeKL3Co9isimpQJ8f3mU=
=R+Z4
-----END PGP SIGNATURE-----


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 3/7] [hrtimers] Fix oldvalue return in setitimer
  2006-01-24 21:56   ` Orion Poplawski
@ 2006-01-25  6:56     ` Thomas Gleixner
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Gleixner @ 2006-01-25  6:56 UTC (permalink / raw)
  To: Orion Poplawski; +Cc: linux-kernel

On Tue, 2006-01-24 at 14:56 -0700, Orion Poplawski wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Thomas Gleixner wrote:
> > This resolves bugzilla bug#5617. The oldvalue of the
> > timer was read after the timer was cancelled, so the
> > remaining time was always zero.
> > 
> 
> I'm seeing this problem on recent Fedore development kernels.
> Interestingly, it causes the IDL 7 minute timed demo to exit immediately
> upon trying to plot since it resets the timer and expects the old value
> to be returned.

Thats the same problem.

	tglx



^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2006-01-25  6:55 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-01-20  2:55 [PATCH 0/7] hrtimers updates Thomas Gleixner
2006-01-20  2:55 ` [PATCH 1/7] [hrtimers] Fixup itimer conversion Thomas Gleixner
2006-01-20  2:55 ` [PATCH 2/7] [hrtimers] Fix possible use of NULL pointer in posix-timers Thomas Gleixner
2006-01-20  2:55 ` [PATCH 3/7] [hrtimers] Fix oldvalue return in setitimer Thomas Gleixner
2006-01-24 21:56   ` Orion Poplawski
2006-01-25  6:56     ` Thomas Gleixner
2006-01-20  2:55 ` [PATCH 4/7] [hrtimers] Fix posix-timer requeue race Thomas Gleixner
2006-01-20 11:36   ` Roman Zippel
2006-01-20  2:55 ` [PATCH 5/7] [hrtimers] Cleanups and simplifications Thomas Gleixner
2006-01-20  2:55 ` [PATCH 6/7] [hrtimers] Add back lost credit lines Thomas Gleixner
2006-01-20  2:55 ` [PATCH 7/7] [hrtimers] Set correct initial expiry time for relative SIGEV_NONE timers Thomas Gleixner
2006-01-20  5:11   ` Andrew Morton
2006-01-20  9:49     ` Thomas Gleixner
2006-01-20 16:33   ` George Anzinger
2006-01-20 16:58     ` Thomas Gleixner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).