All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/6] Deferrable timers support for hrtimers/timerfd API
@ 2014-02-20  8:40 Alexey Perevalov
  2014-02-20  8:40 ` [PATCH v3 1/6] tracing/trivial: Add CLOCK_BOOTIME and CLOCK_TAI for human readable clockid trace Alexey Perevalov
                   ` (6 more replies)
  0 siblings, 7 replies; 25+ messages in thread
From: Alexey Perevalov @ 2014-02-20  8:40 UTC (permalink / raw)
  To: linux-kernel, tglx, john.stultz
  Cc: Alexey Perevalov, anton, kyungmin.park, cw00.choi, akpm

Hello.

This is a combo patch set for support defferability in timerfd.
Due implementation of timerfd is based on hrtimers and only on hrtimers,
it was necessary to add such deferrability into hrtimers too.

It brings benefits for user-land application which don't want to break idle state and for
in-kernel users who want to use hrtimer advantages. 

Alexey Perevalov (2):
  tracing/trivial: Add CLOCK_BOOTIME and CLOCK_TAI for human readable
    clockid trace
  tracing/trivial: Add CLOCK_*_DEFERRABLE for tracing clockids

Anton Vorontsov (3):
  kernel/time: Add new helpers to convert ktime to/from jiffies
  timerfd: Factor out timer-type unspecific timerfd_expire()
  timerfd: Add support for deferrable timers

Thomas Gleixner (1):
  hrtimer: Add support for deferrable timer into the hrtimer

 fs/timerfd.c                 |   47 ++++++++++++++++----------------
 include/linux/hrtimer.h      |    3 ++
 include/linux/jiffies.h      |    4 ++-
 include/linux/ktime.h        |    3 +-
 include/trace/events/timer.h |   11 +++++++-
 include/uapi/linux/time.h    |    3 ++
 kernel/hrtimer.c             |   62 +++++++++++++++++++++++++++++++++---------
 kernel/time.c                |   23 ++++++++++++++++
 8 files changed, 117 insertions(+), 39 deletions(-)

-- 
1.7.9.5


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

* [PATCH v3 1/6] tracing/trivial: Add CLOCK_BOOTIME and CLOCK_TAI for human readable clockid trace
  2014-02-20  8:40 [PATCH v3 0/6] Deferrable timers support for hrtimers/timerfd API Alexey Perevalov
@ 2014-02-20  8:40 ` Alexey Perevalov
  2014-02-20 11:01   ` Thomas Gleixner
  2014-02-20  8:40 ` [PATCH v3 2/6] hrtimer: Add support for deferrable timer into the hrtimer Alexey Perevalov
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: Alexey Perevalov @ 2014-02-20  8:40 UTC (permalink / raw)
  To: linux-kernel, tglx, john.stultz
  Cc: Alexey Perevalov, anton, kyungmin.park, cw00.choi, akpm

These clockids is also used in current hrtimer implementation.

Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
 include/trace/events/timer.h |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/trace/events/timer.h b/include/trace/events/timer.h
index 185b2c6..547b79f 100644
--- a/include/trace/events/timer.h
+++ b/include/trace/events/timer.h
@@ -11,7 +11,10 @@
 #define clockid_to_string(clockid)						\
 	__print_symbolic(clockid,						\
 			 { CLOCK_MONOTONIC,		"CLOCK_MONOTONIC" },	\
-			 { CLOCK_REALTIME,		"CLOCK_REALTIME" })
+			 { CLOCK_REALTIME,		"CLOCK_REALTIME" },	\
+			 { CLOCK_BOOTTIME,		"CLOCK_BOOTTIME" },	\
+			 { CLOCK_TAI,			"CLOCK_TAI" }		\
+	)
 
 #define hrmode_to_string(hrmode)						\
 	__print_symbolic(hrmode,						\
-- 
1.7.9.5


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

* [PATCH v3 2/6] hrtimer: Add support for deferrable timer into the hrtimer
  2014-02-20  8:40 [PATCH v3 0/6] Deferrable timers support for hrtimers/timerfd API Alexey Perevalov
  2014-02-20  8:40 ` [PATCH v3 1/6] tracing/trivial: Add CLOCK_BOOTIME and CLOCK_TAI for human readable clockid trace Alexey Perevalov
