* [patch 0/2] posix-cpu-timers: Unbreak interval timers
@ 2019-01-11 13:33 Thomas Gleixner
2019-01-11 13:33 ` [patch 1/2] posix-cpu-timers: Unbreak timer rearming Thomas Gleixner
2019-01-11 13:33 ` [patch 2/2] posix-cpu-timers: Remove private interval storage Thomas Gleixner
0 siblings, 2 replies; 6+ messages in thread
From: Thomas Gleixner @ 2019-01-11 13:33 UTC (permalink / raw)
To: LKML; +Cc: John Stultz, Peter Zijlstra, H.J. Lu
The recent commit which prevented a division by 0 issue in the alarm timer
code broke posix CPU timers as an unwanted side effect.
The following series unbreaks them and removes the duplicated storage of
the interval which is not longer required.
Thanks,
tglx
^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch 1/2] posix-cpu-timers: Unbreak timer rearming
2019-01-11 13:33 [patch 0/2] posix-cpu-timers: Unbreak interval timers Thomas Gleixner
@ 2019-01-11 13:33 ` Thomas Gleixner
2019-01-11 16:10 ` H.J. Lu
2019-01-15 15:36 ` [tip:timers/urgent] " tip-bot for Thomas Gleixner
2019-01-11 13:33 ` [patch 2/2] posix-cpu-timers: Remove private interval storage Thomas Gleixner
1 sibling, 2 replies; 6+ messages in thread
From: Thomas Gleixner @ 2019-01-11 13:33 UTC (permalink / raw)
To: LKML; +Cc: John Stultz, Peter Zijlstra, H.J. Lu
The recent commit which prevented a division by 0 issue in the alarm timer
code broke posix CPU timers as an unwanted side effect.
The reason is that the common rearm code checks for timer->it_interval
being 0 now. What went unnoticed is that the posix cpu timer setup does not
initialize timer->it_interval as it stores the interval in CPU timer
specific storage. The reason for the separate storage is historical as the
posix CPU timers always had a 64bit nanoseconds representation internally
while timer->it_interval is type ktime_t which used to be a modified
timespec representation on 32bit machines.
Instead of reverting the offending commit and fixing the alarmtimer issue
in the alarmtimer code, store the interval in timer->it_interval at CPU
timer setup time so the common code check works. This also repairs the
existing inconistency of the posix CPU timer code which kept a single shot
timer armed despite of the interval being 0.
The separate storage can be removed in mainline, but that needs to be a
separate commit as the current one has to be backported to stable kernels.
Fixes: 0e334db6bb4b ("posix-timers: Fix division by zero bug")
Reported-by: H.J. Lu <hjl.tools@gmail.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: John Stultz <john.stultz@linaro.org>
---
kernel/time/posix-cpu-timers.c | 1 +
1 file changed, 1 insertion(+)
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -685,6 +685,7 @@ static int posix_cpu_timer_set(struct k_
* set up the signal and overrun bookkeeping.
*/
timer->it.cpu.incr = timespec64_to_ns(&new->it_interval);
+ timer->it_interval = ns_to_ktime(timer->it.cpu.incr);
/*
* This acts as a modification timestamp for the timer,
^ permalink raw reply [flat|nested] 6+ messages in thread
* [patch 2/2] posix-cpu-timers: Remove private interval storage
2019-01-11 13:33 [patch 0/2] posix-cpu-timers: Unbreak interval timers Thomas Gleixner
2019-01-11 13:33 ` [patch 1/2] posix-cpu-timers: Unbreak timer rearming Thomas Gleixner
@ 2019-01-11 13:33 ` Thomas Gleixner
2019-01-15 15:39 ` [tip:timers/core] " tip-bot for Thomas Gleixner
1 sibling, 1 reply; 6+ messages in thread
From: Thomas Gleixner @ 2019-01-11 13:33 UTC (permalink / raw)
To: LKML; +Cc: John Stultz, Peter Zijlstra, H.J. Lu
Posix CPU timers store the interval in private storage for historical
reasons (it_interval used to be a non scalar representation on 32bit
systems). This is gone and there is no reason for duplicated storage
anymore.
Use it_interval everywhere.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
include/linux/posix-timers.h | 2 +-
kernel/time/posix-cpu-timers.c | 13 ++++++-------
2 files changed, 7 insertions(+), 8 deletions(-)
--- a/include/linux/posix-timers.h
+++ b/include/linux/posix-timers.h
@@ -12,7 +12,7 @@ struct siginfo;
struct cpu_timer_list {
struct list_head entry;
- u64 expires, incr;
+ u64 expires;
struct task_struct *task;
int firing;
};
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -67,13 +67,13 @@ static void bump_cpu_timer(struct k_itim
int i;
u64 delta, incr;
- if (timer->it.cpu.incr == 0)
+ if (!timer->it_interval)
return;
if (now < timer->it.cpu.expires)
return;
- incr = timer->it.cpu.incr;
+ incr = timer->it_interval;
delta = now + incr - timer->it.cpu.expires;
/* Don't use (incr*2 < delta), incr*2 might overflow. */
@@ -520,7 +520,7 @@ static void cpu_timer_fire(struct k_itim
*/
wake_up_process(timer->it_process);
timer->it.cpu.expires = 0;
- } else if (timer->it.cpu.incr == 0) {
+ } else if (!timer->it_interval) {
/*
* One-shot timer. Clear it as soon as it's fired.
*/
@@ -606,7 +606,7 @@ static int posix_cpu_timer_set(struct k_
*/
ret = 0;
- old_incr = timer->it.cpu.incr;
+ old_incr = timer->it_interval;
old_expires = timer->it.cpu.expires;
if (unlikely(timer->it.cpu.firing)) {
timer->it.cpu.firing = -1;
@@ -684,8 +684,7 @@ static int posix_cpu_timer_set(struct k_
* Install the new reload setting, and
* set up the signal and overrun bookkeeping.
*/
- timer->it.cpu.incr = timespec64_to_ns(&new->it_interval);
- timer->it_interval = ns_to_ktime(timer->it.cpu.incr);
+ timer->it_interval = timespec64_to_ktime(new->it_interval);
/*
* This acts as a modification timestamp for the timer,
@@ -724,7 +723,7 @@ static void posix_cpu_timer_get(struct k
/*
* Easy part: convert the reload time.
*/
- itp->it_interval = ns_to_timespec64(timer->it.cpu.incr);
+ itp->it_interval = ktime_to_timespec64(timer->it_interval);
if (!timer->it.cpu.expires)
return;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch 1/2] posix-cpu-timers: Unbreak timer rearming
2019-01-11 13:33 ` [patch 1/2] posix-cpu-timers: Unbreak timer rearming Thomas Gleixner
@ 2019-01-11 16:10 ` H.J. Lu
2019-01-15 15:36 ` [tip:timers/urgent] " tip-bot for Thomas Gleixner
1 sibling, 0 replies; 6+ messages in thread
From: H.J. Lu @ 2019-01-11 16:10 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: LKML, John Stultz, Peter Zijlstra
On Fri, Jan 11, 2019 at 5:36 AM Thomas Gleixner <tglx@linutronix.de> wrote:
>
> The recent commit which prevented a division by 0 issue in the alarm timer
> code broke posix CPU timers as an unwanted side effect.
>
> The reason is that the common rearm code checks for timer->it_interval
> being 0 now. What went unnoticed is that the posix cpu timer setup does not
> initialize timer->it_interval as it stores the interval in CPU timer
> specific storage. The reason for the separate storage is historical as the
> posix CPU timers always had a 64bit nanoseconds representation internally
> while timer->it_interval is type ktime_t which used to be a modified
> timespec representation on 32bit machines.
>
> Instead of reverting the offending commit and fixing the alarmtimer issue
> in the alarmtimer code, store the interval in timer->it_interval at CPU
> timer setup time so the common code check works. This also repairs the
> existing inconistency of the posix CPU timer code which kept a single shot
> timer armed despite of the interval being 0.
>
> The separate storage can be removed in mainline, but that needs to be a
> separate commit as the current one has to be backported to stable kernels.
>
> Fixes: 0e334db6bb4b ("posix-timers: Fix division by zero bug")
> Reported-by: H.J. Lu <hjl.tools@gmail.com>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Cc: stable@vger.kernel.org
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: John Stultz <john.stultz@linaro.org>
> ---
> kernel/time/posix-cpu-timers.c | 1 +
> 1 file changed, 1 insertion(+)
>
> --- a/kernel/time/posix-cpu-timers.c
> +++ b/kernel/time/posix-cpu-timers.c
> @@ -685,6 +685,7 @@ static int posix_cpu_timer_set(struct k_
> * set up the signal and overrun bookkeeping.
> */
> timer->it.cpu.incr = timespec64_to_ns(&new->it_interval);
> + timer->it_interval = ns_to_ktime(timer->it.cpu.incr);
>
> /*
> * This acts as a modification timestamp for the timer,
>
>
I verified that this patch works on 4.19.14 kernel.
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip:timers/urgent] posix-cpu-timers: Unbreak timer rearming
2019-01-11 13:33 ` [patch 1/2] posix-cpu-timers: Unbreak timer rearming Thomas Gleixner
2019-01-11 16:10 ` H.J. Lu
@ 2019-01-15 15:36 ` tip-bot for Thomas Gleixner
1 sibling, 0 replies; 6+ messages in thread
From: tip-bot for Thomas Gleixner @ 2019-01-15 15:36 UTC (permalink / raw)
To: linux-tip-commits
Cc: mingo, hjl.tools, peterz, hpa, linux-kernel, tglx, john.stultz
Commit-ID: 93ad0fc088c5b4631f796c995bdd27a082ef33a6
Gitweb: https://git.kernel.org/tip/93ad0fc088c5b4631f796c995bdd27a082ef33a6
Author: Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Fri, 11 Jan 2019 14:33:16 +0100
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 15 Jan 2019 16:34:37 +0100
posix-cpu-timers: Unbreak timer rearming
The recent commit which prevented a division by 0 issue in the alarm timer
code broke posix CPU timers as an unwanted side effect.
The reason is that the common rearm code checks for timer->it_interval
being 0 now. What went unnoticed is that the posix cpu timer setup does not
initialize timer->it_interval as it stores the interval in CPU timer
specific storage. The reason for the separate storage is historical as the
posix CPU timers always had a 64bit nanoseconds representation internally
while timer->it_interval is type ktime_t which used to be a modified
timespec representation on 32bit machines.
Instead of reverting the offending commit and fixing the alarmtimer issue
in the alarmtimer code, store the interval in timer->it_interval at CPU
timer setup time so the common code check works. This also repairs the
existing inconistency of the posix CPU timer code which kept a single shot
timer armed despite of the interval being 0.
The separate storage can be removed in mainline, but that needs to be a
separate commit as the current one has to be backported to stable kernels.
Fixes: 0e334db6bb4b ("posix-timers: Fix division by zero bug")
Reported-by: H.J. Lu <hjl.tools@gmail.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/20190111133500.840117406@linutronix.de
---
kernel/time/posix-cpu-timers.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
index 8f0644af40be..80f955210861 100644
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -685,6 +685,7 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int timer_flags,
* set up the signal and overrun bookkeeping.
*/
timer->it.cpu.incr = timespec64_to_ns(&new->it_interval);
+ timer->it_interval = ns_to_ktime(timer->it.cpu.incr);
/*
* This acts as a modification timestamp for the timer,
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [tip:timers/core] posix-cpu-timers: Remove private interval storage
2019-01-11 13:33 ` [patch 2/2] posix-cpu-timers: Remove private interval storage Thomas Gleixner
@ 2019-01-15 15:39 ` tip-bot for Thomas Gleixner
0 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Thomas Gleixner @ 2019-01-15 15:39 UTC (permalink / raw)
To: linux-tip-commits
Cc: mingo, tglx, linux-kernel, john.stultz, hjl.tools, hpa, peterz
Commit-ID: 16118794ede91aac1a73abe15de22d3de9d2b775
Gitweb: https://git.kernel.org/tip/16118794ede91aac1a73abe15de22d3de9d2b775
Author: Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Fri, 11 Jan 2019 14:33:17 +0100
Committer: Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 15 Jan 2019 16:36:13 +0100
posix-cpu-timers: Remove private interval storage
Posix CPU timers store the interval in private storage for historical
reasons (it_interval used to be a non scalar representation on 32bit
systems). This is gone and there is no reason for duplicated storage
anymore.
Use it_interval everywhere.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: "H.J. Lu" <hjl.tools@gmail.com>
Link: https://lkml.kernel.org/r/20190111133500.945255655@linutronix.de
---
include/linux/posix-timers.h | 2 +-
kernel/time/posix-cpu-timers.c | 13 ++++++-------
2 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/include/linux/posix-timers.h b/include/linux/posix-timers.h
index e96581ca7c9d..b20798fc5191 100644
--- a/include/linux/posix-timers.h
+++ b/include/linux/posix-timers.h
@@ -12,7 +12,7 @@ struct siginfo;
struct cpu_timer_list {
struct list_head entry;
- u64 expires, incr;
+ u64 expires;
struct task_struct *task;
int firing;
};
diff --git a/kernel/time/posix-cpu-timers.c b/kernel/time/posix-cpu-timers.c
index 80f955210861..0a426f4e3125 100644
--- a/kernel/time/posix-cpu-timers.c
+++ b/kernel/time/posix-cpu-timers.c
@@ -67,13 +67,13 @@ static void bump_cpu_timer(struct k_itimer *timer, u64 now)
int i;
u64 delta, incr;
- if (timer->it.cpu.incr == 0)
+ if (!timer->it_interval)
return;
if (now < timer->it.cpu.expires)
return;
- incr = timer->it.cpu.incr;
+ incr = timer->it_interval;
delta = now + incr - timer->it.cpu.expires;
/* Don't use (incr*2 < delta), incr*2 might overflow. */
@@ -520,7 +520,7 @@ static void cpu_timer_fire(struct k_itimer *timer)
*/
wake_up_process(timer->it_process);
timer->it.cpu.expires = 0;
- } else if (timer->it.cpu.incr == 0) {
+ } else if (!timer->it_interval) {
/*
* One-shot timer. Clear it as soon as it's fired.
*/
@@ -606,7 +606,7 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int timer_flags,
*/
ret = 0;
- old_incr = timer->it.cpu.incr;
+ old_incr = timer->it_interval;
old_expires = timer->it.cpu.expires;
if (unlikely(timer->it.cpu.firing)) {
timer->it.cpu.firing = -1;
@@ -684,8 +684,7 @@ static int posix_cpu_timer_set(struct k_itimer *timer, int timer_flags,
* Install the new reload setting, and
* set up the signal and overrun bookkeeping.
*/
- timer->it.cpu.incr = timespec64_to_ns(&new->it_interval);
- timer->it_interval = ns_to_ktime(timer->it.cpu.incr);
+ timer->it_interval = timespec64_to_ktime(new->it_interval);
/*
* This acts as a modification timestamp for the timer,
@@ -724,7 +723,7 @@ static void posix_cpu_timer_get(struct k_itimer *timer, struct itimerspec64 *itp
/*
* Easy part: convert the reload time.
*/
- itp->it_interval = ns_to_timespec64(timer->it.cpu.incr);
+ itp->it_interval = ktime_to_timespec64(timer->it_interval);
if (!timer->it.cpu.expires)
return;
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-01-15 15:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-11 13:33 [patch 0/2] posix-cpu-timers: Unbreak interval timers Thomas Gleixner
2019-01-11 13:33 ` [patch 1/2] posix-cpu-timers: Unbreak timer rearming Thomas Gleixner
2019-01-11 16:10 ` H.J. Lu
2019-01-15 15:36 ` [tip:timers/urgent] " tip-bot for Thomas Gleixner
2019-01-11 13:33 ` [patch 2/2] posix-cpu-timers: Remove private interval storage Thomas Gleixner
2019-01-15 15:39 ` [tip:timers/core] " tip-bot for Thomas Gleixner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.