From 8e63d7795e30b4091e303cc8c060509bd8eea742 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Tue, 25 May 2010 20:43:30 +0200 Subject: timers: Fix slack calculation really commit f00e047ef (timers: Fix slack calculation for expired timers) fixed the issue of slack on expired timers only partially. Linus noticed that jiffies is volatile so it is reloaded twice, which generates bad code. But its worse. This can defeat the time_after() check if jiffies are incremented between time_after() and the slack calculation. Fix it by reading jiffies into a local variable, which prevents the compiler from loading it twice. While at it make the > -1 check into >= 0 which is easier to read. Signed-off-by: Thomas Gleixner Cc: Arjan van de Ven Cc: Linus Torvalds --- kernel/timer.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'kernel') diff --git a/kernel/timer.c b/kernel/timer.c index be394af5bc22..d8decb8d46b0 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -747,16 +747,19 @@ EXPORT_SYMBOL(mod_timer_pending); static inline unsigned long apply_slack(struct timer_list *timer, unsigned long expires) { - unsigned long expires_limit, mask; + unsigned long expires_limit, mask, now; int bit; expires_limit = expires; - if (timer->slack > -1) + if (timer->slack >= 0) { expires_limit = expires + timer->slack; - else if (time_after(expires, jiffies)) /* auto slack: use 0.4% */ - expires_limit = expires + (expires - jiffies)/256; - + } else { + now = jiffies; + /* No slack, if already expired else auto slack 0.4% */ + if (time_after(expires, now)) + expires_limit = expires + (expires - now)/256; + } mask = expires ^ expires_limit; if (mask == 0) return expires; -- cgit v1.2.3-59-g8ed1b From 2abfb9e1d470f7082e5e20e4b11a271a0124211b Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Wed, 26 May 2010 16:07:13 +0200 Subject: timers: Move local variable into else section Fix nit-picking coding style detail. Signed-off-by: Thomas Gleixner --- kernel/timer.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'kernel') diff --git a/kernel/timer.c b/kernel/timer.c index d8decb8d46b0..22118342a456 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -747,7 +747,7 @@ EXPORT_SYMBOL(mod_timer_pending); static inline unsigned long apply_slack(struct timer_list *timer, unsigned long expires) { - unsigned long expires_limit, mask, now; + unsigned long expires_limit, mask; int bit; expires_limit = expires; @@ -755,7 +755,8 @@ unsigned long apply_slack(struct timer_list *timer, unsigned long expires) if (timer->slack >= 0) { expires_limit = expires + timer->slack; } else { - now = jiffies; + unsigned long now = jiffies; + /* No slack, if already expired else auto slack 0.4% */ if (time_after(expires, now)) expires_limit = expires + (expires - now)/256; -- cgit v1.2.3-59-g8ed1b From 174bd1994ec67a6e6191c4ed8e5dac17fa221b84 Mon Sep 17 00:00:00 2001 From: Stanislaw Gruszka Date: Tue, 25 May 2010 23:49:12 +0200 Subject: hrtimer: Avoid double seqlock hrtimer_get_softirq_time() has it's own xtime lock protection, so it's safe to use plain __current_kernel_time() and avoid the double seqlock loop. Signed-off-by: Stanislaw Gruszka LKML-Reference: <20100525214912.GA1934@r2bh72.net.upc.cz> Signed-off-by: Thomas Gleixner --- kernel/hrtimer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'kernel') diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c index b9b134b35088..5c69e996bd0f 100644 --- a/kernel/hrtimer.c +++ b/kernel/hrtimer.c @@ -89,7 +89,7 @@ static void hrtimer_get_softirq_time(struct hrtimer_cpu_base *base) do { seq = read_seqbegin(&xtime_lock); - xts = current_kernel_time(); + xts = __current_kernel_time(); tom = wall_to_monotonic; } while (read_seqretry(&xtime_lock, seq)); -- cgit v1.2.3-59-g8ed1b From 45e0fffc8a7778282e6a1514a6ae3e7ae6545111 Mon Sep 17 00:00:00 2001 From: Andrey Vagin Date: Mon, 24 May 2010 12:15:33 -0700 Subject: posix_timer: Fix error path in timer_create Move CLOCK_DISPATCH(which_clock, timer_create, (new_timer)) after all posible EFAULT erros. *_timer_create may allocate/get resources. (for example posix_cpu_timer_create does get_task_struct) [ tglx: fold the remove crappy comment patch into this ] Signed-off-by: Andrey Vagin Cc: Oleg Nesterov Cc: Pavel Emelyanov Cc: Reviewed-by: Stanislaw Gruszka Signed-off-by: Andrew Morton Signed-off-by: Thomas Gleixner --- kernel/posix-timers.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) (limited to 'kernel') diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c index 00d1fda58ab6..ad723420acc3 100644 --- a/kernel/posix-timers.c +++ b/kernel/posix-timers.c @@ -559,14 +559,7 @@ SYSCALL_DEFINE3(timer_create, const clockid_t, which_clock, new_timer->it_id = (timer_t) new_timer_id; new_timer->it_clock = which_clock; new_timer->it_overrun = -1; - error = CLOCK_DISPATCH(which_clock, timer_create, (new_timer)); - if (error) - goto out; - /* - * return the timer_id now. The next step is hard to - * back out if there is an error. - */ if (copy_to_user(created_timer_id, &new_timer_id, sizeof (new_timer_id))) { error = -EFAULT; @@ -597,6 +590,10 @@ SYSCALL_DEFINE3(timer_create, const clockid_t, which_clock, new_timer->sigq->info.si_tid = new_timer->it_id; new_timer->sigq->info.si_code = SI_TIMER; + error = CLOCK_DISPATCH(which_clock, timer_create, (new_timer)); + if (error) + goto out; + spin_lock_irq(¤t->sighand->siglock); new_timer->it_signal = current->signal; list_add(&new_timer->list, ¤t->signal->posix_timers); -- cgit v1.2.3-59-g8ed1b