@ 2014-02-20  8:40 ` Alexey Perevalov
  2014-02-20 18:49   ` John Stultz
  2014-02-20  8:40 ` [PATCH v3 3/6] kernel/time: Add new helpers to convert ktime to/from jiffies Alexey Perevalov
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: Alexey Perevalov @ 2014-02-20  8:40 UTC (permalink / raw)
  To: linux-kernel, tglx, john.stultz
  Cc: anton, kyungmin.park, cw00.choi, akpm, Alexey Perevalov

From: Thomas Gleixner <tglx@linutronix.de>

This patch introduces new public CLOCKID constants for
user space API, such as timerfd. It extends hrtimer API and makes
possible to have unified interfaces where deferreble functionality is
used. In-kernel users such as device drivers could find benefits too.

High resolution timer now could work with CLOCK_REALTIME_DEFERRABLE,
CLOCK_MONOTONIC_DEFERRABLE, CLOCK_BOOTTIME_DEFERRABLE.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
 include/linux/hrtimer.h   |    3 +++
 include/uapi/linux/time.h |    3 +++
 kernel/hrtimer.c          |   62 +++++++++++++++++++++++++++++++++++----------
 3 files changed, 55 insertions(+), 13 deletions(-)

diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
index d19a5c2..fe1159c 100644
--- a/include/linux/hrtimer.h
+++ b/include/linux/hrtimer.h
@@ -158,6 +158,9 @@ enum  hrtimer_base_type {
 	HRTIMER_BASE_REALTIME,
 	HRTIMER_BASE_BOOTTIME,
 	HRTIMER_BASE_TAI,
+	HRTIMER_BASE_MONOTONIC_DEFERRABLE,
+	HRTIMER_BASE_REALTIME_DEFERRABLE,
+	HRTIMER_BASE_BOOTTIME_DEFERRABLE,
 	HRTIMER_MAX_CLOCK_BASES,
 };
 
diff --git a/include/uapi/linux/time.h b/include/uapi/linux/time.h
index e75e1b6..bb8dc60 100644
--- a/include/uapi/linux/time.h
+++ b/include/uapi/linux/time.h
@@ -56,6 +56,9 @@ struct itimerval {
 #define CLOCK_BOOTTIME_ALARM		9
 #define CLOCK_SGI_CYCLE			10	/* Hardware specific */
 #define CLOCK_TAI			11
+#define CLOCK_REALTIME_DEFERRABLE	12
+#define CLOCK_MONOTONIC_DEFERRABLE	13
+#define CLOCK_BOOTTIME_DEFERRABLE	14
 
 #define MAX_CLOCKS			16
 #define CLOCKS_MASK			(CLOCK_REALTIME | CLOCK_MONOTONIC)
diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
index 0909436..d1478fc 100644
--- a/kernel/hrtimer.c
+++ b/kernel/hrtimer.c
@@ -92,14 +92,35 @@ DEFINE_PER_CPU(struct hrtimer_cpu_base, hrtimer_bases) =
 			.get_time = &ktime_get_clocktai,
 			.resolution = KTIME_LOW_RES,
 		},
+		{
+			.index = HRTIMER_BASE_MONOTONIC_DEFERRABLE,
+			.clockid = CLOCK_MONOTONIC_DEFERRABLE,
+			.get_time = &ktime_get,
+			.resolution = KTIME_LOW_RES,
+		},
+		{
+			.index = HRTIMER_BASE_REALTIME_DEFERRABLE,
+			.clockid = CLOCK_REALTIME_DEFERRABLE,
+			.get_time = &ktime_get_real,
+			.resolution = KTIME_LOW_RES,
+		},
+		{
+			.index = HRTIMER_BASE_BOOTTIME_DEFERRABLE,
+			.clockid = CLOCK_BOOTTIME_DEFERRABLE,
+			.get_time = &ktime_get_boottime,
+			.resolution = KTIME_LOW_RES,
+		},
 	}
 };
 
 static const int hrtimer_clock_to_base_table[MAX_CLOCKS] = {
-	[CLOCK_REALTIME]	= HRTIMER_BASE_REALTIME,
-	[CLOCK_MONOTONIC]	= HRTIMER_BASE_MONOTONIC,
-	[CLOCK_BOOTTIME]	= HRTIMER_BASE_BOOTTIME,
-	[CLOCK_TAI]		= HRTIMER_BASE_TAI,
+	[CLOCK_REALTIME]		= HRTIMER_BASE_REALTIME,
+	[CLOCK_MONOTONIC]		= HRTIMER_BASE_MONOTONIC,
+	[CLOCK_BOOTTIME]		= HRTIMER_BASE_BOOTTIME,
+	[CLOCK_TAI]			= HRTIMER_BASE_TAI,
+	[CLOCK_REALTIME_DEFERRABLE]	= HRTIMER_BASE_REALTIME_DEFERRABLE,
+	[CLOCK_MONOTONIC_DEFERRABLE]	= HRTIMER_BASE_MONOTONIC_DEFERRABLE,
+	[CLOCK_BOOTTIME_DEFERRABLE]	= HRTIMER_BASE_BOOTTIME_DEFERRABLE,
 };
 
 static inline int hrtimer_clockid_to_base(clockid_t clock_id)
@@ -194,7 +215,8 @@ hrtimer_check_target(struct hrtimer *timer, struct hrtimer_clock_base *new_base)
 #ifdef CONFIG_HIGH_RES_TIMERS
 	ktime_t expires;
 
-	if (!new_base->cpu_base->hres_active)
+	if (!new_base->cpu_base->hres_active ||
+	    new_base->index >= HRTIMER_BASE_MONOTONIC_DEFERRABLE)
 		return 0;
 
 	expires = ktime_sub(hrtimer_get_expires(timer), new_base->offset);
@@ -556,7 +578,7 @@ hrtimer_force_reprogram(struct hrtimer_cpu_base *cpu_base, int skip_equal)
 
 	expires_next.tv64 = KTIME_MAX;
 
-	for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++, base++) {
+	for (i = 0; i < HRTIMER_BASE_MONOTONIC_DEFERRABLE; i++, base++) {
 		struct hrtimer *timer;
 		struct timerqueue_node *next;
 
@@ -615,6 +637,13 @@ static int hrtimer_reprogram(struct hrtimer *timer,
 		return 0;
 
 	/*
+	 * Deferrable timers are not touching the underlying
+	 * hardware.
+	 */
+	if (base->index >= HRTIMER_BASE_MONOTONIC_DEFERRABLE)
+		return 0;
+
+	/*
 	 * CLOCK_REALTIME timer might be requested with an absolute
 	 * expiry time which is less than base->offset. Nothing wrong
 	 * about that, just avoid to call into the tick code, which
@@ -924,7 +953,10 @@ static void __remove_hrtimer(struct hrtimer *timer,
 
 			expires = ktime_sub(hrtimer_get_expires(timer),
 					    base->offset);
-			if (base->cpu_base->expires_next.tv64 == expires.tv64)
+
+			/* We only care about non deferrable timers here */
+			if (base->index <= HRTIMER_BASE_MONOTONIC_DEFERRABLE &&
+			    base->cpu_base->expires_next.tv64 == expires.tv64)
 				hrtimer_force_reprogram(base->cpu_base, 1);
 		}
 #endif
@@ -1152,7 +1184,7 @@ ktime_t hrtimer_get_next_event(void)
 	raw_spin_lock_irqsave(&cpu_base->lock, flags);
 
 	if (!hrtimer_hres_active()) {
-		for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++, base++) {
+		for (i = 0; i < HRTIMER_BASE_MONOTONIC_DEFERRABLE; i++, base++) {
 			struct hrtimer *timer;
 			struct timerqueue_node *next;
 
@@ -1284,7 +1316,8 @@ void hrtimer_interrupt(struct clock_event_device *dev)
 {
 	struct hrtimer_cpu_base *cpu_base = &__get_cpu_var(hrtimer_bases);
 	ktime_t expires_next, now, entry_time, delta;
-	int i, retries = 0;
+	unsigned long bases;
+	int retries = 0;
 
 	BUG_ON(!cpu_base->hres_active);
 	cpu_base->nr_events++;
@@ -1302,14 +1335,16 @@ retry:
 	 * this CPU.
 	 */
 	cpu_base->expires_next.tv64 = KTIME_MAX;
+	bases = cpu_base->active_bases;
 
-	for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++) {
+	while (bases) {
 		struct hrtimer_clock_base *base;
 		struct timerqueue_node *node;
 		ktime_t basenow;
+		int i;
 
-		if (!(cpu_base->active_bases & (1 << i)))
-			continue;
+		i = __ffs(bases);
+		bases &= ~(1 << i);
 
 		base = cpu_base->clock_base + i;
 		basenow = ktime_add(now, base->offset);
@@ -1339,7 +1374,8 @@ retry:
 						    base->offset);
 				if (expires.tv64 < 0)
 					expires.tv64 = KTIME_MAX;
-				if (expires.tv64 < expires_next.tv64)
+				if (expires.tv64 < expires_next.tv64 &&
+				    base->index <= HRTIMER_BASE_MONOTONIC_DEFERRABLE)
 					expires_next = expires;
 				break;
 			}
-- 
1.7.9.5


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

* [PATCH v3 3/6] kernel/time: Add new helpers to convert ktime to/from jiffies
  2014-02-20  8:40 [PATCH v3 0/6] Deferrable timers support for hrtimers/timerfd API Alexey Perevalov
  2014-02-20  8:40 ` [PATCH v3 1/6] tracing/trivial: Add CLOCK_BOOTIME and CLOCK_TAI for human readable clockid trace Alexey Perevalov
  2014-02-20  8:40 ` [PATCH v3 2/6] hrtimer: Add support for deferrable timer into the hrtimer Alexey Perevalov
@ 2014-02-20  8:40 ` Alexey Perevalov
  2014-02-20 10:34   ` Thomas Gleixner
  2014-02-20  8:40 ` [PATCH v3 4/6] timerfd: Factor out timer-type unspecific timerfd_expire() Alexey Perevalov
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: Alexey Perevalov @ 2014-02-20  8:40 UTC (permalink / raw)
  To: linux-kernel, tglx, john.stultz
  Cc: Anton Vorontsov, anton, kyungmin.park, cw00.choi, akpm,
	Anton Vorontsov, Alexey Perevalov

From: Anton Vorontsov <anton@scarybugs.org>

Two new functions: jiffies_to_ktime() and ktime_to_jiffies(), we'll use
them for timerfd deferred timers handling.

We fully reuse the logic from timespec implementations, so the functions
are pretty straightforward.

The only tricky part is in headers: we have to include jiffies.h after
we defined ktime_t, this is because ktime.h needs some declarations from
jiffies.h (e.g. TICK_NSEC).

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
 include/linux/jiffies.h |    4 +++-
 include/linux/ktime.h   |    3 ++-
 kernel/time.c           |   23 +++++++++++++++++++++++
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/include/linux/jiffies.h b/include/linux/jiffies.h
index 1f44466..cbb15e0 100644
--- a/include/linux/jiffies.h
+++ b/include/linux/jiffies.h
@@ -6,6 +6,7 @@
 #include <linux/types.h>
 #include <linux/time.h>
 #include <linux/timex.h>
+#include <linux/ktime.h>
 #include <asm/param.h>			/* for HZ */
 
 /*
@@ -308,7 +309,8 @@ extern void jiffies_to_timespec(const unsigned long jiffies,
 extern unsigned long timeval_to_jiffies(const struct timeval *value);
 extern void jiffies_to_timeval(const unsigned long jiffies,
 			       struct timeval *value);
-
+extern unsigned long ktime_to_jiffies(ktime_t *value);
+extern void jiffies_to_ktime(const unsigned long jiffies, ktime_t *value);
 extern clock_t jiffies_to_clock_t(unsigned long x);
 static inline clock_t jiffies_delta_to_clock_t(long delta)
 {
diff --git a/include/linux/ktime.h b/include/linux/ktime.h
index 31c0cd1..e8ed619 100644
--- a/include/linux/ktime.h
+++ b/include/linux/ktime.h
@@ -22,7 +22,6 @@
 #define _LINUX_KTIME_H
 
 #include <linux/time.h>
-#include <linux/jiffies.h>
 
 /*
  * ktime_t:
@@ -58,6 +57,8 @@ union ktime {
 
 typedef union ktime ktime_t;		/* Kill this */
 
