All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch 0/4] hrtimers: Handle remaining time correctly for CONFIG_TIME_LOW_RES=y
@ 2016-01-14 16:54 ` Thomas Gleixner
  0 siblings, 0 replies; 17+ messages in thread
From: Thomas Gleixner @ 2016-01-14 16:54 UTC (permalink / raw)
  To: LKML
  Cc: Ingo Molnar, Peter Zijlstra, Helge Deller, John Stultz,
	linux-m68k, dhowells

Helge reported, that timerfd returns occasionally remaining time larger than
the relative time which was used to arm the timer. This is caused by the extra
jiffy which we add in hrtimer_start_range_ns() if CONFIG_TIME_LOW_RES=y.

This is not only an issue for timerfd. We have the same problem in
posix-timers and itimers.

This series adds infrastructure to the core to handle that cases and converts
the users over to it.

Thanks,

	tglx
---
 fs/timerfd.c               |    2 -
 include/linux/hrtimer.h    |   34 +++++++++++++++++++++++++--
 kernel/time/hrtimer.c      |   55 ++++++++++++++++++++++++++++++---------------
 kernel/time/itimer.c       |    2 -
 kernel/time/posix-timers.c |    2 -
 kernel/time/timer_list.c   |    2 -
 6 files changed, 72 insertions(+), 25 deletions(-)

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

* [patch 0/4] hrtimers: Handle remaining time correctly for CONFIG_TIME_LOW_RES=y
@ 2016-01-14 16:54 ` Thomas Gleixner
  0 siblings, 0 replies; 17+ messages in thread
From: Thomas Gleixner @ 2016-01-14 16:54 UTC (permalink / raw)
  To: LKML
  Cc: Ingo Molnar, Peter Zijlstra, Helge Deller, John Stultz,
	linux-m68k, dhowells

Helge reported, that timerfd returns occasionally remaining time larger than
the relative time which was used to arm the timer. This is caused by the extra
jiffy which we add in hrtimer_start_range_ns() if CONFIG_TIME_LOW_RES=y.

This is not only an issue for timerfd. We have the same problem in
posix-timers and itimers.

This series adds infrastructure to the core to handle that cases and converts
the users over to it.

Thanks,

	tglx
---
 fs/timerfd.c               |    2 -
 include/linux/hrtimer.h    |   34 +++++++++++++++++++++++++--
 kernel/time/hrtimer.c      |   55 ++++++++++++++++++++++++++++++---------------
 kernel/time/itimer.c       |    2 -
 kernel/time/posix-timers.c |    2 -
 kernel/time/timer_list.c   |    2 -
 6 files changed, 72 insertions(+), 25 deletions(-)

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

* [patch 2/4] timerfd: Handle relative timers with CONFIG_TIME_LOW_RES proper
  2016-01-14 16:54 ` Thomas Gleixner
  (?)
  (?)
@ 2016-01-14 16:54 ` Thomas Gleixner
  2016-01-17 10:43   ` [tip:timers/urgent] " tip-bot for Thomas Gleixner
  -1 siblings, 1 reply; 17+ messages in thread
From: Thomas Gleixner @ 2016-01-14 16:54 UTC (permalink / raw)
  To: LKML
  Cc: Ingo Molnar, Peter Zijlstra, Helge Deller, John Stultz,
	linux-m68k, dhowells

[-- Attachment #1: timerfd--Handle-relative-timers-with-CONFIG_TIME_LOW_RES-proper.patch --]
[-- Type: text/plain, Size: 960 bytes --]

Helge reported that a relative timer can return a remaining time larger than
the programmed relative time on parisc and other architectures which have
CONFIG_TIME_LOW_RES set. This happens because we add a jiffie to the resulting
expiry time to prevent short timeouts.

Use the new function hrtimer_expires_remaining_adjusted() to calculate the
remaining time. It takes that extra added time into account for relative
timers.

Reported-by: Helge Deller <deller@gmx.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 fs/timerfd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/fs/timerfd.c
+++ b/fs/timerfd.c
@@ -153,7 +153,7 @@ static ktime_t timerfd_get_remaining(str
 	if (isalarm(ctx))
 		remaining = alarm_expires_remaining(&ctx->t.alarm);
 	else
-		remaining = hrtimer_expires_remaining(&ctx->t.tmr);
+		remaining = hrtimer_expires_remaining_adjusted(&ctx->t.tmr);
 
 	return remaining.tv64 < 0 ? ktime_set(0, 0): remaining;
 }

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

* [patch 1/4] hrtimer: Handle remaining time proper for TIME_LOW_RES
  2016-01-14 16:54 ` Thomas Gleixner
@ 2016-01-14 16:54   ` Thomas Gleixner
  -1 siblings, 0 replies; 17+ messages in thread
From: Thomas Gleixner @ 2016-01-14 16:54 UTC (permalink / raw)
  To: LKML
  Cc: Ingo Molnar, Peter Zijlstra, Helge Deller, John Stultz,
	linux-m68k, dhowells

[-- Attachment #1: hrtimer--Handle-remaining-time-proper-for-TIME_LOW_RES.patch --]
[-- Type: text/plain, Size: 6852 bytes --]

If CONFIG_TIME_LOW_RES is enabled we add a jiffie to the relative timeout to
prevent short sleeps, but we do not account for that in interfaces which
retrieve the remaining time.

Helge observed that timerfd can return a remaining time larger than the
relative timeout. That's not expected and breaks userland test programs.

Store the information that the timer was armed relative and provide functions
to adjust the remaining time. To avoid bloating the hrtimer struct make state
a u8, which as a bonus results in better code on x86 at least.

Reported-by: Helge Deller <deller@gmx.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/hrtimer.h  |   34 ++++++++++++++++++++++++++---
 kernel/time/hrtimer.c    |   55 +++++++++++++++++++++++++++++++----------------
 kernel/time/timer_list.c |    2 -
 3 files changed, 69 insertions(+), 22 deletions(-)

--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -87,7 +87,8 @@ enum hrtimer_restart {
  * @function:	timer expiry callback function
  * @base:	pointer to the timer base (per cpu and per clock)
  * @state:	state information (See bit values above)
- * @start_pid: timer statistics field to store the pid of the task which
+ * @is_rel:	Set if the timer was armed relative
+ * @start_pid:  timer statistics field to store the pid of the task which
  *		started the timer
  * @start_site:	timer statistics field to store the site where the timer
  *		was started
@@ -101,7 +102,8 @@ struct hrtimer {
 	ktime_t				_softexpires;
 	enum hrtimer_restart		(*function)(struct hrtimer *);
 	struct hrtimer_clock_base	*base;
-	unsigned long			state;
+	u8				state;
+	u8				is_rel;
 #ifdef CONFIG_TIMER_STATS
 	int				start_pid;
 	void				*start_site;
@@ -321,6 +323,27 @@ static inline void clock_was_set_delayed
 
 #endif
 
+static inline ktime_t
+__hrtimer_expires_remaining_adjusted(const struct hrtimer *timer, ktime_t now)
+{
+	ktime_t rem = ktime_sub(timer->node.expires, now);
+
+	/*
+	 * Adjust relative timers for the extra we added in
+	 * hrtimer_start_range_ns() to prevent short timeouts.
+	 */
+	if (IS_ENABLED(CONFIG_TIME_LOW_RES) && timer->is_rel)
+		rem.tv64 -= hrtimer_resolution;
+	return rem;
+}
+
+static inline ktime_t
+hrtimer_expires_remaining_adjusted(const struct hrtimer *timer)
+{
+	return __hrtimer_expires_remaining_adjusted(timer,
+						    timer->base->get_time());
+}
+
 extern void clock_was_set(void);
 #ifdef CONFIG_TIMERFD
 extern void timerfd_clock_was_set(void);
@@ -390,7 +413,12 @@ static inline void hrtimer_restart(struc
 }
 
 /* Query timers: */
-extern ktime_t hrtimer_get_remaining(const struct hrtimer *timer);
+extern ktime_t __hrtimer_get_remaining(const struct hrtimer *timer, bool adjust);
+
+static inline ktime_t hrtimer_get_remaining(const struct hrtimer *timer)
+{
+	return __hrtimer_get_remaining(timer, false);
+}
 
 extern u64 hrtimer_get_next_event(void);
 
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -897,10 +897,10 @@ static int enqueue_hrtimer(struct hrtime
  */
 static void __remove_hrtimer(struct hrtimer *timer,
 			     struct hrtimer_clock_base *base,
-			     unsigned long newstate, int reprogram)
+			     u8 newstate, int reprogram)
 {
 	struct hrtimer_cpu_base *cpu_base = base->cpu_base;
-	unsigned int state = timer->state;
+	u8 state = timer->state;
 
 	timer->state = newstate;
 	if (!(state & HRTIMER_STATE_ENQUEUED))
@@ -930,7 +930,7 @@ static inline int
 remove_hrtimer(struct hrtimer *timer, struct hrtimer_clock_base *base, bool restart)
 {
 	if (hrtimer_is_queued(timer)) {
-		unsigned long state = timer->state;
+		u8 state = timer->state;
 		int reprogram;
 
 		/*
@@ -954,6 +954,22 @@ remove_hrtimer(struct hrtimer *timer, st
 	return 0;
 }
 
+static inline ktime_t hrtimer_update_lowres(struct hrtimer *timer, ktime_t tim,
+					    const enum hrtimer_mode mode)
+{
+#ifdef CONFIG_TIME_LOW_RES
+	/*
+	 * CONFIG_TIME_LOW_RES indicates that the system has no way to return
+	 * granular time values. For relative timers we add hrtimer_resolution
+	 * (i.e. one jiffie) to prevent short timeouts.
+	 */
+	timer->is_rel = mode & HRTIMER_MODE_REL;
+	if (timer->is_rel)
+		tim = ktime_add_safe(tim, ktime_set(0, hrtimer_resolution));
+#endif
+	return tim;
+}
+
 /**
  * hrtimer_start_range_ns - (re)start an hrtimer on the current CPU
  * @timer:	the timer to be added
@@ -974,19 +990,10 @@ void hrtimer_start_range_ns(struct hrtim
 	/* Remove an active timer from the queue: */
 	remove_hrtimer(timer, base, true);
 
-	if (mode & HRTIMER_MODE_REL) {
+	if (mode & HRTIMER_MODE_REL)
 		tim = ktime_add_safe(tim, base->get_time());
-		/*
-		 * CONFIG_TIME_LOW_RES is a temporary way for architectures
-		 * to signal that they simply return xtime in
-		 * do_gettimeoffset(). In this case we want to round up by
-		 * resolution when starting a relative timer, to avoid short
-		 * timeouts. This will go away with the GTOD framework.
-		 */
-#ifdef CONFIG_TIME_LOW_RES
-		tim = ktime_add_safe(tim, ktime_set(0, hrtimer_resolution));
-#endif
-	}
+
+	tim = hrtimer_update_lowres(timer, tim, mode);
 
 	hrtimer_set_expires_range_ns(timer, tim, delta_ns);
 
@@ -1074,19 +1081,23 @@ EXPORT_SYMBOL_GPL(hrtimer_cancel);
 /**
  * hrtimer_get_remaining - get remaining time for the timer
  * @timer:	the timer to read
+ * @adjust:	adjust relative timers when CONFIG_TIME_LOW_RES=y
  */
-ktime_t hrtimer_get_remaining(const struct hrtimer *timer)
+ktime_t __hrtimer_get_remaining(const struct hrtimer *timer, bool adjust)
 {
 	unsigned long flags;
 	ktime_t rem;
 
 	lock_hrtimer_base(timer, &flags);
-	rem = hrtimer_expires_remaining(timer);
+	if (IS_ENABLED(CONFIG_TIME_LOW_RES) && adjust)
+		rem = hrtimer_expires_remaining_adjusted(timer);
+	else
+		rem = hrtimer_expires_remaining(timer);
 	unlock_hrtimer_base(timer, &flags);
 
 	return rem;
 }
-EXPORT_SYMBOL_GPL(hrtimer_get_remaining);
+EXPORT_SYMBOL_GPL(__hrtimer_get_remaining);
 
 #ifdef CONFIG_NO_HZ_COMMON
 /**
@@ -1220,6 +1231,14 @@ static void __run_hrtimer(struct hrtimer
 	fn = timer->function;
 
 	/*
+	 * Clear the 'is relative' flag for the TIME_LOW_RES case. If the
+	 * timer is restarted with a period then it becomes an absolute
+	 * timer. If its not restarted it does not matter.
+	 */
+	if (IS_ENABLED(CONFIG_TIME_LOW_RES))
+		timer->is_rel = false;
+
+	/*
 	 * Because we run timers from hardirq context, there is no chance
 	 * they get migrated to another cpu, therefore its safe to unlock
 	 * the timer base.
--- a/kernel/time/timer_list.c
+++ b/kernel/time/timer_list.c
@@ -69,7 +69,7 @@ print_timer(struct seq_file *m, struct h
 	print_name_offset(m, taddr);
 	SEQ_printf(m, ", ");
 	print_name_offset(m, timer->function);
-	SEQ_printf(m, ", S:%02lx", timer->state);
+	SEQ_printf(m, ", S:%02x", timer->state);
 #ifdef CONFIG_TIMER_STATS
 	SEQ_printf(m, ", ");
 	print_name_offset(m, timer->start_site);

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

* [patch 2/4] timerfd: Handle relative timers with CONFIG_TIME_LOW_RES proper
  2016-01-14 16:54 ` Thomas Gleixner
                   ` (2 preceding siblings ...)
  (?)
@ 2016-01-14 16:54 ` Thomas Gleixner
  -1 siblings, 0 replies; 17+ messages in thread
