* [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
* 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
* [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
* [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 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).