+#include <linux/jiffies.h>
+
 /*
  * ktime_t definitions when using the 64-bit scalar representation:
  */
diff --git a/kernel/time.c b/kernel/time.c
index 7c7964c..22580a0 100644
--- a/kernel/time.c
+++ b/kernel/time.c
@@ -29,6 +29,7 @@
 
 #include <linux/export.h>
 #include <linux/timex.h>
+#include <linux/ktime.h>
 #include <linux/capability.h>
 #include <linux/timekeeper_internal.h>
 #include <linux/errno.h>
@@ -575,6 +576,28 @@ void jiffies_to_timeval(const unsigned long jiffies, struct timeval *value)
 }
 EXPORT_SYMBOL(jiffies_to_timeval);
 
+unsigned long ktime_to_jiffies(ktime_t *value)
+{
+	struct timespec ts = ktime_to_timespec(*value);
+
+	/*
+	 * nsecs_to_jiffies(ktime_to_ns(*ktime)) is unsafe as nsecs_to_jiffies
+	 * doesn't handle MAX_JIFFY_OFFSET. So we reuse the logic from the
+	 * timespec to jiffies conversion function.
+	 */
+	return timespec_to_jiffies(&ts);
+}
+EXPORT_SYMBOL(ktime_to_jiffies);
+
+void jiffies_to_ktime(const unsigned long jiffies, ktime_t *value)
+{
+	struct timespec ts;
+
+	jiffies_to_timespec(jiffies, &ts);
+	*value = timespec_to_ktime(ts);
+}
+EXPORT_SYMBOL(jiffies_to_ktime);
+
 /*
  * Convert jiffies/jiffies_64 to clock_t and back.
  */
-- 
1.7.9.5


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

* [PATCH v3 4/6] timerfd: Factor out timer-type unspecific timerfd_expire()
  2014-02-20  8:40 [PATCH v3 0/6] Deferrable timers support for hrtimers/timerfd API Alexey Perevalov
                   ` (2 preceding siblings ...)
  2014-02-20  8:40 ` [PATCH v3 3/6] kernel/time: Add new helpers to convert ktime to/from jiffies Alexey Perevalov
@ 2014-02-20  8:40 ` Alexey Perevalov
  2014-02-20 10:52   ` Thomas Gleixner
  2014-02-20  8:40 ` [PATCH v3 5/6] timerfd: Add support for deferrable timers Alexey Perevalov
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 25+ messages in thread
From: Alexey Perevalov @ 2014-02-20  8:40 UTC (permalink / raw)
  To: linux-kernel, tglx, john.stultz
  Cc: Anton Vorontsov, anton, kyungmin.park, cw00.choi, akpm,
	Anton Vorontsov, Alexey Perevalov

From: Anton Vorontsov <anton@enomsg.org>

There is nothing hrtimer-specific inside the timerfd_tmrproc(), except
the function prototype. We're about to add other timer types, so factor
out generic timerfd_expire() helper from timerfd_tmrproc().

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
 fs/timerfd.c |   40 +++++++++++++++++++---------------------
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/fs/timerfd.c b/fs/timerfd.c
index 9293121..3561ce7 100644
--- a/fs/timerfd.c
+++ b/fs/timerfd.c
@@ -229,6 +229,23 @@ static unsigned int timerfd_poll(struct file *file, poll_table *wait)
 	return events;
 }
 
+static u64 timerfd_rearm(struct timerfd_ctx *ctx)
+{
+	u64 orun;
+
+	if (isalarm(ctx)) {
+		orun += alarm_forward_now(
+			&ctx->t.alarm, ctx->tintv) - 1;
+		alarm_restart(&ctx->t.alarm);
+	} else {
+		orun += hrtimer_forward_now(&ctx->t.tmr,
+		     ctx->tintv) - 1;
+		hrtimer_restart(&ctx->t.tmr);
+	}
+
+	return orun;
+}
+
 static ssize_t timerfd_read(struct file *file, char __user *buf, size_t count,
 			    loff_t *ppos)
 {
@@ -265,15 +282,7 @@ static ssize_t timerfd_read(struct file *file, char __user *buf, size_t count,
 			 * callback to avoid DoS attacks specifying a very
 			 * short timer period.
 			 */
-			if (isalarm(ctx)) {
-				ticks += alarm_forward_now(
-					&ctx->t.alarm, ctx->tintv) - 1;
-				alarm_restart(&ctx->t.alarm);
-			} else {
-				ticks += hrtimer_forward_now(&ctx->t.tmr,
-							     ctx->tintv) - 1;
-				hrtimer_restart(&ctx->t.tmr);
-			}
+			ticks += timerfd_rearm(ctx);
 		}
 		ctx->expired = 0;
 		ctx->ticks = 0;
@@ -421,18 +430,7 @@ static int do_timerfd_gettime(int ufd, struct itimerspec *t)
 	spin_lock_irq(&ctx->wqh.lock);
 	if (ctx->expired && ctx->tintv.tv64) {
 		ctx->expired = 0;
-
-		if (isalarm(ctx)) {
-			ctx->ticks +=
-				alarm_forward_now(
-					&ctx->t.alarm, ctx->tintv) - 1;
-			alarm_restart(&ctx->t.alarm);
-		} else {
-			ctx->ticks +=
-				hrtimer_forward_now(&ctx->t.tmr, ctx->tintv)
-				- 1;
-			hrtimer_restart(&ctx->t.tmr);
-		}
+		ctx->ticks += timerfd_rearm(ctx);
 	}
 	t->it_value = ktime_to_timespec(timerfd_get_remaining(ctx));
 	t->it_interval = ktime_to_timespec(ctx->tintv);
-- 
1.7.9.5


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

* [PATCH v3 5/6] timerfd: Add support for deferrable timers
  2014-02-20  8:40 [PATCH v3 0/6] Deferrable timers support for hrtimers/timerfd API Alexey Perevalov
                   ` (3 preceding siblings ...)
  2014-02-20  8:40 ` [PATCH v3 4/6] timerfd: Factor out timer-type unspecific timerfd_expire() Alexey Perevalov
@ 2014-02-20  8:40 ` Alexey Perevalov
  2014-02-20 11:09   ` Thomas Gleixner
  2014-02-20  8:40 ` [PATCH v3 6/6] tracing/trivial: Add CLOCK_*_DEFERRABLE for tracing clockids Alexey Perevalov
  2014-02-20 11:12 ` [PATCH v3 0/6] Deferrable timers support for hrtimers/timerfd API Thomas Gleixner
  6 siblings, 1 reply; 25+ messages in thread
From: Alexey Perevalov @ 2014-02-20  8:40 UTC (permalink / raw)
  To: linux-kernel, tglx, john.stultz
  Cc: Anton Vorontsov, anton, kyungmin.park, cw00.choi, akpm,
	Anton Vorontsov, Alexey Perevalov

From: Anton Vorontsov <anton@enomsg.org>

This patch implements a userland-side API for generic deferrable timers,
per linux/timer.h:

 * A deferrable timer will work normally when the system is busy, but
 * will not cause a CPU to come out of idle just to service it; instead,
 * the timer will be serviced when the CPU eventually wakes up with a
 * subsequent non-deferrable timer.

These timers are crucial for power saving, i.e. periodic tasks that want
to work in background when the system is under use, but don't want to
cause wakeups themselves.

The deferred timers are somewhat orthogonal to high-res external timers,
since the deferred timer is tied to the system load, not just to some
external decrementer source.

Currently, the implementation is based on high resolution timers.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
 fs/timerfd.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/timerfd.c b/fs/timerfd.c
index 3561ce7..c132fd2 100644
--- a/fs/timerfd.c
+++ b/fs/timerfd.c
@@ -231,7 +231,7 @@ static unsigned int timerfd_poll(struct file *file, poll_table *wait)
 
 static u64 timerfd_rearm(struct timerfd_ctx *ctx)
 {
-	u64 orun;
+	u64 orun = 0;
 
 	if (isalarm(ctx)) {
 		orun += alarm_forward_now(
@@ -326,7 +326,10 @@ SYSCALL_DEFINE2(timerfd_create, int, clockid, int, flags)
 	    (clockid != CLOCK_MONOTONIC &&
 	     clockid != CLOCK_REALTIME &&
 	     clockid != CLOCK_REALTIME_ALARM &&
-	     clockid != CLOCK_BOOTTIME_ALARM))
+	     clockid != CLOCK_BOOTTIME_ALARM &&
+	     clockid != CLOCK_REALTIME_DEFERRABLE &&
+	     clockid != CLOCK_MONOTONIC_DEFERRABLE &&
+	     clockid != CLOCK_BOOTTIME_DEFERRABLE))
 		return -EINVAL;
 
 	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
@@ -354,7 +357,7 @@ SYSCALL_DEFINE2(timerfd_create, int, clockid, int, flags)
 	return ufd;
 }
 
-static int do_timerfd_settime(int ufd, int flags, 
+static int do_timerfd_settime(int ufd, int flags,
 		const struct itimerspec *new,
 		struct itimerspec *old)
 {
-- 
1.7.9.5


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

* [PATCH v3 6/6] tracing/trivial: Add CLOCK_*_DEFERRABLE for tracing clockids
  2014-02-20  8:40 [PATCH v3 0/6] Deferrable timers support for hrtimers/timerfd API Alexey Perevalov
                   ` (4 preceding siblings ...)
  2014-02-20  8:40 ` [PATCH v3 5/6] timerfd: Add support for deferrable timers Alexey Perevalov
@ 2014-02-20  8:40 ` Alexey Perevalov
  2014-02-20 11:12 ` [PATCH v3 0/6] Deferrable timers support for hrtimers/timerfd API Thomas Gleixner
  6 siblings, 0 replies; 25+ messages in thread
From: Alexey Perevalov @ 2014-02-20  8:40 UTC (permalink / raw)
  To: linux-kernel, tglx, john.stultz
  Cc: Alexey Perevalov, anton, kyungmin.park, cw00.choi, akpm

Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
---
 include/trace/events/timer.h |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/trace/events/timer.h b/include/trace/events/timer.h
index 547b79f..ea3119a 100644
--- a/include/trace/events/timer.h
+++ b/include/trace/events/timer.h
@@ -13,7 +13,13 @@
 			 { CLOCK_MONOTONIC,		"CLOCK_MONOTONIC" },	\
 			 { CLOCK_REALTIME,		"CLOCK_REALTIME" },	\
 			 { CLOCK_BOOTTIME,		"CLOCK_BOOTTIME" },	\
-			 { CLOCK_TAI,			"CLOCK_TAI" }		\
+			 { CLOCK_TAI,			"CLOCK_TAI" },		\
+			 { CLOCK_MONOTONIC_DEFERRABLE,				\
+				"CLOCK_MONOTONIC_DEFERRABLE" },			\
+			 { CLOCK_REALTIME_DEFERRABLE,				\
+				"CLOCK_REALTIME_DEFERRABLE" },			\
+			 { CLOCK_BOOTTIME_DEFERRABLE,				\
+				"CLOCK_BOOTTIME_DEFERRABLE" }			\
 	)
 
 #define hrmode_to_string(hrmode)						\
-- 
1.7.9.5


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

* Re: [PATCH v3 3/6] kernel/time: Add new helpers to convert ktime to/from jiffies
  2014-02-20  8:40 ` [PATCH v3 3/6] kernel/time: Add new helpers to convert ktime to/from jiffies Alexey Perevalov