From: Thomas Gleixner @ 2016-01-14 16:54 UTC (permalink / raw)
  To: LKML
  Cc: Ingo Molnar, Peter Zijlstra, Helge Deller, John Stultz,
	linux-m68k, dhowells

[-- Attachment #1: timerfd--Handle-relative-timers-with-CONFIG_TIME_LOW_RES-proper.patch --]
[-- Type: text/plain, Size: 960 bytes --]

Helge reported that a relative timer can return a remaining time larger than
the programmed relative time on parisc and other architectures which have
CONFIG_TIME_LOW_RES set. This happens because we add a jiffie to the resulting
expiry time to prevent short timeouts.

Use the new function hrtimer_expires_remaining_adjusted() to calculate the
remaining time. It takes that extra added time into account for relative
timers.

Reported-by: Helge Deller <deller@gmx.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 fs/timerfd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/fs/timerfd.c
+++ b/fs/timerfd.c
@@ -153,7 +153,7 @@ static ktime_t timerfd_get_remaining(str
 	if (isalarm(ctx))
 		remaining = alarm_expires_remaining(&ctx->t.alarm);
 	else
-		remaining = hrtimer_expires_remaining(&ctx->t.tmr);
+		remaining = hrtimer_expires_remaining_adjusted(&ctx->t.tmr);
 
 	return remaining.tv64 < 0 ? ktime_set(0, 0): remaining;
 }

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

* [patch 1/4] hrtimer: Handle remaining time proper for TIME_LOW_RES
@ 2016-01-14 16:54   ` Thomas Gleixner
  0 siblings, 0 replies; 17+ messages in thread
From: Thomas Gleixner @ 2016-01-14 16:54 UTC (permalink / raw)
  To: LKML
  Cc: Ingo Molnar, Peter Zijlstra, Helge Deller, John Stultz,
	linux-m68k, dhowells

[-- Attachment #1: hrtimer--Handle-remaining-time-proper-for-TIME_LOW_RES.patch --]
[-- Type: text/plain, Size: 6852 bytes --]

If CONFIG_TIME_LOW_RES is enabled we add a jiffie to the relative timeout to
prevent short sleeps, but we do not account for that in interfaces which
retrieve the remaining time.

Helge observed that timerfd can return a remaining time larger than the
relative timeout. That's not expected and breaks userland test programs.

Store the information that the timer was armed relative and provide functions
to adjust the remaining time. To avoid bloating the hrtimer struct make state
a u8, which as a bonus results in better code on x86 at least.

Reported-by: Helge Deller <deller@gmx.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/hrtimer.h  |   34 ++++++++++++++++++++++++++---
 kernel/time/hrtimer.c    |   55 +++++++++++++++++++++++++++++++----------------
 kernel/time/timer_list.c |    2 -
 3 files changed, 69 insertions(+), 22 deletions(-)

--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -87,7 +87,8 @@ enum hrtimer_restart {
  * @function:	timer expiry callback function
  * @base:	pointer to the timer base (per cpu and per clock)
  * @state:	state information (See bit values above)
- * @start_pid: timer statistics field to store the pid of the task which
+ * @is_rel:	Set if the timer was armed relative
+ * @start_pid:  timer statistics field to store the pid of the task which
  *		started the timer
  * @start_site:	timer statistics field to store the site where the timer
  *		was started
@@ -101,7 +102,8 @@ struct hrtimer {
 	ktime_t				_softexpires;
 	enum hrtimer_restart		(*function)(struct hrtimer *);
 	struct hrtimer_clock_base	*base;
-	unsigned long			state;
+	u8				state;
+	u8				is_rel;
 #ifdef CONFIG_TIMER_STATS
 	int				start_pid;
 	void				*start_site;
@@ -321,6 +323,27 @@ static inline void clock_was_set_delayed
 
 #endif
 
+static inline ktime_t
+__hrtimer_expires_remaining_adjusted(const struct hrtimer *timer, ktime_t now)
+{
+	ktime_t rem = ktime_sub(timer->node.expires, now);
+
+	/*
+	 * Adjust relative timers for the extra we added in
+	 * hrtimer_start_range_ns() to prevent short timeouts.
+	 */
+	if (IS_ENABLED(CONFIG_TIME_LOW_RES) && timer->is_rel)
+		rem.tv64 -= hrtimer_resolution;
+	return rem;
+}
+
+static inline ktime_t
+hrtimer_expires_remaining_adjusted(const struct hrtimer *timer)
+{
+	return __hrtimer_expires_remaining_adjusted(timer,
+						    timer->base->get_time());
+}
+
 extern void clock_was_set(void);
 #ifdef CONFIG_TIMERFD
 extern void timerfd_clock_was_set(void);
@@ -390,7 +413,12 @@ static inline void hrtimer_restart(struc
 }
 
 /* Query timers: */
-extern ktime_t hrtimer_get_remaining(const struct hrtimer *timer);
+extern ktime_t __hrtimer_get_remaining(const struct hrtimer *timer, bool adjust);
+
+static inline ktime_t hrtimer_get_remaining(const struct hrtimer *timer)
+{
+	return __hrtimer_get_remaining(timer, false);
+}
 
 extern u64 hrtimer_get_next_event(void);
 
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -897,10 +897,10 @@ static int enqueue_hrtimer(struct hrtime
  */
 static void __remove_hrtimer(struct hrtimer *timer,
 			     struct hrtimer_clock_base *base,
-			     unsigned long newstate, int reprogram)
+			     u8 newstate, int reprogram)
 {
 	struct hrtimer_cpu_base *cpu_base = base->cpu_base;
-	unsigned int state = timer->state;
+	u8 state = timer->state;
 
 	timer->state = newstate;
 	if (!(state & HRTIMER_STATE_ENQUEUED))
@@ -930,7 +930,7 @@ static inline int
 remove_hrtimer(struct hrtimer *timer, struct hrtimer_clock_base *base, bool restart)
 {
 	if (hrtimer_is_queued(timer)) {
-		unsigned long state = timer->state;
+		u8 state = timer->state;
 		int reprogram;
 
 		/*
@@ -954,6 +954,22 @@ remove_hrtimer(struct hrtimer *timer, st
 	return 0;
 }
 
+static inline ktime_t hrtimer_update_lowres(struct hrtimer *timer, ktime_t tim,
+					    const enum hrtimer_mode mode)
+{
+#ifdef CONFIG_TIME_LOW_RES
+	/*
+	 * CONFIG_TIME_LOW_RES indicates that the system has no way to return
+	 * granular time values. For relative timers we add hrtimer_resolution
+	 * (i.e. one jiffie) to prevent short timeouts.
+	 */
+	timer->is_rel = mode & HRTIMER_MODE_REL;
+	if (timer->is_rel)
+		tim = ktime_add_safe(tim, ktime_set(0, hrtimer_resolution));
+#endif
+	return tim;
+}
+
 /**
  * hrtimer_start_range_ns - (re)start an hrtimer on the current CPU
  * @timer:	the timer to be added
@@ -974,19 +990,10 @@ void hrtimer_start_range_ns(struct hrtim
 	/* Remove an active timer from the queue: */
 	remove_hrtimer(timer, base, true);
 
-	if (mode & HRTIMER_MODE_REL) {
+	if (mode & HRTIMER_MODE_REL)
 		tim = ktime_add_safe(tim, base->get_time());
-		/*
-		 * CONFIG_TIME_LOW_RES is a temporary way for architectures
-		 * to signal that they simply return xtime in
-		 * do_gettimeoffset(). In this case we want to round up by
-		 * resolution when starting a relative timer, to avoid short
-		 * timeouts. This will go away with the GTOD framework.
-		 */
-#ifdef CONFIG_TIME_LOW_RES
-		tim = ktime_add_safe(tim, ktime_set(0, hrtimer_resolution));
-#endif
-	}
+
+	tim = hrtimer_update_lowres(timer, tim, mode);
 
 	hrtimer_set_expires_range_ns(timer, tim, delta_ns);
 
@@ -1074,19 +1081,23 @@ EXPORT_SYMBOL_GPL(hrtimer_cancel);
 /**
  * hrtimer_get_remaining - get remaining time for the timer
  * @timer:	the timer to read
+ * @adjust:	adjust relative timers when CONFIG_TIME_LOW_RES=y
  */
-ktime_t hrtimer_get_remaining(const struct hrtimer *timer)
+ktime_t __hrtimer_get_remaining(const struct hrtimer *timer, bool adjust)
 {
 	unsigned long flags;
 	ktime_t rem;
 
 	lock_hrtimer_base(timer, &flags);
-	rem = hrtimer_expires_remaining(timer);
+	if (IS_ENABLED(CONFIG_TIME_LOW_RES) && adjust)
+		rem = hrtimer_expires_remaining_adjusted(timer);
+	else
+		rem = hrtimer_expires_remaining(timer);
 	unlock_hrtimer_base(timer, &flags);
 
 	return rem;
 }
-EXPORT_SYMBOL_GPL(hrtimer_get_remaining);
+EXPORT_SYMBOL_GPL(__hrtimer_get_remaining);
 
 #ifdef CONFIG_NO_HZ_COMMON
 /**
@@ -1220,6 +1231,14 @@ static void __run_hrtimer(struct hrtimer
 	fn = timer->function;
 
 	/*
+	 * Clear the 'is relative' flag for the TIME_LOW_RES case. If the
+	 * timer is restarted with a period then it becomes an absolute
+	 * timer. If its not restarted it does not matter.
+	 */
+	if (IS_ENABLED(CONFIG_TIME_LOW_RES))
+		timer->is_rel = false;
+
+	/*
 	 * Because we run timers from hardirq context, there is no chance
 	 * they get migrated to another cpu, therefore its safe to unlock
 	 * the timer base.
--- a/kernel/time/timer_list.c
+++ b/kernel/time/timer_list.c
@@ -69,7 +69,7 @@ print_timer(struct seq_file *m, struct h
 	print_name_offset(m, taddr);
 	SEQ_printf(m, ", ");
 	print_name_offset(m, timer->function);
-	SEQ_printf(m, ", S:%02lx", timer->state);
+	SEQ_printf(m, ", S:%02x", timer->state);
 #ifdef CONFIG_TIMER_STATS
 	SEQ_printf(m, ", ");
 	print_name_offset(m, timer->start_site);

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

* [patch 3/4] posix-timers: Handle relative timers with CONFIG_TIME_LOW_RES proper
  2016-01-14 16:54 ` Thomas Gleixner
@ 2016-01-14 16:54   ` Thomas Gleixner
  -1 siblings, 0 replies; 17+ messages in thread
From: Thomas Gleixner @ 2016-01-14 16:54 UTC (permalink / raw)
  To: LKML
  Cc: Ingo Molnar, Peter Zijlstra, Helge Deller, John Stultz,
	linux-m68k, dhowells

[-- Attachment #1: posix-timers--Handle-relative-timers-with-CONFIG_TIME_LOW_RES-proper.patch --]
[-- Type: text/plain, Size: 897 bytes --]

As Helge reported for timerfd we have the same issue in posix timers. We
return remaining time larger than the programmed relative time to user space
in case of CONFIG_TIME_LOW_RES=y. Use the proper function to adjust the extra
time added in hrtimer_start_range_ns().

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/posix-timers.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -760,7 +760,7 @@ common_timer_get(struct k_itimer *timr,
 	    (timr->it_sigev_notify & ~SIGEV_THREAD_ID) == SIGEV_NONE))
 		timr->it_overrun += (unsigned int) hrtimer_forward(timer, now, iv);
 
-	remaining = ktime_sub(hrtimer_get_expires(timer), now);
+	remaining = __hrtimer_expires_remaining_adjusted(timer, now);
 	/* Return 0 only, when the timer is expired and not pending */
 	if (remaining.tv64 <= 0) {
 		/*

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

* [patch 3/4] posix-timers: Handle relative timers with CONFIG_TIME_LOW_RES proper
@ 2016-01-14 16:54   ` Thomas Gleixner
  0 siblings, 0 replies; 17+ messages in thread
From: Thomas Gleixner @ 2016-01-14 16:54 UTC (permalink / raw)
  To: LKML
  Cc: Ingo Molnar, Peter Zijlstra, Helge Deller, John Stultz,
	linux-m68k, dhowells

[-- Attachment #1: posix-timers--Handle-relative-timers-with-CONFIG_TIME_LOW_RES-proper.patch --]
[-- Type: text/plain, Size: 897 bytes --]

As Helge reported for timerfd we have the same issue in posix timers. We
return remaining time larger than the programmed relative time to user space
in case of CONFIG_TIME_LOW_RES=y. Use the proper function to adjust the extra
time added in hrtimer_start_range_ns().

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/posix-timers.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -760,7 +760,7 @@ common_timer_get(struct k_itimer *timr,
 	    (timr->it_sigev_notify & ~SIGEV_THREAD_ID) == SIGEV_NONE))
 		timr->it_overrun += (unsigned int) hrtimer_forward(timer, now, iv);
 
-	remaining = ktime_sub(hrtimer_get_expires(timer), now);
+	remaining = __hrtimer_expires_remaining_adjusted(timer, now);
 	/* Return 0 only, when the timer is expired and not pending */
 	if (remaining.tv64 <= 0) {
 		/*

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

* [patch 4/4] itimers: Handle relative timers with CONFIG_TIME_LOW_RES proper
  2016-01-14 16:54 ` Thomas Gleixner
                   ` (5 preceding siblings ...)
  (?)
@ 2016-01-14 16:54 ` Thomas Gleixner
  2016-01-17 10:43   ` [tip:timers/urgent] " tip-bot for Thomas Gleixner
  -1 siblings, 1 reply; 17+ messages in thread
From: Thomas Gleixner @ 2016-01-14 16:54 UTC (permalink / raw)
  To: LKML
  Cc: Ingo Molnar, Peter Zijlstra, Helge Deller, John Stultz,
	linux-m68k, dhowells

[-- Attachment #1: itimers--Handle-relative-timers-with-CONFIG_TIME_LOW_RES-proper.patch --]
[-- Type: text/plain, Size: 710 bytes --]

As Helge reported for timerfd we have the same issue in itimers. We return
remaining time larger than the programmed relative time to user space in case
of CONFIG_TIME_LOW_RES=y. Use the proper function to adjust the extra time
added in hrtimer_start_range_ns().

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/itimer.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/kernel/time/itimer.c
+++ b/kernel/time/itimer.c
@@ -26,7 +26,7 @@
  */
 static struct timeval itimer_get_remtime(struct hrtimer *timer)
 {
-	ktime_t rem = hrtimer_get_remaining(timer);
+	ktime_t rem = __hrtimer_get_remaining(timer, true);
 
 	/*
 	 * Racy but safe: if the itimer expires after the above

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

* [patch 4/4] itimers: Handle relative timers with CONFIG_TIME_LOW_RES proper
  2016-01-14 16:54 ` Thomas Gleixner
                   ` (4 preceding siblings ...)
  (?)
@ 2016-01-14 16:54 ` Thomas Gleixner
  -1 siblings, 0 replies; 17+ messages in thread
From: Thomas Gleixner @ 2016-01-14 16:54 UTC (permalink / raw)
  To: LKML
  Cc: Ingo Molnar, Peter Zijlstra, Helge Deller, John Stultz,
	linux-m68k, dhowells

[-- Attachment #1: itimers--Handle-relative-timers-with-CONFIG_TIME_LOW_RES-proper.patch --]
[-- Type: text/plain, Size: 710 bytes --]

As Helge reported for timerfd we have the same issue in itimers. We return
remaining time larger than the programmed relative time to user space in case
of CONFIG_TIME_LOW_RES=y. Use the proper function to adjust the extra time
added in hrtimer_start_range_ns().

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/itimer.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/kernel/time/itimer.c
+++ b/kernel/time/itimer.c
@@ -26,7 +26,7 @@
  */
 static struct timeval itimer_get_remtime(struct hrtimer *timer)
 {
-	ktime_t rem = hrtimer_get_remaining(timer);
+	ktime_t rem = __hrtimer_get_remaining(timer, true);
 
 	/*
 	 * Racy but safe: if the itimer expires after the above

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

* Re: [patch 1/4] hrtimer: Handle remaining time proper for TIME_LOW_RES
  2016-01-14 16:54   ` Thomas Gleixner
@ 2016-01-16 18:36     ` Helge Deller
  -1 siblings, 0 replies; 17+ messages in thread
From: Helge Deller @ 2016-01-16 18:36 UTC (permalink / raw)
  To: Thomas Gleixner, linux-parisc, John David Anglin
  Cc: LKML, Ingo Molnar, Peter Zijlstra, Helge Deller, John Stultz,
	linux-m68k, dhowells


Hi Thomas,

* Thomas Gleixner <tglx@linutronix.de>:
> If CONFIG_TIME_LOW_RES is enabled we add a jiffie to the relative timeout to
> prevent short sleeps, but we do not account for that in interfaces which
> retrieve the remaining time.
> 
> Helge observed that timerfd can return a remaining time larger than the
> relative timeout. That's not expected and breaks userland test programs.
> 
> Store the information that the timer was armed relative and provide functions
> to adjust the remaining time. To avoid bloating the hrtimer struct make state
> a u8, which as a bonus results in better code on x86 at least.
> 
> Reported-by: Helge Deller <deller@gmx.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Great!
You can add for this series:
Tested-by: Helge Deller <deller@gmx.de>

Any chance it can get backported to v4.4 ?

Thanks,
Helge

> ---
>  include/linux/hrtimer.h  |   34 ++++++++++++++++++++++++++---
>  kernel/time/hrtimer.c    |   55 +++++++++++++++++++++++++++++++----------------
>  kernel/time/timer_list.c |    2 -
>  3 files changed, 69 insertions(+), 22 deletions(-)
> 
> --- a/include/linux/hrtimer.h
> +++ b/include/linux/hrtimer.h
> @@ -87,7 +87,8 @@ enum hrtimer_restart {
>   * @function:	timer expiry callback function
>   * @base:	pointer to the timer base (per cpu and per clock)
>   * @state:	state information (See bit values above)
> - * @start_pid: timer statistics field to store the pid of the task which
> + * @is_rel:	Set if the timer was armed relative
> + * @start_pid:  timer statistics field to store the pid of the task which
>   *		started the timer
>   * @start_site:	timer statistics field to store the site where the timer
>   *		was started
> @@ -101,7 +102,8 @@ struct hrtimer {
>  	ktime_t				_softexpires;
>  	enum hrtimer_restart		(*function)(struct hrtimer *);
>  	struct hrtimer_clock_base	*base;
> -	unsigned long			state;
> +	u8				state;
> +	u8				is_rel;
>  #ifdef CONFIG_TIMER_STATS
>  	int				start_pid;
>  	void				*start_site;
> @@ -321,6 +323,27 @@ static inline void clock_was_set_delayed
>  
>  #endif
>  
> +static inline ktime_t
> +__hrtimer_expires_remaining_adjusted(const struct hrtimer *timer, ktime_t now)
> +{
> +	ktime_t rem = ktime_sub(timer->node.expires, now);
> +
> +	/*
> +	 * Adjust relative timers for the extra we added in
> +	 * hrtimer_start_range_ns() to prevent short timeouts.
> +	 */
> +	if (IS_ENABLED(CONFIG_TIME_LOW_RES) && timer->is_rel)
> +		rem.tv64 -= hrtimer_resolution;
> +	return rem;
> +}
> +
> +static inline ktime_t
> +hrtimer_expires_remaining_adjusted(const struct hrtimer *timer)
> +{
> +	return __hrtimer_expires_remaining_adjusted(timer,
> +						    timer->base->get_time());
> +}
> +
>  extern void clock_was_set(void);
>  #ifdef CONFIG_TIMERFD
>  extern void timerfd_clock_was_set(void);
> @@ -390,7 +413,12 @@ static inline void hrtimer_restart(struc
>  }
>  
>  /* Query timers: */
> -extern ktime_t hrtimer_get_remaining(const struct hrtimer *timer);
> +extern ktime_t __hrtimer_get_remaining(const struct hrtimer *timer, bool adjust);
> +
> +static inline ktime_t hrtimer_get_remaining(const struct hrtimer *timer)
> +{
> +	return __hrtimer_get_remaining(timer, false);
> +}
>  
>  extern u64 hrtimer_get_next_event(void);
>  
> --- a/kernel/time/hrtimer.c
> +++ b/kernel/time/hrtimer.c
> @@ -897,10 +897,10 @@ static int enqueue_hrtimer(struct hrtime
>   */
>  static void __remove_hrtimer(struct hrtimer *timer,
>  			     struct hrtimer_clock_base *base,
> -			     unsigned long newstate, int reprogram)
> +			     u8 newstate, int reprogram)
>  {
>  	struct hrtimer_cpu_base *cpu_base = base->cpu_base;
> -	unsigned int state = timer->state;
> +	u8 state = timer->state;
>  
>  	timer->state = newstate;
>  	if (!(state & HRTIMER_STATE_ENQUEUED))
> @@ -930,7 +930,7 @@ static inline int
>  remove_hrtimer(struct hrtimer *timer, struct hrtimer_clock_base *base, bool restart)
>  {
>  	if (hrtimer_is_queued(timer)) {
> -		unsigned long state = timer->state;
> +		u8 state = timer->state;
>  		int reprogram;
>  
>  		/*
> @@ -954,6 +954,22 @@ remove_hrtimer(struct hrtimer *timer, st
>  	return 0;
>  }
>  
> +static inline ktime_t hrtimer_update_lowres(struct hrtimer *timer, ktime_t tim,
> +					    const enum hrtimer_mode mode)
> +{
> +#ifdef CONFIG_TIME_LOW_RES
> +	/*
> +	 * CONFIG_TIME_LOW_RES indicates that the system has no way to return
> +	 * granular time values. For relative timers we add hrtimer_resolution
> +	 * (i.e. one jiffie) to prevent short timeouts.
> +	 */
> +	timer->is_rel = mode & HRTIMER_MODE_REL;
> +	if (timer->is_rel)
> +		tim = ktime_add_safe(tim, ktime_set(0, hrtimer_resolution));
> +#endif
> +	return tim;
> +}
> +
>  /**
>   * hrtimer_start_range_ns - (re)start an hrtimer on the current CPU
>   * @timer:	the timer to be added
> @@ -974,19 +990,10 @@ void hrtimer_start_range_ns(struct hrtim
>  	/* Remove an active timer from the queue: */
>  	remove_hrtimer(timer, base, true);
>  
> -	if (mode & HRTIMER_MODE_REL) {
> +	if (mode & HRTIMER_MODE_REL)
>  		tim = ktime_add_safe(tim, base->get_time());
> -		/*
> -		 * CONFIG_TIME_LOW_RES is a temporary way for architectures
> -		 * to signal that they simply return xtime in
> -		 * do_gettimeoffset(). In this case we want to round up by
> -		 * resolution when starting a relative timer, to avoid short
> -		 * timeouts. This will go away with the GTOD framework.
> -		 */
> -#ifdef CONFIG_TIME_LOW_RES
> -		tim = ktime_add_safe(tim, ktime_set(0, hrtimer_resolution));
> -#endif
> -	}
> +
> +	tim = hrtimer_update_lowres(timer, tim, mode);
>  
>  	hrtimer_set_expires_range_ns(timer, tim, delta_ns);
>  
> @@ -1074,19 +1081,23 @@ EXPORT_SYMBOL_GPL(hrtimer_cancel);
>  /**
>   * hrtimer_get_remaining - get remaining time for the timer
>   * @timer:	the timer to read
> + * @adjust:	adjust relative timers when CONFIG_TIME_LOW_RES=y
>   */
> -ktime_t hrtimer_get_remaining(const struct hrtimer *timer)
> +ktime_t __hrtimer_get_remaining(const struct hrtimer *timer, bool adjust)
>  {
>  	unsigned long flags;
>  	ktime_t rem;
>  
>  	lock_hrtimer_base(timer, &flags);
> -	rem = hrtimer_expires_remaining(timer);
> +	if (IS_ENABLED(CONFIG_TIME_LOW_RES) && adjust)
> +		rem = hrtimer_expires_remaining_adjusted(timer);
> +	else
> +		rem = hrtimer_expires_remaining(timer);
>  	unlock_hrtimer_base(timer, &flags);
>  
>  	return rem;
>  }
> -EXPORT_SYMBOL_GPL(hrtimer_get_remaining);
> +EXPORT_SYMBOL_GPL(__hrtimer_get_remaining);
>  
>  #ifdef CONFIG_NO_HZ_COMMON
>  /**
> @@ -1220,6 +1231,14 @@ static void __run_hrtimer(struct hrtimer
>  	fn = timer->function;
>  
>  	/*
> +	 * Clear the 'is relative' flag for the TIME_LOW_RES case. If the
> +	 * timer is restarted with a period then it becomes an absolute
> +	 * timer. If its not restarted it does not matter.
> +	 */
> +	if (IS_ENABLED(CONFIG_TIME_LOW_RES))
> +		timer->is_rel = false;
> +
> +	/*
>  	 * Because we run timers from hardirq context, there is no chance
>  	 * they get migrated to another cpu, therefore its safe to unlock
>  	 * the timer base.
> --- a/kernel/time/timer_list.c
> +++ b/kernel/time/timer_list.c
> @@ -69,7 +69,7 @@ print_timer(struct seq_file *m, struct h
>  	print_name_offset(m, taddr);
>  	SEQ_printf(m, ", ");
>  	print_name_offset(m, timer->function);
> -	SEQ_printf(m, ", S:%02lx", timer->state);
> +	SEQ_printf(m, ", S:%02x", timer->state);
>  #ifdef CONFIG_TIMER_STATS
>  	SEQ_printf(m, ", ");
>  	print_name_offset(m, timer->start_site);
> 
> 

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

* Re: [patch 1/4] hrtimer: Handle remaining time proper for TIME_LOW_RES
@ 2016-01-16 18:36     ` Helge Deller
  0 siblings, 0 replies; 17+ messages in thread
From: Helge Deller @ 2016-01-16 18:36 UTC (permalink / raw)
  To: Thomas Gleixner, linux-parisc, John David Anglin
  Cc: LKML, Ingo Molnar, Peter Zijlstra, Helge Deller, John Stultz,
	linux-m68k, dhowells


Hi Thomas,

* Thomas Gleixner <tglx@linutronix.de>:
> If CONFIG_TIME_LOW_RES is enabled we add a jiffie to the relative timeout to
> prevent short sleeps, but we do not account for that in interfaces which
> retrieve the remaining time.
> 
> Helge observed that timerfd can return a remaining time larger than the
> relative timeout. That's not expected and breaks userland test programs.
> 
> Store the information that the timer was armed relative and provide functions
> to adjust the remaining time. To avoid bloating the hrtimer struct make state
> a u8, which as a bonus results in better code on x86 at least.
> 
> Reported-by: Helge Deller <deller@gmx.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Great!
You can add for this series:
Tested-by: Helge Deller <deller@gmx.de>

Any chance it can get backported to v4.4 ?

Thanks,
Helge

> ---
>  include/linux/hrtimer.h  |   34 ++++++++++++++++++++++++++---
>  kernel/time/hrtimer.c    |   55 +++++++++++++++++++++++++++++++----------------
>  kernel/time/timer_list.c |    2 -
>  3 files changed, 69 insertions(+), 22 deletions(-)
> 
> --- a/include/linux/hrtimer.h
> +++ b/include/linux/hrtimer.h
> @@ -87,7 +87,8 @@ enum hrtimer_restart {
>   * @function:	timer expiry callback function
>   * @base:	pointer to the timer base (per cpu and per clock)
>   * @state:	state information (See bit values above)
> - * @start_pid: timer statistics field to store the pid of the task which
> + * @is_rel:	Set if the timer was armed relative
> + * @start_pid:  timer statistics field to store the pid of the task which
>   *		started the timer
>   * @start_site:	timer statistics field to store the site where the timer
>   *		was started
> @@ -101,7 +102,8 @@ struct hrtimer {
>  	ktime_t				_softexpires;
>  	enum hrtimer_restart		(*function)(struct hrtimer *);
>  	struct hrtimer_clock_base	*base;
> -	unsigned long			state;
> +	u8				state;
> +	u8				is_rel;
>  #ifdef CONFIG_TIMER_STATS
>  	int				start_pid;
>  	void				*start_site;
> @@ -321,6 +323,27 @@ static inline void clock_was_set_delayed
>  
>  #endif
>  
> +static inline ktime_t
> +__hrtimer_expires_remaining_adjusted(const struct hrtimer *timer, ktime_t now)
> +{
> +	ktime_t rem = ktime_sub(timer->node.expires, now);
> +
> +	/*
> +	 * Adjust relative timers for the extra we added in
> +	 * hrtimer_start_range_ns() to prevent short timeouts.
> +	 */
> +	if (IS_ENABLED(CONFIG_TIME_LOW_RES) && timer->is_rel)
> +		rem.tv64 -= hrtimer_resolution;
> +	return rem;
> +}
> +
> +static inline ktime_t
> +hrtimer_expires_remaining_adjusted(const struct hrtimer *timer)
> +{
> +	return __hrtimer_expires_remaining_adjusted(timer,
> +						    timer->base->get_time());
> +}
> +
>  extern void clock_was_set(void);
>  #ifdef CONFIG_TIMERFD
>  extern void timerfd_clock_was_set(void);
> @@ -390,7 +413,12 @@ static inline void hrtimer_restart(struc
>  }
>  
>  /* Query timers: */
> -extern ktime_t hrtimer_get_remaining(const struct hrtimer *timer);
> +extern ktime_t __hrtimer_get_remaining(const struct hrtimer *timer, bool adjust);
> +
> +static inline ktime_t hrtimer_get_remaining(const struct hrtimer *timer)
> +{
> +	return __hrtimer_get_remaining(timer, false);
> +}
>  
>  extern u64 hrtimer_get_next_event(void);
>  
> --- a/kernel/time/hrtimer.c
> +++ b/kernel/time/hrtimer.c
> @@ -897,10 +897,10 @@ static int enqueue_hrtimer(struct hrtime
>   */
>  static void __remove_hrtimer(struct hrtimer *timer,
>  			     struct hrtimer_clock_base *base,
> -			     unsigned long newstate, int reprogram)
> +			     u8 newstate, int reprogram)
>  {
>  	struct hrtimer_cpu_base *cpu_base = base->cpu_base;
> -	unsigned int state = timer->state;
> +	u8 state = timer->state;
>  
>  	timer->state = newstate;
>  	if (!(state & HRTIMER_STATE_ENQUEUED))
> @@ -930,7 +930,7 @@ static inline int
>  remove_hrtimer(struct hrtimer *timer, struct hrtimer_clock_base *base, bool restart)
>  {
>  	if (hrtimer_is_queued(timer)) {
> -		unsigned long state = timer->state;
> +		u8 state = timer->state;
>  		int reprogram;
>  
>  		/*
> @@ -954,6 +954,22 @@ remove_hrtimer(struct hrtimer *timer, st
>  	return 0;
>  }
>  
> +static inline ktime_t hrtimer_update_lowres(struct hrtimer *timer, ktime_t tim,
> +					    const enum hrtimer_mode mode)
> +{
> +#ifdef CONFIG_TIME_LOW_RES
> +	/*
> +	 * CONFIG_TIME_LOW_RES indicates that the system has no way to return
> +	 * granular time values. For relative timers we add hrtimer_resolution
> +	 * (i.e. one jiffie) to prevent short timeouts.
> +	 */
> +	timer->is_rel = mode & HRTIMER_MODE_REL;
> +	if (timer->is_rel)
> +		tim = ktime_add_safe(tim, ktime_set(0, hrtimer_resolution));
> +#endif
> +	return tim;
> +}
> +
>  /**
>   * hrtimer_start_range_ns - (re)start an hrtimer on the current CPU
>   * @timer:	the timer to be added
> @@ -974,19 +990,10 @@ void hrtimer_start_range_ns(struct hrtim
>  	/* Remove an active timer from the queue: */
>  	remove_hrtimer(timer, base, true);
>  
> -	if (mode & HRTIMER_MODE_REL) {
> +	if (mode & HRTIMER_MODE_REL)
>  		tim = ktime_add_safe(tim, base->get_time());
> -		/*
> -		 * CONFIG_TIME_LOW_RES is a temporary way for architectures
> -		 * to signal that they simply return xtime in
> -		 * do_gettimeoffset(). In this case we want to round up by
> -		 * resolution when starting a relative timer, to avoid short
> -		 * timeouts. This will go away with the GTOD framework.
> -		 */
> -#ifdef CONFIG_TIME_LOW_RES
> -		tim = ktime_add_safe(tim, ktime_set(0, hrtimer_resolution));
> -#endif
> -	}
> +
> +	tim = hrtimer_update_lowres(timer, tim, mode);
>  
>  	hrtimer_set_expires_range_ns(timer, tim, delta_ns);
>  
> @@ -1074,19 +1081,23 @@ EXPORT_SYMBOL_GPL(hrtimer_cancel);
>  /**
>   * hrtimer_get_remaining - get remaining time for the timer
>   * @timer:	the timer to read
> + * @adjust:	adjust relative timers when CONFIG_TIME_LOW_RES=y
>   */
> -ktime_t hrtimer_get_remaining(const struct hrtimer *timer)
> +ktime_t __hrtimer_get_remaining(const struct hrtimer *timer, bool adjust)
>  {
>  	unsigned long flags;
>  	ktime_t rem;
>  
>  	lock_hrtimer_base(timer, &flags);
> -	rem = hrtimer_expires_remaining(timer);
> +	if (IS_ENABLED(CONFIG_TIME_LOW_RES) && adjust)
> +		rem = hrtimer_expires_remaining_adjusted(timer);
> +	else
> +		rem = hrtimer_expires_remaining(timer);
>  	unlock_hrtimer_base(timer, &flags);
>  
>  	return rem;
>  }
> -EXPORT_SYMBOL_GPL(hrtimer_get_remaining);
> +EXPORT_SYMBOL_GPL(__hrtimer_get_remaining);
>  
>  #ifdef CONFIG_NO_HZ_COMMON
>  /**
> @@ -1220,6 +1231,14 @@ static void __run_hrtimer(struct hrtimer
>  	fn = timer->function;
>  
>  	/*
> +	 * Clear the 'is relative' flag for the TIME_LOW_RES case. If the
> +	 * timer is restarted with a period then it becomes an absolute
> +	 * timer. If its not restarted it does not matter.
> +	 */
> +	if (IS_ENABLED(CONFIG_TIME_LOW_RES))
> +		timer->is_rel = false;
> +
> +	/*
>  	 * Because we run timers from hardirq context, there is no chance
>  	 * they get migrated to another cpu, therefore its safe to unlock
>  	 * the timer base.
> --- a/kernel/time/timer_list.c
> +++ b/kernel/time/timer_list.c
> @@ -69,7 +69,7 @@ print_timer(struct seq_file *m, struct h
>  	print_name_offset(m, taddr);
>  	SEQ_printf(m, ", ");
>  	print_name_offset(m, timer->function);
> -	SEQ_printf(m, ", S:%02lx", timer->state);
> +	SEQ_printf(m, ", S:%02x", timer->state);
>  #ifdef CONFIG_TIMER_STATS
>  	SEQ_printf(m, ", ");
>  	print_name_offset(m, timer->start_site);
> 
> 

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

* Re: [patch 1/4] hrtimer: Handle remaining time proper for TIME_LOW_RES
  2016-01-14 16:54   ` Thomas Gleixner
  (?)
  (?)
@ 2016-01-16 18:36   ` Helge Deller
  -1 siblings, 0 replies; 17+ messages in thread
From: Helge Deller @ 2016-01-16 18:36 UTC (permalink / raw)
  To: Thomas Gleixner, linux-parisc, John David Anglin
  Cc: LKML, Ingo Molnar, Peter Zijlstra, Helge Deller, John Stultz,
	linux-m68k, dhowells


Hi Thomas,

* Thomas Gleixner <tglx@linutronix.de>:
> If CONFIG_TIME_LOW_RES is enabled we add a jiffie to the relative timeout to
> prevent short sleeps, but we do not account for that in interfaces which
> retrieve the remaining time.
> 
> Helge observed that timerfd can return a remaining time larger than the
> relative timeout. That's not expected and breaks userland test programs.
> 
> Store the information that the timer was armed relative and provide functions
> to adjust the remaining time. To avoid bloating the hrtimer struct make state
> a u8, which as a bonus results in better code on x86 at least.
> 
> Reported-by: Helge Deller <deller@gmx.de>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Great!
You can add for this series:
Tested-by: Helge Deller <deller@gmx.de>

Any chance it can get backported to v4.4 ?

Thanks,
Helge

> ---
>  include/linux/hrtimer.h  |   34 ++++++++++++++++++++++++++---
>  kernel/time/hrtimer.c    |   55 +++++++++++++++++++++++++++++++----------------
>  kernel/time/timer_list.c |    2 -
>  3 files changed, 69 insertions(+), 22 deletions(-)
> 
> --- a/include/linux/hrtimer.h
> +++ b/include/linux/hrtimer.h
> @@ -87,7 +87,8 @@ enum hrtimer_restart {
>   * @function:	timer expiry callback function
>   * @base:	pointer to the timer base (per cpu and per clock)
>   * @state:	state information (See bit values above)
> - * @start_pid: timer statistics field to store the pid of the task which
> + * @is_rel:	Set if the timer was armed relative
> + * @start_pid:  timer statistics field to store the pid of the task which
>   *		started the timer
>   * @start_site:	timer statistics field to store the site where the timer
>   *		was started
> @@ -101,7 +102,8 @@ struct hrtimer {
>  	ktime_t				_softexpires;
>  	enum hrtimer_restart		(*function)(struct hrtimer *);
>  	struct hrtimer_clock_base	*base;
> -	unsigned long			state;
> +	u8				state;
> +	u8				is_rel;
>  #ifdef CONFIG_TIMER_STATS
>  	int				start_pid;
>  	void				*start_site;
> @@ -321,6 +323,27 @@ static inline void clock_was_set_delayed
>  
>  #endif
>  
> +static inline ktime_t
> +__hrtimer_expires_remaining_adjusted(const struct hrtimer *timer, ktime_t now)
> +{
> +	ktime_t rem = ktime_sub(timer->node.expires, now);
> +
> +	/*
> +	 * Adjust relative timers for the extra we added in
> +	 * hrtimer_start_range_ns() to prevent short timeouts.
> +	 */
> +	if (IS_ENABLED(CONFIG_TIME_LOW_RES) && timer->is_rel)
> +		rem.tv64 -= hrtimer_resolution;
> +	return rem;
> +}
> +
> +static inline ktime_t
> +hrtimer_expires_remaining_adjusted(const struct hrtimer *timer)
> +{
> +	return __hrtimer_expires_remaining_adjusted(timer,
> +						    timer->base->get_time());
> +}
> +
>  extern void clock_was_set(void);
>  #ifdef CONFIG_TIMERFD
>  extern void timerfd_clock_was_set(void);
> @@ -390,7 +413,12 @@ static inline void hrtimer_restart(struc
>  }
>  
>  /* Query timers: */
> -extern ktime_t hrtimer_get_remaining(const struct hrtimer *timer);
> +extern ktime_t __hrtimer_get_remaining(const struct hrtimer *timer, bool adjust);
> +
> +static inline ktime_t hrtimer_get_remaining(const struct hrtimer *timer)
> +{
> +	return __hrtimer_get_remaining(timer, false);
> +}
>  
>  extern u64 hrtimer_get_next_event(void);
>  
> --- a/kernel/time/hrtimer.c
> +++ b/kernel/time/hrtimer.c
> @@ -897,10 +897,10 @@ static int enqueue_hrtimer(struct hrtime
>   */
>  static void __remove_hrtimer(struct hrtimer *timer,
>  			     struct hrtimer_clock_base *base,
> -			     unsigned long newstate, int reprogram)
> +			     u8 newstate, int reprogram)
>  {
>  	struct hrtimer_cpu_base *cpu_base = base->cpu_base;
> -	unsigned int state = timer->state;
> +	u8 state = timer->state;
>  
>  	timer->state = newstate;
>  	if (!(state & HRTIMER_STATE_ENQUEUED))
> @@ -930,7 +930,7 @@ static inline int
>  remove_hrtimer(struct hrtimer *timer, struct hrtimer_clock_base *base, bool restart)
>  {
>  	if (hrtimer_is_queued(timer)) {
> -		unsigned long state = timer->state;
> +		u8 state = timer->state;
>  		int reprogram;
>  
>  		/*
> @@ -954,6 +954,22 @@ remove_hrtimer(struct hrtimer *timer, st
>  	return 0;
>  }
>  
> +static inline ktime_t hrtimer_update_lowres(struct hrtimer *timer, ktime_t tim,
> +					    const enum hrtimer_mode mode)
> +{
> +#ifdef CONFIG_TIME_LOW_RES
> +	/*
> +	 * CONFIG_TIME_LOW_RES indicates that the system has no way to return
> +	 * granular time values. For relative timers we add hrtimer_resolution
> +	 * (i.e. one jiffie) to prevent short timeouts.
> +	 */
> +	timer->is_rel = mode & HRTIMER_MODE_REL;
> +	if (timer->is_rel)
> +		tim = ktime_add_safe(tim, ktime_set(0, hrtimer_resolution));
> +#endif
> +	return tim;
> +}
> +
>  /**
>   * hrtimer_start_range_ns - (re)start an hrtimer on the current CPU
>   * @timer:	the timer to be added
> @@ -974,19 +990,10 @@ void hrtimer_start_range_ns(struct hrtim
>  	/* Remove an active timer from the queue: */
>  	remove_hrtimer(timer, base, true);
>  
> -	if (mode & HRTIMER_MODE_REL) {
> +	if (mode & HRTIMER_MODE_REL)
>  		tim = ktime_add_safe(tim, base->get_time());
> -		/*
> -		 * CONFIG_TIME_LOW_RES is a temporary way for architectures
> -		 * to signal that they simply return xtime in
> -		 * do_gettimeoffset(). In this case we want to round up by
> -		 * resolution when starting a relative timer, to avoid short
> -		 * timeouts. This will go away with the GTOD framework.
> -		 */
> -#ifdef CONFIG_TIME_LOW_RES
> -		tim = ktime_add_safe(tim, ktime_set(0, hrtimer_resolution));
> -#endif
> -	}
> +
> +	tim = hrtimer_update_lowres(timer, tim, mode);
>  
>  	hrtimer_set_expires_range_ns(timer, tim, delta_ns);
>  
> @@ -1074,19 +1081,23 @@ EXPORT_SYMBOL_GPL(hrtimer_cancel);
>  /**
>   * hrtimer_get_remaining - get remaining time for the timer
>   * @timer:	the timer to read
> + * @adjust:	adjust relative timers when CONFIG_TIME_LOW_RES=y
>   */
> -ktime_t hrtimer_get_remaining(const struct hrtimer *timer)
> +ktime_t __hrtimer_get_remaining(const struct hrtimer *timer, bool adjust)
>  {
>  	unsigned long flags;
>  	ktime_t rem;
>  
>  	lock_hrtimer_base(timer, &flags);
> -	rem = hrtimer_expires_remaining(timer);
> +	if (IS_ENABLED(CONFIG_TIME_LOW_RES) && adjust)
> +		rem = hrtimer_expires_remaining_adjusted(timer);
> +	else
> +		rem = hrtimer_expires_remaining(timer);
>  	unlock_hrtimer_base(timer, &flags);
>  
>  	return rem;
>  }
> -EXPORT_SYMBOL_GPL(hrtimer_get_remaining);
> +EXPORT_SYMBOL_GPL(__hrtimer_get_remaining);
>  
>  #ifdef CONFIG_NO_HZ_COMMON
>  /**
> @@ -1220,6 +1231,14 @@ static void __run_hrtimer(struct hrtimer
>  	fn = timer->function;
>  
>  	/*
> +	 * Clear the 'is relative' flag for the TIME_LOW_RES case. If the
> +	 * timer is restarted with a period then it becomes an absolute
> +	 * timer. If its not restarted it does not matter.
> +	 */
> +	if (IS_ENABLED(CONFIG_TIME_LOW_RES))
> +		timer->is_rel = false;
> +
> +	/*
>  	 * Because we run timers from hardirq context, there is no chance
>  	 * they get migrated to another cpu, therefore its safe to unlock
>  	 * the timer base.
> --- a/kernel/time/timer_list.c
> +++ b/kernel/time/timer_list.c
> @@ -69,7 +69,7 @@ print_timer(struct seq_file *m, struct h
>  	print_name_offset(m, taddr);
>  	SEQ_printf(m, ", ");
>  	print_name_offset(m, timer->function);
> -	SEQ_printf(m, ", S:%02lx", timer->state);
> +	SEQ_printf(m, ", S:%02x", timer->state);
>  #ifdef CONFIG_TIMER_STATS
>  	SEQ_printf(m, ", ");
>  	print_name_offset(m, timer->start_site);
> 
> 

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

* [tip:timers/urgent] hrtimer: Handle remaining time proper for TIME_LOW_RES
  2016-01-14 16:54   ` Thomas Gleixner
                     ` (2 preceding siblings ...)
  (?)
@ 2016-01-17 10:42   ` tip-bot for Thomas Gleixner
  -1 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Thomas Gleixner @ 2016-01-17 10:42 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: john.stultz, linux-kernel, hpa, deller, mingo, tglx, peterz

Commit-ID:  203cbf77de59fc8f13502dcfd11350c6d4a5c95f
Gitweb:     http://git.kernel.org/tip/203cbf77de59fc8f13502dcfd11350c6d4a5c95f
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 14 Jan 2016 16:54:46 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 17 Jan 2016 11:13:55 +0100

hrtimer: Handle remaining time proper for TIME_LOW_RES

If CONFIG_TIME_LOW_RES is enabled we add a jiffie to the relative timeout to
prevent short sleeps, but we do not account for that in interfaces which
retrieve the remaining time.

Helge observed that timerfd can return a remaining time larger than the
relative timeout. That's not expected and breaks userland test programs.

Store the information that the timer was armed relative and provide functions
to adjust the remaining time. To avoid bloating the hrtimer struct make state
a u8, which as a bonus results in better code on x86 at least.

Reported-and-tested-by: Helge Deller <deller@gmx.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: John Stultz <john.stultz@linaro.org>
Cc: linux-m68k@lists.linux-m68k.org
Cc: dhowells@redhat.com
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/20160114164159.273328486@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 include/linux/hrtimer.h  | 34 +++++++++++++++++++++++++++---
 kernel/time/hrtimer.c    | 55 ++++++++++++++++++++++++++++++++----------------
 kernel/time/timer_list.c |  2 +-
 3 files changed, 69 insertions(+), 22 deletions(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index 76dd4f0..2ead22d 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -87,7 +87,8 @@ enum hrtimer_restart {
  * @function:	timer expiry callback function
  * @base:	pointer to the timer base (per cpu and per clock)
  * @state:	state information (See bit values above)
- * @start_pid: timer statistics field to store the pid of the task which
+ * @is_rel:	Set if the timer was armed relative
+ * @start_pid:  timer statistics field to store the pid of the task which
  *		started the timer
  * @start_site:	timer statistics field to store the site where the timer
  *		was started
@@ -101,7 +102,8 @@ struct hrtimer {
 	ktime_t				_softexpires;
 	enum hrtimer_restart		(*function)(struct hrtimer *);
 	struct hrtimer_clock_base	*base;
-	unsigned long			state;
+	u8				state;
+	u8				is_rel;
 #ifdef CONFIG_TIMER_STATS
 	int				start_pid;
 	void				*start_site;
@@ -321,6 +323,27 @@ static inline void clock_was_set_delayed(void) { }
 
 #endif
 
+static inline ktime_t
+__hrtimer_expires_remaining_adjusted(const struct hrtimer *timer, ktime_t now)
+{
+	ktime_t rem = ktime_sub(timer->node.expires, now);
+
+	/*
+	 * Adjust relative timers for the extra we added in
+	 * hrtimer_start_range_ns() to prevent short timeouts.
+	 */
+	if (IS_ENABLED(CONFIG_TIME_LOW_RES) && timer->is_rel)
+		rem.tv64 -= hrtimer_resolution;
+	return rem;
+}
+
+static inline ktime_t
+hrtimer_expires_remaining_adjusted(const struct hrtimer *timer)
+{
+	return __hrtimer_expires_remaining_adjusted(timer,
+						    timer->base->get_time());
+}
+
 extern void clock_was_set(void);
 #ifdef CONFIG_TIMERFD
 extern void timerfd_clock_was_set(void);
@@ -390,7 +413,12 @@ static inline void hrtimer_restart(struct hrtimer *timer)
 }
 
 /* Query timers: */
-extern ktime_t hrtimer_get_remaining(const struct hrtimer *timer);
+extern ktime_t __hrtimer_get_remaining(const struct hrtimer *timer, bool adjust);
+
+static inline ktime_t hrtimer_get_remaining(const struct hrtimer *timer)
+{
+	return __hrtimer_get_remaining(timer, false);
+}
 
 extern u64 hrtimer_get_next_event(void);
 
diff --git a/kernel/time/hrtimer.c b/kernel/time/hrtimer.c
index 435b885..fa909f9 100644
--- a/kernel/time/hrtimer.c
+++ b/kernel/time/hrtimer.c
@@ -897,10 +897,10 @@ static int enqueue_hrtimer(struct hrtimer *timer,
  */
 static void __remove_hrtimer(struct hrtimer *timer,
 			     struct hrtimer_clock_base *base,
-			     unsigned long newstate, int reprogram)
+			     u8 newstate, int reprogram)
 {
 	struct hrtimer_cpu_base *cpu_base = base->cpu_base;
-	unsigned int state = timer->state;
+	u8 state = timer->state;
 
 	timer->state = newstate;
 	if (!(state & HRTIMER_STATE_ENQUEUED))
@@ -930,7 +930,7 @@ static inline int
 remove_hrtimer(struct hrtimer *timer, struct hrtimer_clock_base *base, bool restart)
 {
 	if (hrtimer_is_queued(timer)) {
-		unsigned long state = timer->state;
+		u8 state = timer->state;
 		int reprogram;
 
 		/*
@@ -954,6 +954,22 @@ remove_hrtimer(struct hrtimer *timer, struct hrtimer_clock_base *base, bool rest
 	return 0;
 }
 
+static inline ktime_t hrtimer_update_lowres(struct hrtimer *timer, ktime_t tim,
+					    const enum hrtimer_mode mode)
+{
+#ifdef CONFIG_TIME_LOW_RES
+	/*
+	 * CONFIG_TIME_LOW_RES indicates that the system has no way to return
+	 * granular time values. For relative timers we add hrtimer_resolution
+	 * (i.e. one jiffie) to prevent short timeouts.
+	 */
+	timer->is_rel = mode & HRTIMER_MODE_REL;
+	if (timer->is_rel)
+		tim = ktime_add_safe(tim, ktime_set(0, hrtimer_resolution));
+#endif
+	return tim;
+}
+
 /**
  * hrtimer_start_range_ns - (re)start an hrtimer on the current CPU
  * @timer:	the timer to be added
@@ -974,19 +990,10 @@ void hrtimer_start_range_ns(struct hrtimer *timer, ktime_t tim,
 	/* Remove an active timer from the queue: */
 	remove_hrtimer(timer, base, true);
 
-	if (mode & HRTIMER_MODE_REL) {
+	if (mode & HRTIMER_MODE_REL)
 		tim = ktime_add_safe(tim, base->get_time());
-		/*
-		 * CONFIG_TIME_LOW_RES is a temporary way for architectures
-		 * to signal that they simply return xtime in
-		 * do_gettimeoffset(). In this case we want to round up by
-		 * resolution when starting a relative timer, to avoid short
-		 * timeouts. This will go away with the GTOD framework.
-		 */
-#ifdef CONFIG_TIME_LOW_RES
-		tim = ktime_add_safe(tim, ktime_set(0, hrtimer_resolution));
-#endif
-	}
+
+	tim = hrtimer_update_lowres(timer, tim, mode);
 
 	hrtimer_set_expires_range_ns(timer, tim, delta_ns);
 
@@ -1074,19 +1081,23 @@ EXPORT_SYMBOL_GPL(hrtimer_cancel);
 /**
  * hrtimer_get_remaining - get remaining time for the timer
  * @timer:	the timer to read
+ * @adjust:	adjust relative timers when CONFIG_TIME_LOW_RES=y
  */
-ktime_t hrtimer_get_remaining(const struct hrtimer *timer)
+ktime_t __hrtimer_get_remaining(const struct hrtimer *timer, bool adjust)
 {
 	unsigned long flags;
 	ktime_t rem;
 
 	lock_hrtimer_base(timer, &flags);
-	rem = hrtimer_expires_remaining(timer);
+	if (IS_ENABLED(CONFIG_TIME_LOW_RES) && adjust)
+		rem = hrtimer_expires_remaining_adjusted(timer);
+	else
+		rem = hrtimer_expires_remaining(timer);
 	unlock_hrtimer_base(timer, &flags);
 
 	return rem;
 }
-EXPORT_SYMBOL_GPL(hrtimer_get_remaining);
+EXPORT_SYMBOL_GPL(__hrtimer_get_remaining);
 
 #ifdef CONFIG_NO_HZ_COMMON
 /**
@@ -1220,6 +1231,14 @@ static void __run_hrtimer(struct hrtimer_cpu_base *cpu_base,
 	fn = timer->function;
 
 	/*
+	 * Clear the 'is relative' flag for the TIME_LOW_RES case. If the
+	 * timer is restarted with a period then it becomes an absolute
+	 * timer. If its not restarted it does not matter.
+	 */
+	if (IS_ENABLED(CONFIG_TIME_LOW_RES))
+		timer->is_rel = false;
+
+	/*
 	 * Because we run timers from hardirq context, there is no chance
 	 * they get migrated to another cpu, therefore its safe to unlock
 	 * the timer base.
diff --git a/kernel/time/timer_list.c b/kernel/time/timer_list.c
index f75e35b..ba7d8b2 100644
--- a/kernel/time/timer_list.c
+++ b/kernel/time/timer_list.c
@@ -69,7 +69,7 @@ print_timer(struct seq_file *m, struct hrtimer *taddr, struct hrtimer *timer,
 	print_name_offset(m, taddr);
 	SEQ_printf(m, ", ");
 	print_name_offset(m, timer->function);
-	SEQ_printf(m, ", S:%02lx", timer->state);
+	SEQ_printf(m, ", S:%02x", timer->state);
 #ifdef CONFIG_TIMER_STATS
 	SEQ_printf(m, ", ");
 	print_name_offset(m, timer->start_site);

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

* [tip:timers/urgent] timerfd: Handle relative timers with CONFIG_TIME_LOW_RES proper
  2016-01-14 16:54 ` [patch 2/4] timerfd: Handle relative timers with CONFIG_TIME_LOW_RES proper Thomas Gleixner
@ 2016-01-17 10:43   ` tip-bot for Thomas Gleixner
  0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Thomas Gleixner @ 2016-01-17 10:43 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: john.stultz, peterz, tglx, linux-kernel, mingo, deller, hpa

Commit-ID:  b62526ed11a1fe3861ab98d40b7fdab8981d788a
Gitweb:     http://git.kernel.org/tip/b62526ed11a1fe3861ab98d40b7fdab8981d788a
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 14 Jan 2016 16:54:46 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 17 Jan 2016 11:13:55 +0100

timerfd: Handle relative timers with CONFIG_TIME_LOW_RES proper

Helge reported that a relative timer can return a remaining time larger than
the programmed relative time on parisc and other architectures which have
CONFIG_TIME_LOW_RES set. This happens because we add a jiffie to the resulting
expiry time to prevent short timeouts.

Use the new function hrtimer_expires_remaining_adjusted() to calculate the
remaining time. It takes that extra added time into account for relative
timers.

Reported-and-tested-by: Helge Deller <deller@gmx.de>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: John Stultz <john.stultz@linaro.org>
Cc: linux-m68k@lists.linux-m68k.org
Cc: dhowells@redhat.com
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/20160114164159.354500742@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 fs/timerfd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/timerfd.c b/fs/timerfd.c
index b94fa6c..053818d 100644
--- a/fs/timerfd.c
+++ b/fs/timerfd.c
@@ -153,7 +153,7 @@ static ktime_t timerfd_get_remaining(struct timerfd_ctx *ctx)
 	if (isalarm(ctx))
 		remaining = alarm_expires_remaining(&ctx->t.alarm);
 	else
-		remaining = hrtimer_expires_remaining(&ctx->t.tmr);
+		remaining = hrtimer_expires_remaining_adjusted(&ctx->t.tmr);
 
 	return remaining.tv64 < 0 ? ktime_set(0, 0): remaining;
 }

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

* [tip:timers/urgent] posix-timers: Handle relative timers with CONFIG_TIME_LOW_RES proper
  2016-01-14 16:54   ` Thomas Gleixner
  (?)
@ 2016-01-17 10:43   ` tip-bot for Thomas Gleixner
  -1 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Thomas Gleixner @ 2016-01-17 10:43 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: deller, peterz, linux-kernel, john.stultz, mingo, tglx, hpa

Commit-ID:  572c39172684c3711e4a03c9a7380067e2b0661c
Gitweb:     http://git.kernel.org/tip/572c39172684c3711e4a03c9a7380067e2b0661c
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 14 Jan 2016 16:54:47 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 17 Jan 2016 11:13:55 +0100

posix-timers: Handle relative timers with CONFIG_TIME_LOW_RES proper

As Helge reported for timerfd we have the same issue in posix timers. We
return remaining time larger than the programmed relative time to user space
in case of CONFIG_TIME_LOW_RES=y. Use the proper function to adjust the extra
time added in hrtimer_start_range_ns().

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Helge Deller <deller@gmx.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: linux-m68k@lists.linux-m68k.org
Cc: dhowells@redhat.com
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/20160114164159.450510905@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/posix-timers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/posix-timers.c b/kernel/time/posix-timers.c
index 31d11ac..f2826c3 100644
--- a/kernel/time/posix-timers.c
+++ b/kernel/time/posix-timers.c
@@ -760,7 +760,7 @@ common_timer_get(struct k_itimer *timr, struct itimerspec *cur_setting)
 	    (timr->it_sigev_notify & ~SIGEV_THREAD_ID) == SIGEV_NONE))
 		timr->it_overrun += (unsigned int) hrtimer_forward(timer, now, iv);
 
-	remaining = ktime_sub(hrtimer_get_expires(timer), now);
+	remaining = __hrtimer_expires_remaining_adjusted(timer, now);
 	/* Return 0 only, when the timer is expired and not pending */
 	if (remaining.tv64 <= 0) {
 		/*

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

* [tip:timers/urgent] itimers: Handle relative timers with CONFIG_TIME_LOW_RES proper
  2016-01-14 16:54 ` Thomas Gleixner
@ 2016-01-17 10:43   ` tip-bot for Thomas Gleixner
  0 siblings, 0 replies; 17+ messages in thread
From: tip-bot for Thomas Gleixner @ 2016-01-17 10:43 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: peterz, hpa, tglx, deller, mingo, john.stultz, linux-kernel

Commit-ID:  51cbb5242a41700a3f250ecfb48dcfb7e4375ea4
Gitweb:     http://git.kernel.org/tip/51cbb5242a41700a3f250ecfb48dcfb7e4375ea4
Author:     Thomas Gleixner <tglx@linutronix.de>
AuthorDate: Thu, 14 Jan 2016 16:54:48 +0000
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sun, 17 Jan 2016 11:13:55 +0100

itimers: Handle relative timers with CONFIG_TIME_LOW_RES proper

As Helge reported for timerfd we have the same issue in itimers. We return
remaining time larger than the programmed relative time to user space in case
of CONFIG_TIME_LOW_RES=y. Use the proper function to adjust the extra time
added in hrtimer_start_range_ns().

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Helge Deller <deller@gmx.de>
Cc: John Stultz <john.stultz@linaro.org>
Cc: linux-m68k@lists.linux-m68k.org
Cc: dhowells@redhat.com
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/20160114164159.528222587@linutronix.de
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/time/itimer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/time/itimer.c b/kernel/time/itimer.c
index 8d262b4..1d5c720 100644
--- a/kernel/time/itimer.c
+++ b/kernel/time/itimer.c
@@ -26,7 +26,7 @@
  */
 static struct timeval itimer_get_remtime(struct hrtimer *timer)
 {
-	ktime_t rem = hrtimer_get_remaining(timer);
+	ktime_t rem = __hrtimer_get_remaining(timer, true);
 
 	/*
 	 * Racy but safe: if the itimer expires after the above

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

end of thread, other threads:[~2016-01-17 10:44 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-14 16:54 [patch 0/4] hrtimers: Handle remaining time correctly for CONFIG_TIME_LOW_RES=y Thomas Gleixner
2016-01-14 16:54 ` Thomas Gleixner
2016-01-14 16:54 ` [patch 1/4] hrtimer: Handle remaining time proper for TIME_LOW_RES Thomas Gleixner
2016-01-14 16:54   ` Thomas Gleixner
2016-01-16 18:36   ` Helge Deller
2016-01-16 18:36     ` Helge Deller
2016-01-16 18:36   ` Helge Deller
2016-01-17 10:42   ` [tip:timers/urgent] " tip-bot for Thomas Gleixner
2016-01-14 16:54 ` [patch 2/4] timerfd: Handle relative timers with CONFIG_TIME_LOW_RES proper Thomas Gleixner
2016-01-17 10:43   ` [tip:timers/urgent] " tip-bot for Thomas Gleixner
2016-01-14 16:54 ` [patch 2/4] " Thomas Gleixner
2016-01-14 16:54 ` [patch 3/4] posix-timers: " Thomas Gleixner
2016-01-14 16:54   ` Thomas Gleixner
2016-01-17 10:43   ` [tip:timers/urgent] " tip-bot for Thomas Gleixner
2016-01-14 16:54 ` [patch 4/4] itimers: " Thomas Gleixner
2016-01-14 16:54 ` Thomas Gleixner
2016-01-17 10:43   ` [tip:timers/urgent] " 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.