@ 2014-02-20 10:34   ` Thomas Gleixner
  0 siblings, 0 replies; 25+ messages in thread
From: Thomas Gleixner @ 2014-02-20 10:34 UTC (permalink / raw)
  To: Alexey Perevalov
  Cc: linux-kernel, john.stultz, Anton Vorontsov, anton, kyungmin.park,
	cw00.choi, akpm, Anton Vorontsov

On Thu, 20 Feb 2014, Alexey Perevalov wrote:

> From: Anton Vorontsov <anton@scarybugs.org>
> 
> Two new functions: jiffies_to_ktime() and ktime_to_jiffies(), we'll use
> them for timerfd deferred timers handling.

I don't think so. That's probably a leftover from the mess you tried
to create earlier.

Thanks,

	tglx

 

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

* Re: [PATCH v3 4/6] timerfd: Factor out timer-type unspecific timerfd_expire()
  2014-02-20  8:40 ` [PATCH v3 4/6] timerfd: Factor out timer-type unspecific timerfd_expire() Alexey Perevalov
@ 2014-02-20 10:52   ` Thomas Gleixner
  2014-02-20 16:30     ` Alexey Perevalov
  2014-02-21  4:13     ` Anton Vorontsov
  0 siblings, 2 replies; 25+ messages in thread
From: Thomas Gleixner @ 2014-02-20 10:52 UTC (permalink / raw)
  To: Alexey Perevalov
  Cc: linux-kernel, john.stultz, Anton Vorontsov, anton, kyungmin.park,
	cw00.choi, akpm, Anton Vorontsov

On Thu, 20 Feb 2014, Alexey Perevalov wrote:

> From: Anton Vorontsov <anton@enomsg.org>
> 
> There is nothing hrtimer-specific inside the timerfd_tmrproc(), except
> the function prototype. We're about to add other timer types, so factor
> out generic timerfd_expire() helper from timerfd_tmrproc().

This changelog is completely useless. How is timerfd_tmrproc, which is
not a function but a function pointer, related to the patch?

Moving duplicated code to a common function is nice, but ....
 
> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> ---
>  fs/timerfd.c |   40 +++++++++++++++++++---------------------
>  1 file changed, 19 insertions(+), 21 deletions(-)
> 
> diff --git a/fs/timerfd.c b/fs/timerfd.c
> index 9293121..3561ce7 100644
> --- a/fs/timerfd.c
> +++ b/fs/timerfd.c
> @@ -229,6 +229,23 @@ static unsigned int timerfd_poll(struct file *file, poll_table *wait)
>  	return events;
>  }
>  
> +static u64 timerfd_rearm(struct timerfd_ctx *ctx)
> +{
> +	u64 orun;
> +
> +	if (isalarm(ctx)) {
> +		orun += alarm_forward_now(
> +			&ctx->t.alarm, ctx->tintv) - 1;
> +		alarm_restart(&ctx->t.alarm);
> +	} else {
> +		orun += hrtimer_forward_now(&ctx->t.tmr,
> +		     ctx->tintv) - 1;
> +		hrtimer_restart(&ctx->t.tmr);

Warnings are there to be ignored and testing of user space
interfaces after a change is overrated, right?

Aside of that you just blindly copied the original code w/o fixing up
the now unnecessary line breaks.

The summary of this patch is:

    1) Breaks existing functionality including user space ABI
    2) Compiler warnings ignored
    3) Untested
    4) Utter lack of programming style
    5) Useless changelog

Impressive for a trivial thing like this.

Thanks,

	tglx

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

* Re: [PATCH v3 1/6] tracing/trivial: Add CLOCK_BOOTIME and CLOCK_TAI for human readable clockid trace
  2014-02-20  8:40 ` [PATCH v3 1/6] tracing/trivial: Add CLOCK_BOOTIME and CLOCK_TAI for human readable clockid trace Alexey Perevalov
@ 2014-02-20 11:01   ` Thomas Gleixner
  2014-02-20 16:25     ` Alexey Perevalov
  0 siblings, 1 reply; 25+ messages in thread
From: Thomas Gleixner @ 2014-02-20 11:01 UTC (permalink / raw)
  To: Alexey Perevalov
  Cc: linux-kernel, john.stultz, anton, kyungmin.park, cw00.choi, akpm



On Thu, 20 Feb 2014, Alexey Perevalov wrote:

> These clockids is also used in current hrtimer implementation.
> 
> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> ---
>  include/trace/events/timer.h |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/trace/events/timer.h b/include/trace/events/timer.h
> index 185b2c6..547b79f 100644
> --- a/include/trace/events/timer.h
> +++ b/include/trace/events/timer.h
> @@ -11,7 +11,10 @@
>  #define clockid_to_string(clockid)						\
>  	__print_symbolic(clockid,						\
>  			 { CLOCK_MONOTONIC,		"CLOCK_MONOTONIC" },	\
> -			 { CLOCK_REALTIME,		"CLOCK_REALTIME" })
> +			 { CLOCK_REALTIME,		"CLOCK_REALTIME" },	\
> +			 { CLOCK_BOOTTIME,		"CLOCK_BOOTTIME" },	\
> +			 { CLOCK_TAI,			"CLOCK_TAI" }		\
> +	)

patching file include/trace/events/timer.h
Hunk #1 FAILED at 11.
1 out of 1 hunk FAILED -- rejects in file include/trace/events/timer.h

Brilliant stuff this.



  

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

* Re: [PATCH v3 5/6] timerfd: Add support for deferrable timers
  2014-02-20  8:40 ` [PATCH v3 5/6] timerfd: Add support for deferrable timers Alexey Perevalov
@ 2014-02-20 11:09   ` Thomas Gleixner
  2014-02-21  4:11     ` Anton Vorontsov
  0 siblings, 1 reply; 25+ messages in thread
From: Thomas Gleixner @ 2014-02-20 11:09 UTC (permalink / raw)
  To: Alexey Perevalov
  Cc: linux-kernel, john.stultz, Anton Vorontsov, anton, kyungmin.park,
	cw00.choi, akpm, Anton Vorontsov

On Thu, 20 Feb 2014, Alexey Perevalov wrote:

> From: Anton Vorontsov <anton@enomsg.org>
> 
> This patch implements a userland-side API for generic deferrable timers,
> per linux/timer.h:
> 
>  * A deferrable timer will work normally when the system is busy, but
>  * will not cause a CPU to come out of idle just to service it; instead,
>  * the timer will be serviced when the CPU eventually wakes up with a
>  * subsequent non-deferrable timer.
> 
> These timers are crucial for power saving, i.e. periodic tasks that want
> to work in background when the system is under use, but don't want to
> cause wakeups themselves.
> 
> The deferred timers are somewhat orthogonal to high-res external timers,
> since the deferred timer is tied to the system load, not just to some
> external decrementer source.

Again this changelog makes no sense. What's orthogonal to high-res
timers and why are they external?

So 5 out of 6 patches are a trainwreck in various degrees of
wreckage. A pretty impressive achievement.

Thanks,

	tglx

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

* Re: [PATCH v3 0/6] Deferrable timers support for hrtimers/timerfd API
  2014-02-20  8:40 [PATCH v3 0/6] Deferrable timers support for hrtimers/timerfd API Alexey Perevalov
                   ` (5 preceding siblings ...)
  2014-02-20  8:40 ` [PATCH v3 6/6] tracing/trivial: Add CLOCK_*_DEFERRABLE for tracing clockids Alexey Perevalov
@ 2014-02-20 11:12 ` Thomas Gleixner
  2014-02-20 16:53   ` Alexey Perevalov
  6 siblings, 1 reply; 25+ messages in thread
From: Thomas Gleixner @ 2014-02-20 11:12 UTC (permalink / raw)
  To: Alexey Perevalov
  Cc: linux-kernel, john.stultz, anton, kyungmin.park, cw00.choi, akpm

On Thu, 20 Feb 2014, Alexey Perevalov wrote:

> Hello.
> 
> This is a combo patch set for support defferability in timerfd.
> Due implementation of timerfd is based on hrtimers and only on hrtimers,
> it was necessary to add such deferrability into hrtimers too.

Sigh, this is not about timerfd. Adding new clock ids exposes them to
all other hrtimer based interfaces as well.
 
Thanks,

	tglx

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

* Re: [PATCH v3 1/6] tracing/trivial: Add CLOCK_BOOTIME and CLOCK_TAI for human readable clockid trace
  2014-02-20 11:01   ` Thomas Gleixner
@ 2014-02-20 16:25     ` Alexey Perevalov
  0 siblings, 0 replies; 25+ messages in thread
From: Alexey Perevalov @ 2014-02-20 16:25 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, john.stultz, anton, kyungmin.park, cw00.choi, akpm

On 02/20/2014 03:01 PM, Thomas Gleixner wrote:
>
> On Thu, 20 Feb 2014, Alexey Perevalov wrote:
>
>> These clockids is also used in current hrtimer implementation.
>>
>> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
>> ---
>>   include/trace/events/timer.h |    5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/trace/events/timer.h b/include/trace/events/timer.h
>> index 185b2c6..547b79f 100644
>> --- a/include/trace/events/timer.h
>> +++ b/include/trace/events/timer.h
>> @@ -11,7 +11,10 @@
>>   #define clockid_to_string(clockid)						\
>>   	__print_symbolic(clockid,						\
>>   			 { CLOCK_MONOTONIC,		"CLOCK_MONOTONIC" },	\
>> -			 { CLOCK_REALTIME,		"CLOCK_REALTIME" })
>> +			 { CLOCK_REALTIME,		"CLOCK_REALTIME" },	\
>> +			 { CLOCK_BOOTTIME,		"CLOCK_BOOTTIME" },	\
>> +			 { CLOCK_TAI,			"CLOCK_TAI" }		\
>> +	)
> patching file include/trace/events/timer.h
> Hunk #1 FAILED at 11.
> 1 out of 1 hunk FAILED -- rejects in file include/trace/events/timer.h
>
> Brilliant stuff this.
>
>
>
>    
>
Sorry, I missed one patch


-- 
Best regards,
Alexey Perevalov

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

* Re: [PATCH v3 4/6] timerfd: Factor out timer-type unspecific timerfd_expire()
  2014-02-20 10:52   ` Thomas Gleixner
@ 2014-02-20 16:30     ` Alexey Perevalov
  2014-02-21  4:13     ` Anton Vorontsov
  1 sibling, 0 replies; 25+ messages in thread
From: Alexey Perevalov @ 2014-02-20 16:30 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, john.stultz, Anton Vorontsov, anton, kyungmin.park,
	cw00.choi, akpm, Anton Vorontsov

On 02/20/2014 02:52 PM, Thomas Gleixner wrote:
> On Thu, 20 Feb 2014, Alexey Perevalov wrote:
>
>> From: Anton Vorontsov <anton@enomsg.org>
>>
>> There is nothing hrtimer-specific inside the timerfd_tmrproc(), except
>> the function prototype. We're about to add other timer types, so factor
>> out generic timerfd_expire() helper from timerfd_tmrproc().
> This changelog is completely useless. How is timerfd_tmrproc, which is
> not a function but a function pointer, related to the patch?
>
> Moving duplicated code to a common function is nice, but ....
>   
>> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
>> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
>> ---
>>   fs/timerfd.c |   40 +++++++++++++++++++---------------------
>>   1 file changed, 19 insertions(+), 21 deletions(-)
>>
>> diff --git a/fs/timerfd.c b/fs/timerfd.c
>> index 9293121..3561ce7 100644
>> --- a/fs/timerfd.c
>> +++ b/fs/timerfd.c
>> @@ -229,6 +229,23 @@ static unsigned int timerfd_poll(struct file *file, poll_table *wait)
>>   	return events;
>>   }
>>   
>> +static u64 timerfd_rearm(struct timerfd_ctx *ctx)
>> +{
>> +	u64 orun;
>> +
>> +	if (isalarm(ctx)) {
>> +		orun += alarm_forward_now(
>> +			&ctx->t.alarm, ctx->tintv) - 1;
>> +		alarm_restart(&ctx->t.alarm);
>> +	} else {
>> +		orun += hrtimer_forward_now(&ctx->t.tmr,
>> +		     ctx->tintv) - 1;
>> +		hrtimer_restart(&ctx->t.tmr);
> Warnings are there to be ignored and testing of user space
> interfaces after a change is overrated, right?
>
> Aside of that you just blindly copied the original code w/o fixing up
> the now unnecessary line breaks.
>
> The summary of this patch is:
>
>      1) Breaks existing functionality including user space ABI
>      2) Compiler warnings ignored
>      3) Untested
>      4) Utter lack of programming style
>      5) Useless changelog
>
> Impressive for a trivial thing like this.
>
> Thanks,
>
> 	tglx
>
Compiler warning - if you mean uninitialized orun, I fixed it
ill-timed in the next patch,
yes it should be in original patch.

-- 
Best regards,
Alexey Perevalov

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

* Re: [PATCH v3 0/6] Deferrable timers support for hrtimers/timerfd API
  2014-02-20 11:12 ` [PATCH v3 0/6] Deferrable timers support for hrtimers/timerfd API Thomas Gleixner
@ 2014-02-20 16:53   ` Alexey Perevalov
  2014-02-20 21:21     ` Thomas Gleixner
  0 siblings, 1 reply; 25+ messages in thread
From: Alexey Perevalov @ 2014-02-20 16:53 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, john.stultz, Anton Vorontsov, kyungmin.park,
	cw00.choi, akpm

On 02/20/2014 03:12 PM, Thomas Gleixner wrote:
> On Thu, 20 Feb 2014, Alexey Perevalov wrote:
>
>> Hello.
>>
>> This is a combo patch set for support defferability in timerfd.
>> Due implementation of timerfd is based on hrtimers and only on hrtimers,
>> it was necessary to add such deferrability into hrtimers too.
> Sigh, this is not about timerfd. Adding new clock ids exposes them to
> all other hrtimer based interfaces as well.
Posix timers, maybe something else,
do you want to separate this patch set, or add to this patch set,
1. proper description
2. and if it'll touch posix-timer, posix_timers clocks should be 
extended too.

>   
> Thanks,
>
> 	tglx
>


-- 
Best regards,
Alexey Perevalov

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

* Re: [PATCH v3 2/6] hrtimer: Add support for deferrable timer into the hrtimer
  2014-02-20  8:40 ` [PATCH v3 2/6] hrtimer: Add support for deferrable timer into the hrtimer Alexey Perevalov
@ 2014-02-20 18:49   ` John Stultz
  2014-02-20 21:18     ` Thomas Gleixner
  0 siblings, 1 reply; 25+ messages in thread
From: John Stultz @ 2014-02-20 18:49 UTC (permalink / raw)
  To: Alexey Perevalov, linux-kernel, tglx
  Cc: anton, kyungmin.park, cw00.choi, akpm

On 02/20/2014 12:40 AM, Alexey Perevalov wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
>
> This patch introduces new public CLOCKID constants for
> user space API, such as timerfd. It extends hrtimer API and makes
> possible to have unified interfaces where deferreble functionality is
> used. In-kernel users such as device drivers could find benefits too.
>
> High resolution timer now could work with CLOCK_REALTIME_DEFERRABLE,
> CLOCK_MONOTONIC_DEFERRABLE, CLOCK_BOOTTIME_DEFERRABLE.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> ---
>  include/linux/hrtimer.h   |    3 +++
>  include/uapi/linux/time.h |    3 +++
>  kernel/hrtimer.c          |   62 +++++++++++++++++++++++++++++++++++----------
>  3 files changed, 55 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
> index d19a5c2..fe1159c 100644
> --- a/include/linux/hrtimer.h
> +++ b/include/linux/hrtimer.h
> @@ -158,6 +158,9 @@ enum  hrtimer_base_type {
>  	HRTIMER_BASE_REALTIME,
>  	HRTIMER_BASE_BOOTTIME,
>  	HRTIMER_BASE_TAI,
> +	HRTIMER_BASE_MONOTONIC_DEFERRABLE,
> +	HRTIMER_BASE_REALTIME_DEFERRABLE,
> +	HRTIMER_BASE_BOOTTIME_DEFERRABLE,
>  	HRTIMER_MAX_CLOCK_BASES,
>  };
>  
> diff --git a/include/uapi/linux/time.h b/include/uapi/linux/time.h
> index e75e1b6..bb8dc60 100644
> --- a/include/uapi/linux/time.h
> +++ b/include/uapi/linux/time.h
> @@ -56,6 +56,9 @@ struct itimerval {
>  #define CLOCK_BOOTTIME_ALARM		9
>  #define CLOCK_SGI_CYCLE			10	/* Hardware specific */
>  #define CLOCK_TAI			11
> +#define CLOCK_REALTIME_DEFERRABLE	12
> +#define CLOCK_MONOTONIC_DEFERRABLE	13
> +#define CLOCK_BOOTTIME_DEFERRABLE	14

Adding the deferrable HRTIMER bases above is right, but I don't think we
agreed on adding the _DEFERRABLE clockids.

I'd instead prefer you use add a new TIMER_DEFERABLE flags argument, and
use the combination of the clockid + flag to decide which HRTIMER base
is used.

thanks
-john


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

* Re: [PATCH v3 2/6] hrtimer: Add support for deferrable timer into the hrtimer
  2014-02-20 18:49   ` John Stultz
@ 2014-02-20 21:18     ` Thomas Gleixner
  2014-02-20 21:20       ` John Stultz
  0 siblings, 1 reply; 25+ messages in thread
From: Thomas Gleixner @ 2014-02-20 21:18 UTC (permalink / raw)
  To: John Stultz
  Cc: Alexey Perevalov, linux-kernel, anton, kyungmin.park, cw00.choi, akpm

On Thu, 20 Feb 2014, John Stultz wrote:
> On 02/20/2014 12:40 AM, Alexey Perevalov wrote:
> > diff --git a/include/uapi/linux/time.h b/include/uapi/linux/time.h
> > index e75e1b6..bb8dc60 100644
> > --- a/include/uapi/linux/time.h
> > +++ b/include/uapi/linux/time.h
> > @@ -56,6 +56,9 @@ struct itimerval {
> >  #define CLOCK_BOOTTIME_ALARM		9
> >  #define CLOCK_SGI_CYCLE			10	/* Hardware specific */
> >  #define CLOCK_TAI			11
> > +#define CLOCK_REALTIME_DEFERRABLE	12
> > +#define CLOCK_MONOTONIC_DEFERRABLE	13
> > +#define CLOCK_BOOTTIME_DEFERRABLE	14
> 
> Adding the deferrable HRTIMER bases above is right, but I don't think we
> agreed on adding the _DEFERRABLE clockids.
> 
> I'd instead prefer you use add a new TIMER_DEFERABLE flags argument, and
> use the combination of the clockid + flag to decide which HRTIMER base
> is used.

And how does that work with anything else than timerfd? If we add
deferrable posix clocks then we add them for the other interfaces
which take a clockid as well.

Thanks,

	tglx

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

* Re: [PATCH v3 2/6] hrtimer: Add support for deferrable timer into the hrtimer
  2014-02-20 21:18     ` Thomas Gleixner
@ 2014-02-20 21:20       ` John Stultz
  2014-02-20 21:56         ` Thomas Gleixner
  0 siblings, 1 reply; 25+ messages in thread
From: John Stultz @ 2014-02-20 21:20 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Alexey Perevalov, linux-kernel, anton, kyungmin.park, cw00.choi, akpm

On 02/20/2014 01:18 PM, Thomas Gleixner wrote:
> On Thu, 20 Feb 2014, John Stultz wrote:
>> On 02/20/2014 12:40 AM, Alexey Perevalov wrote:
>>> diff --git a/include/uapi/linux/time.h b/include/uapi/linux/time.h
>>> index e75e1b6..bb8dc60 100644
>>> --- a/include/uapi/linux/time.h
>>> +++ b/include/uapi/linux/time.h
>>> @@ -56,6 +56,9 @@ struct itimerval {
>>>  #define CLOCK_BOOTTIME_ALARM		9
>>>  #define CLOCK_SGI_CYCLE			10	/* Hardware specific */
>>>  #define CLOCK_TAI			11
>>> +#define CLOCK_REALTIME_DEFERRABLE	12
>>> +#define CLOCK_MONOTONIC_DEFERRABLE	13
>>> +#define CLOCK_BOOTTIME_DEFERRABLE	14
>> Adding the deferrable HRTIMER bases above is right, but I don't think we
>> agreed on adding the _DEFERRABLE clockids.
>>
>> I'd instead prefer you use add a new TIMER_DEFERABLE flags argument, and
>> use the combination of the clockid + flag to decide which HRTIMER base
>> is used.
> And how does that work with anything else than timerfd? If we add
> deferrable posix clocks then we add them for the other interfaces
> which take a clockid as well.
Other interfaces have flag arguments as well (for things like
TIMER_ABSTIME).

thanks
-john


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

* Re: [PATCH v3 0/6] Deferrable timers support for hrtimers/timerfd API
  2014-02-20 16:53   ` Alexey Perevalov
@ 2014-02-20 21:21     ` Thomas Gleixner
  0 siblings, 0 replies; 25+ messages in thread
From: Thomas Gleixner @ 2014-02-20 21:21 UTC (permalink / raw)
  To: Alexey Perevalov
  Cc: linux-kernel, john.stultz, Anton Vorontsov, kyungmin.park,
	cw00.choi, akpm

On Thu, 20 Feb 2014, Alexey Perevalov wrote:

> On 02/20/2014 03:12 PM, Thomas Gleixner wrote:
> > On Thu, 20 Feb 2014, Alexey Perevalov wrote:
> > 
> > > Hello.
> > > 
> > > This is a combo patch set for support defferability in timerfd.
> > > Due implementation of timerfd is based on hrtimers and only on hrtimers,
> > > it was necessary to add such deferrability into hrtimers too.
> > Sigh, this is not about timerfd. Adding new clock ids exposes them to
> > all other hrtimer based interfaces as well.
> Posix timers, maybe something else,
> do you want to separate this patch set, or add to this patch set,
> 1. proper description
> 2. and if it'll touch posix-timer, posix_timers clocks should be extended too.

Did you even try to understand the scope of the deferrable clock id
changes?

I really wonder why you try to hack such an interface without even
remotely understanding how the related user space interfaces work.

Thanks,

	tglx



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

* Re: [PATCH v3 2/6] hrtimer: Add support for deferrable timer into the hrtimer
  2014-02-20 21:20       ` John Stultz
@ 2014-02-20 21:56         ` Thomas Gleixner
  0 siblings, 0 replies; 25+ messages in thread
From: Thomas Gleixner @ 2014-02-20 21:56 UTC (permalink / raw)
  To: John Stultz
  Cc: Alexey Perevalov, linux-kernel, anton, kyungmin.park, cw00.choi, akpm

On Thu, 20 Feb 2014, John Stultz wrote:
> On 02/20/2014 01:18 PM, Thomas Gleixner wrote:
> > On Thu, 20 Feb 2014, John Stultz wrote:
> >> On 02/20/2014 12:40 AM, Alexey Perevalov wrote:
> >>> diff --git a/include/uapi/linux/time.h b/include/uapi/linux/time.h
> >>> index e75e1b6..bb8dc60 100644
> >>> --- a/include/uapi/linux/time.h
> >>> +++ b/include/uapi/linux/time.h
> >>> @@ -56,6 +56,9 @@ struct itimerval {
> >>>  #define CLOCK_BOOTTIME_ALARM		9
> >>>  #define CLOCK_SGI_CYCLE			10	/* Hardware specific */
> >>>  #define CLOCK_TAI			11
> >>> +#define CLOCK_REALTIME_DEFERRABLE	12
> >>> +#define CLOCK_MONOTONIC_DEFERRABLE	13
> >>> +#define CLOCK_BOOTTIME_DEFERRABLE	14
> >> Adding the deferrable HRTIMER bases above is right, but I don't think we
> >> agreed on adding the _DEFERRABLE clockids.
> >>
> >> I'd instead prefer you use add a new TIMER_DEFERABLE flags argument, and
> >> use the combination of the clockid + flag to decide which HRTIMER base
> >> is used.
> > And how does that work with anything else than timerfd? If we add
> > deferrable posix clocks then we add them for the other interfaces
> > which take a clockid as well.
> Other interfaces have flag arguments as well (for things like
> TIMER_ABSTIME).

Fair enough. Let me look tomorrow what implications that
has. Definitely a bit more intrusive, but probably more elegant.

Thanks,

	tglx

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

* Re: [PATCH v3 5/6] timerfd: Add support for deferrable timers
  2014-02-20 11:09   ` Thomas Gleixner
@ 2014-02-21  4:11     ` Anton Vorontsov
  2014-02-21 10:40       ` Thomas Gleixner
  0 siblings, 1 reply; 25+ messages in thread
From: Anton Vorontsov @ 2014-02-21  4:11 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Alexey Perevalov, linux-kernel, john.stultz, anton,
	kyungmin.park, cw00.choi, akpm

On Thu, Feb 20, 2014 at 12:09:43PM +0100, Thomas Gleixner wrote:
> On Thu, 20 Feb 2014, Alexey Perevalov wrote:
> > From: Anton Vorontsov <anton@enomsg.org>
> > 
> > This patch implements a userland-side API for generic deferrable timers,
> > per linux/timer.h:
> > 
> >  * A deferrable timer will work normally when the system is busy, but
> >  * will not cause a CPU to come out of idle just to service it; instead,
> >  * the timer will be serviced when the CPU eventually wakes up with a
> >  * subsequent non-deferrable timer.
> > 
> > These timers are crucial for power saving, i.e. periodic tasks that want
> > to work in background when the system is under use, but don't want to
> > cause wakeups themselves.
> > 
> > The deferred timers are somewhat orthogonal to high-res external timers,
> > since the deferred timer is tied to the system load, not just to some
> > external decrementer source.
> 
> Again this changelog makes no sense. What's orthogonal to high-res
> timers and why are they external?

Not trying to defend the current series, just felt the need clarify this
one.

By orthogonal I meant that comparing to high resolution timers' use cases,
deferred timers can be super-low resolution, super inaccurate. We don't
know exactly when they will fire, all we know is something like "every 0.2
seconds, iff the system/user is doing something, otherwise don't bother."

As for external (my bad, shouldn't invent personal terminology): the
hrtimers are tied to some clock source (which is "external" to me), but
deferred timers are mostly tied to the system's activity.

Thanks,

Anton

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

* Re: [PATCH v3 4/6] timerfd: Factor out timer-type unspecific timerfd_expire()
  2014-02-20 10:52   ` Thomas Gleixner
  2014-02-20 16:30     ` Alexey Perevalov
@ 2014-02-21  4:13     ` Anton Vorontsov
  2014-02-21  7:04       ` Alexey Perevalov
  2014-02-21 10:41       ` Thomas Gleixner
  1 sibling, 2 replies; 25+ messages in thread
From: Anton Vorontsov @ 2014-02-21  4:13 UTC (permalink / raw)
  To: Alexey Perevalov
  Cc: Thomas Gleixner, linux-kernel, john.stultz, anton, kyungmin.park,
	cw00.choi, akpm

On Thu, Feb 20, 2014 at 11:52:03AM +0100, Thomas Gleixner wrote:
> > From: Anton Vorontsov <anton@enomsg.org>
> > 
> > There is nothing hrtimer-specific inside the timerfd_tmrproc(), except
> > the function prototype. We're about to add other timer types, so factor
> > out generic timerfd_expire() helper from timerfd_tmrproc().
> 
> This changelog is completely useless. How is timerfd_tmrproc, which is
> not a function but a function pointer, related to the patch?
> 
> Moving duplicated code to a common function is nice, but ....

> > Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
> > Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
... 
> Warnings are there to be ignored and testing of user space
> interfaces after a change is overrated, right?
> 
> Aside of that you just blindly copied the original code w/o fixing up
> the now unnecessary line breaks.

Alexey,

While I appreciate the desire to be careful with authorship and stuff,
please remove my name as an author of this patch -- the current code has
nothing to do with my original work, and I surely don't want to take any
responsibility for it. This is a common practice if you modify someone's
patch to a great extend.

Thomas is bashing the thing, which has my name on it; although _my_ patch
did not produce any warnings, came with a completely different changelog,
and served completely different purposes.

Instead of rushing with resending yet another series, please actually read
Thomas' review.

Thanks,

Anton

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

* Re: [PATCH v3 4/6] timerfd: Factor out timer-type unspecific timerfd_expire()
  2014-02-21  4:13     ` Anton Vorontsov
@ 2014-02-21  7:04       ` Alexey Perevalov
  2014-02-21 10:41       ` Thomas Gleixner
  1 sibling, 0 replies; 25+ messages in thread
From: Alexey Perevalov @ 2014-02-21  7:04 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: Thomas Gleixner, linux-kernel, john.stultz, anton, kyungmin.park,
	cw00.choi, akpm

On 02/21/2014 08:13 AM, Anton Vorontsov wrote:
> On Thu, Feb 20, 2014 at 11:52:03AM +0100, Thomas Gleixner wrote:
>>> From: Anton Vorontsov <anton@enomsg.org>
>>>
>>> There is nothing hrtimer-specific inside the timerfd_tmrproc(), except
>>> the function prototype. We're about to add other timer types, so factor
>>> out generic timerfd_expire() helper from timerfd_tmrproc().
>> This changelog is completely useless. How is timerfd_tmrproc, which is
>> not a function but a function pointer, related to the patch?
>>
>> Moving duplicated code to a common function is nice, but ....
>>> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
>>> Signed-off-by: Alexey Perevalov <a.perevalov@samsung.com>
> ...
>> Warnings are there to be ignored and testing of user space
>> interfaces after a change is overrated, right?
>>
>> Aside of that you just blindly copied the original code w/o fixing up
>> the now unnecessary line breaks.
> Alexey,
>
> While I appreciate the desire to be careful with authorship and stuff,
> please remove my name as an author of this patch -- the current code has
> nothing to do with my original work, and I surely don't want to take any
> responsibility for it. This is a common practice if you modify someone's
> patch to a great extend.
>
> Thomas is bashing the thing, which has my name on it; although _my_ patch
> did not produce any warnings, came with a completely different changelog,
> and served completely different purposes.
Sorry, it was introduced by me, while rebasing.

>
> Instead of rushing with resending yet another series, please actually read
> Thomas' review.
>
> Thanks,
>
> Anton
>



-- 
Best regards,
Alexey Perevalov

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

* Re: [PATCH v3 5/6] timerfd: Add support for deferrable timers
  2014-02-21  4:11     ` Anton Vorontsov
@ 2014-02-21 10:40       ` Thomas Gleixner
  0 siblings, 0 replies; 25+ messages in thread
From: Thomas Gleixner @ 2014-02-21 10:40 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: Alexey Perevalov, linux-kernel, john.stultz, anton,
	kyungmin.park, cw00.choi, akpm

On Thu, 20 Feb 2014, Anton Vorontsov wrote:
> On Thu, Feb 20, 2014 at 12:09:43PM +0100, Thomas Gleixner wrote:
> > On Thu, 20 Feb 2014, Alexey Perevalov wrote:
> > > From: Anton Vorontsov <anton@enomsg.org>
> > > 
> > > This patch implements a userland-side API for generic deferrable timers,
> > > per linux/timer.h:
> > > 
> > >  * A deferrable timer will work normally when the system is busy, but
> > >  * will not cause a CPU to come out of idle just to service it; instead,
> > >  * the timer will be serviced when the CPU eventually wakes up with a
> > >  * subsequent non-deferrable timer.
> > > 
> > > These timers are crucial for power saving, i.e. periodic tasks that want
> > > to work in background when the system is under use, but don't want to
> > > cause wakeups themselves.
> > > 
> > > The deferred timers are somewhat orthogonal to high-res external timers,
> > > since the deferred timer is tied to the system load, not just to some
> > > external decrementer source.
> > 
> > Again this changelog makes no sense. What's orthogonal to high-res
> > timers and why are they external?
> 
> Not trying to defend the current series, just felt the need clarify this
> one.
> 
> By orthogonal I meant that comparing to high resolution timers' use cases,
> deferred timers can be super-low resolution, super inaccurate. We don't
> know exactly when they will fire, all we know is something like "every 0.2
> seconds, iff the system/user is doing something, otherwise don't bother."

Ok.
 
> As for external (my bad, shouldn't invent personal terminology): the
> hrtimers are tied to some clock source (which is "external" to me), but
> deferred timers are mostly tied to the system's activity.

I see what you mean, but that wants some different wording.

Thanks,

	tglx

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

* Re: [PATCH v3 4/6] timerfd: Factor out timer-type unspecific timerfd_expire()
  2014-02-21  4:13     ` Anton Vorontsov
  2014-02-21  7:04       ` Alexey Perevalov
@ 2014-02-21 10:41       ` Thomas Gleixner
  1 sibling, 0 replies; 25+ messages in thread
From: Thomas Gleixner @ 2014-02-21 10:41 UTC (permalink / raw)
  To: Anton Vorontsov
  Cc: Alexey Perevalov, linux-kernel, john.stultz, anton,
	kyungmin.park, cw00.choi, akpm

On Thu, 20 Feb 2014, Anton Vorontsov wrote:
> On Thu, Feb 20, 2014 at 11:52:03AM +0100, Thomas Gleixner wrote:
> Instead of rushing with resending yet another series, please actually read
> Thomas' review.

And please hold off until we sorted out the clockid vs. flags discussion.

Thanks,

	tglx

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

end of thread, other threads:[~2014-02-21 10:40 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-20  8:40 [PATCH v3 0/6] Deferrable timers support for hrtimers/timerfd API Alexey Perevalov
2014-02-20  8:40 ` [PATCH v3 1/6] tracing/trivial: Add CLOCK_BOOTIME and CLOCK_TAI for human readable clockid trace Alexey Perevalov
2014-02-20 11:01   ` Thomas Gleixner
2014-02-20 16:25     ` Alexey Perevalov
2014-02-20  8:40 ` [PATCH v3 2/6] hrtimer: Add support for deferrable timer into the hrtimer Alexey Perevalov
2014-02-20 18:49   ` John Stultz
2014-02-20 21:18     ` Thomas Gleixner
2014-02-20 21:20       ` John Stultz
2014-02-20 21:56         ` Thomas Gleixner
2014-02-20  8:40 ` [PATCH v3 3/6] kernel/time: Add new helpers to convert ktime to/from jiffies Alexey Perevalov
2014-02-20 10:34   ` Thomas Gleixner
2014-02-20  8:40 ` [PATCH v3 4/6] timerfd: Factor out timer-type unspecific timerfd_expire() Alexey Perevalov
2014-02-20 10:52   ` Thomas Gleixner
2014-02-20 16:30     ` Alexey Perevalov
2014-02-21  4:13     ` Anton Vorontsov
2014-02-21  7:04       ` Alexey Perevalov
2014-02-21 10:41       ` Thomas Gleixner
2014-02-20  8:40 ` [PATCH v3 5/6] timerfd: Add support for deferrable timers Alexey Perevalov
2014-02-20 11:09   ` Thomas Gleixner
2014-02-21  4:11     ` Anton Vorontsov
2014-02-21 10:40       ` Thomas Gleixner
2014-02-20  8:40 ` [PATCH v3 6/6] tracing/trivial: Add CLOCK_*_DEFERRABLE for tracing clockids Alexey Perevalov
2014-02-20 11:12 ` [PATCH v3 0/6] Deferrable timers support for hrtimers/timerfd API Thomas Gleixner
2014-02-20 16:53   ` Alexey Perevalov
2014-02-20 21:21     ` 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.