2014-01-02 18:31:26

by Alexey Perevalov

[permalink] [raw]
Subject: [PATCH 0/3] Deferrable timers support for timerfd API

Hello dear community.

This is reworked patch set of original Anton's Vorontsov
proposal regarding unified deferrable timers in user space.
http://lwn.net/Articles/514707/


I decided to resubmit it due we found it usefull for us too.

timerfd was modified since Anton's commit, Alarm support was added.
This isn't only rebase. Anton's previous version used deferrable timer
in couple with hrtimer. This version uses only deferrable timer. It
mean the behaviour of overrun number is different.
e.g. if you don't poll one second timer for a 10 seconds - you'll get
10 overruns with hrtimer, but in deferrable only version you'll get
only one overrun.

This version introduces new clockid (CLOCK_DEFERRABLE) , for timerfd_create, instead of
new flag (TFD_TIMER_DEFERRABLE) for timerfd_settime introduced in previous version.


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

fs/timerfd.c | 97 ++++++++++++++++++++++++++++++++++-----------
include/linux/jiffies.h | 4 +-
include/linux/ktime.h | 3 +-
include/uapi/linux/time.h | 1 +
kernel/time.c | 23 +++++++++++
5 files changed, 102 insertions(+), 26 deletions(-)

--
1.7.9.5


2014-01-02 18:31:34

by Alexey Perevalov

[permalink] [raw]
Subject: [PATCH 1/3] kernel/time: Add new helpers to convert ktime to/from jiffies

From: Anton Vorontsov <[email protected]>

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 <[email protected]>
Signed-off-by: Alexey Perevalov <[email protected]>
---
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 d235e88..1ba02ae 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 */

/*
@@ -302,7 +303,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

2014-01-02 18:31:40

by Alexey Perevalov

[permalink] [raw]
Subject: [PATCH 2/3] timerfd: Factor out timer-type unspecific timerfd_expire()

From: Anton Vorontsov <[email protected]>

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 <[email protected]>
Signed-off-by: Alexey Perevalov <[email protected]>
---
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

2014-01-02 18:31:38

by Alexey Perevalov

[permalink] [raw]
Subject: [PATCH 3/3] timerfd: Add support for deferrable timers

From: Anton Vorontsov <[email protected]>

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.

So, currently, the implementation has a HZ precision, and the maximum
interval is jiffies resolution (i.e. with HZ=1000, on 32 bit that would
be around max 49 days). Of course we can implement longer timeouts by
rearming the timer, although it probably wouldn't make much sense in
real world, so we keep it simple and just return E2BIG if we don't like
the interval.

Signed-off-by: Anton Vorontsov <[email protected]>
Signed-off-by: Alexey Perevalov <[email protected]>
---
fs/timerfd.c | 59 ++++++++++++++++++++++++++++++++++++++++++---
include/uapi/linux/time.h | 1 +
2 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/fs/timerfd.c b/fs/timerfd.c
index 3561ce7..331ce4b 100644
--- a/fs/timerfd.c
+++ b/fs/timerfd.c
@@ -30,6 +30,7 @@ struct timerfd_ctx {
union {
struct hrtimer tmr;
struct alarm alarm;
+ struct timer_list dtmr;
} t;
ktime_t tintv;
ktime_t moffs;
@@ -51,6 +52,11 @@ static inline bool isalarm(struct timerfd_ctx *ctx)
ctx->clockid == CLOCK_BOOTTIME_ALARM;
}

+static inline bool isdeferrable(struct timerfd_ctx *ctx)
+{
+ return ctx->clockid == CLOCK_DEFERRABLE;
+}
+
/*
* This gets called when the timer event triggers. We set the "expired"
* flag, but we do not re-arm the timer (in case it's necessary,
@@ -75,6 +81,11 @@ static enum hrtimer_restart timerfd_tmrproc(struct hrtimer *htmr)
return HRTIMER_NORESTART;
}

+static void timerfd_dtmrproc(unsigned long data)
+{
+ timerfd_triggered((struct timerfd_ctx *)data);
+}
+
static enum alarmtimer_restart timerfd_alarmproc(struct alarm *alarm,
ktime_t now)
{
@@ -151,12 +162,36 @@ static ktime_t timerfd_get_remaining(struct timerfd_ctx *ctx)

if (isalarm(ctx))
remaining = alarm_expires_remaining(&ctx->t.alarm);
- else
+ else if (isdeferrable(ctx)) {
+ ktime_t expires;
+ jiffies_to_ktime(ctx->t.dtmr.expires, &expires);
+ remaining = ktime_sub(expires, ktime_get());
+ } else
remaining = hrtimer_expires_remaining(&ctx->t.tmr);

return remaining.tv64 < 0 ? ktime_set(0, 0): remaining;
}

+static bool timerfd_deferrable_valid(ktime_t intv)
+{
+ ktime_t max;
+
+ jiffies_to_ktime(MAX_JIFFY_OFFSET, &max);
+ if (intv.tv64 > max.tv64)
+ return 0;
+ return 1;
+}
+
+static int timerfd_setup_deferrable(struct timerfd_ctx *ctx, ktime_t texp)
+{
+ if (!timerfd_deferrable_valid(ctx->tintv) ||
+ !timerfd_deferrable_valid(texp))
+ return -E2BIG;
+
+ mod_timer(&ctx->t.dtmr, ktime_to_jiffies(&texp) + 1);
+ return 0;
+}
+
static int timerfd_setup(struct timerfd_ctx *ctx, int flags,
const struct itimerspec *ktmr)
{
@@ -177,6 +212,9 @@ static int timerfd_setup(struct timerfd_ctx *ctx, int flags,
ctx->clockid == CLOCK_REALTIME_ALARM ?
ALARM_REALTIME : ALARM_BOOTTIME,
timerfd_alarmproc);
+ } else if (isdeferrable(ctx)) {
+ ctx->t.dtmr.function = timerfd_dtmrproc;
+ ctx->t.dtmr.data = (unsigned long)ctx;
} else {
hrtimer_init(&ctx->t.tmr, clockid, htmode);
hrtimer_set_expires(&ctx->t.tmr, texp);
@@ -189,6 +227,8 @@ static int timerfd_setup(struct timerfd_ctx *ctx, int flags,
alarm_start(&ctx->t.alarm, texp);
else
alarm_start_relative(&ctx->t.alarm, texp);
+ } else if (isdeferrable(ctx)) {
+ timerfd_setup_deferrable(ctx, texp);
} else {
hrtimer_start(&ctx->t.tmr, texp, htmode);
}
@@ -207,6 +247,8 @@ static int timerfd_release(struct inode *inode, struct file *file)

if (isalarm(ctx))
alarm_cancel(&ctx->t.alarm);
+ else if (isdeferrable(ctx))
+ del_timer_sync(&ctx->t.dtmr);
else
hrtimer_cancel(&ctx->t.tmr);
kfree_rcu(ctx, rcu);
@@ -231,12 +273,15 @@ 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(
&ctx->t.alarm, ctx->tintv) - 1;
alarm_restart(&ctx->t.alarm);
+ } else if (isdeferrable(ctx)) {
+ mod_timer(&ctx->t.dtmr, jiffies +
+ ktime_to_jiffies(&ctx->tintv) + 1);
} else {
orun += hrtimer_forward_now(&ctx->t.tmr,
ctx->tintv) - 1;
@@ -326,7 +371,8 @@ 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_DEFERRABLE))
return -EINVAL;

ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
@@ -341,6 +387,8 @@ SYSCALL_DEFINE2(timerfd_create, int, clockid, int, flags)
ctx->clockid == CLOCK_REALTIME_ALARM ?
ALARM_REALTIME : ALARM_BOOTTIME,
timerfd_alarmproc);
+ else if (isdeferrable(ctx))
+ init_timer_deferrable(&ctx->t.dtmr);
else
hrtimer_init(&ctx->t.tmr, clockid, HRTIMER_MODE_ABS);

@@ -354,7 +402,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)
{
@@ -384,6 +432,9 @@ static int do_timerfd_settime(int ufd, int flags,
if (isalarm(ctx)) {
if (alarm_try_to_cancel(&ctx->t.alarm) >= 0)
break;
+ } else if (isdeferrable(ctx)) {
+ if (try_to_del_timer_sync(&ctx->t.dtmr) >= 0)
+ break;
} else {
if (hrtimer_try_to_cancel(&ctx->t.tmr) >= 0)
break;
diff --git a/include/uapi/linux/time.h b/include/uapi/linux/time.h
index e75e1b6..3481cb3 100644
--- a/include/uapi/linux/time.h
+++ b/include/uapi/linux/time.h
@@ -56,6 +56,7 @@ struct itimerval {
#define CLOCK_BOOTTIME_ALARM 9
#define CLOCK_SGI_CYCLE 10 /* Hardware specific */
#define CLOCK_TAI 11
+#define CLOCK_DEFERRABLE 12

#define MAX_CLOCKS 16
#define CLOCKS_MASK (CLOCK_REALTIME | CLOCK_MONOTONIC)
--
1.7.9.5

2014-01-02 19:32:43

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH 3/3] timerfd: Add support for deferrable timers

On 01/02/2014 10:30 AM, Alexey Perevalov wrote:
> From: Anton Vorontsov <[email protected]>
>
> 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.
>
> So, currently, the implementation has a HZ precision, and the maximum
> interval is jiffies resolution (i.e. with HZ=1000, on 32 bit that would
> be around max 49 days). Of course we can implement longer timeouts by
> rearming the timer, although it probably wouldn't make much sense in
> real world, so we keep it simple and just return E2BIG if we don't like
> the interval.
>
> Signed-off-by: Anton Vorontsov <[email protected]>
> Signed-off-by: Alexey Perevalov <[email protected]>
> ---
> fs/timerfd.c | 59 ++++++++++++++++++++++++++++++++++++++++++---
> include/uapi/linux/time.h | 1 +
> 2 files changed, 56 insertions(+), 4 deletions(-)
>
> diff --git a/fs/timerfd.c b/fs/timerfd.c
> index 3561ce7..331ce4b 100644
> --- a/fs/timerfd.c
> +++ b/fs/timerfd.c
> @@ -30,6 +30,7 @@ struct timerfd_ctx {
> union {
> struct hrtimer tmr;
> struct alarm alarm;
> + struct timer_list dtmr;
> } t;
> ktime_t tintv;
> ktime_t moffs;
> @@ -51,6 +52,11 @@ static inline bool isalarm(struct timerfd_ctx *ctx)
> ctx->clockid == CLOCK_BOOTTIME_ALARM;
> }
>
> +static inline bool isdeferrable(struct timerfd_ctx *ctx)
> +{
> + return ctx->clockid == CLOCK_DEFERRABLE;
> +}
> +
> /*
> * This gets called when the timer event triggers. We set the "expired"
> * flag, but we do not re-arm the timer (in case it's necessary,
[snip]
> diff --git a/include/uapi/linux/time.h b/include/uapi/linux/time.h
> index e75e1b6..3481cb3 100644
> --- a/include/uapi/linux/time.h
> +++ b/include/uapi/linux/time.h
> @@ -56,6 +56,7 @@ struct itimerval {
> #define CLOCK_BOOTTIME_ALARM 9
> #define CLOCK_SGI_CYCLE 10 /* Hardware specific */
> #define CLOCK_TAI 11
> +#define CLOCK_DEFERRABLE 12
>
> #define MAX_CLOCKS 16
> #define CLOCKS_MASK (CLOCK_REALTIME | CLOCK_MONOTONIC)


So, I'm a bit concerned about the CLOCK_DEFERRABLE clockid, as it isn't
clear what time domain it uses. Its unlikely to be its own time domain,
so this is really a misuse of extending the clockids.

It seems to me that deferrability is an attribute of the time domain. So
instead of having a CLOCK_DEFERRABLE, I suspect we'll want something
like CLOCK_MONOTONIC_DEFER/CLOCK_REALTIME_DEFER, etc.

That is, unless we extend the timerfd interface to also take something
like a properties flag or something.

thanks
-john





2014-01-02 23:17:38

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH 0/3] Deferrable timers support for timerfd API

On 01/02/2014 10:30 AM, Alexey Perevalov wrote:
> This version introduces new clockid (CLOCK_DEFERRABLE) , for timerfd_create, instead of
> new flag (TFD_TIMER_DEFERRABLE) for timerfd_settime introduced in previous version.

So why did you make this change?

thanks
-john

2014-01-03 17:45:13

by Alexey Perevalov

[permalink] [raw]
Subject: Re: [PATCH 0/3] Deferrable timers support for timerfd API

On 01/03/2014 03:17 AM, John Stultz wrote:
> On 01/02/2014 10:30 AM, Alexey Perevalov wrote:
>> This version introduces new clockid (CLOCK_DEFERRABLE) , for timerfd_create, instead of
>> new flag (TFD_TIMER_DEFERRABLE) for timerfd_settime introduced in previous version.
> So why did you make this change?
>
> thanks
> -john
>
>

I looked at alarm timers and found approach of making timer behavior
persistent per file descriptor is better than
changeable by timerfd_settime. I think "end user wake up from suspend"
and "don't wake up in idle" is the same thing on the same abstraction level.

Yes Anton's previous patches worked with CLOCK_MONOTONIC only and I
didn't intend to use it with CLOCK_REALTIME, cause it's hard to me to
find such use case.
Another way - it's stay as was Anton's patch, I mean as flag for the
timerfd_settime, but in original patch set both hrtimer and deferrable
timers initialized in timerfd_create, I think it's not needed. Also
ability to change timer behavior looks not good if you couldn't change
alarm timer behavior, not unified API.

If I'm right, only high resolution timer could be REALTIME, and there is
no deferrable behavior for hrtimer only for timer_list.


--
Best regards,
Alexey Perevalov

2014-01-04 00:18:32

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH 0/3] Deferrable timers support for timerfd API

On 01/03/2014 09:45 AM, Alexey Perevalov wrote:
> On 01/03/2014 03:17 AM, John Stultz wrote:
>> On 01/02/2014 10:30 AM, Alexey Perevalov wrote:
>>> This version introduces new clockid (CLOCK_DEFERRABLE) , for
>>> timerfd_create, instead of
>>> new flag (TFD_TIMER_DEFERRABLE) for timerfd_settime introduced in
>>> previous version.
>> So why did you make this change?
>>
>> thanks
>> -john
>>
>>
>
> I looked at alarm timers and found approach of making timer behavior
> persistent per file descriptor is better than
> changeable by timerfd_settime. I think "end user wake up from suspend"
> and "don't wake up in idle" is the same thing on the same abstraction
> level.
>
> Yes Anton's previous patches worked with CLOCK_MONOTONIC only and I
> didn't intend to use it with CLOCK_REALTIME, cause it's hard to me to
> find such use case.
> Another way - it's stay as was Anton's patch, I mean as flag for the
> timerfd_settime, but in original patch set both hrtimer and deferrable
> timers initialized in timerfd_create, I think it's not needed. Also
> ability to change timer behavior looks not good if you couldn't change
> alarm timer behavior, not unified API.

So while the alarm timers are a reasonable precedent, I think they were
introduced prior to the timerfd interface, so it seemed at the time
having new clockids for the functionality was required to work with the
existing syscalls that use the clockid (Though in retrospect, I question
if it would have been better to use timer flags to introduce the alarm
functionality rather then introducing it via a clockid, as it would
simplify the clockid definitions).

Now that we have the timerfd interface, and if this functionality is
really limited to the timerfds, then we may want to consider what might
be, at least to me, the cleaner approach of using the flag.

Either way, I'd like to make sure we have a sound rational. My worry is
that deferrable timers would be desired on more then just
CLOCK_MONOTONIC, so we could quite likely end up with quite a few new
clockids (ie: CLOCK_BOOTTIME_DEFERRED, CLOCK_TAI_DEFERRED,
CLOCK_REALTIME_DEFERRED).


>
> If I'm right, only high resolution timer could be REALTIME, and there
> is no deferrable behavior for hrtimer only for timer_list.
>
>
I'm not sure I understood this part. Could you explain further?

thanks
-john

2014-01-05 19:33:55

by Alexey Perevalov

[permalink] [raw]
Subject: Re: [PATCH 0/3] Deferrable timers support for timerfd API

On 01/04/2014 04:18 AM, John Stultz wrote:
> On 01/03/2014 09:45 AM, Alexey Perevalov wrote:
>> On 01/03/2014 03:17 AM, John Stultz wrote:
>>> On 01/02/2014 10:30 AM, Alexey Perevalov wrote:
>>>> This version introduces new clockid (CLOCK_DEFERRABLE) , for
>>>> timerfd_create, instead of
>>>> new flag (TFD_TIMER_DEFERRABLE) for timerfd_settime introduced in
>>>> previous version.
>>> So why did you make this change?
>>>
>>> thanks
>>> -john
>>>
>>>
>> I looked at alarm timers and found approach of making timer behavior
>> persistent per file descriptor is better than
>> changeable by timerfd_settime. I think "end user wake up from suspend"
>> and "don't wake up in idle" is the same thing on the same abstraction
>> level.
>>
>> Yes Anton's previous patches worked with CLOCK_MONOTONIC only and I
>> didn't intend to use it with CLOCK_REALTIME, cause it's hard to me to
>> find such use case.
>> Another way - it's stay as was Anton's patch, I mean as flag for the
>> timerfd_settime, but in original patch set both hrtimer and deferrable
>> timers initialized in timerfd_create, I think it's not needed. Also
>> ability to change timer behavior looks not good if you couldn't change
>> alarm timer behavior, not unified API.
> So while the alarm timers are a reasonable precedent, I think they were
> introduced prior to the timerfd interface, so it seemed at the time
> having new clockids for the functionality was required to work with the
> existing syscalls that use the clockid (Though in retrospect, I question
> if it would have been better to use timer flags to introduce the alarm
> functionality rather then introducing it via a clockid, as it would
> simplify the clockid definitions).
As I understood alarm and deferrability it's type of repetition (timer
trigger condition),
but REALTIME, BOOTTIME, MONOTONIC it's a type of time representation.
Mixed it in one clockid, maybe it's a controversially. Which flags do
you want to use, flags of timerfd_settime?

>
> Now that we have the timerfd interface, and if this functionality is
> really limited to the timerfds, then we may want to consider what might
> be, at least to me, the cleaner approach of using the flag.
>
> Either way, I'd like to make sure we have a sound rational. My worry is
> that deferrable timers would be desired on more then just
> CLOCK_MONOTONIC, so we could quite likely end up with quite a few new
> clockids (ie: CLOCK_BOOTTIME_DEFERRED, CLOCK_TAI_DEFERRED,
> CLOCK_REALTIME_DEFERRED).
>
Here I'm totally agree CLOCK_DEFERRABLE is not maintainable named constant.


>> If I'm right, only high resolution timer could be REALTIME, and there
>> is no deferrable behavior for hrtimer only for timer_list.
>>
>>
> I'm not sure I understood this part. Could you explain further?
I meant, we couldn't use hrtimer for deferrability, right now.

>
> thanks
> -john
>
>


--
Best regards,
Alexey Perevalov

2014-01-09 20:33:09

by John Stultz

[permalink] [raw]
Subject: Re: [PATCH 0/3] Deferrable timers support for timerfd API

On Sun, Jan 5, 2014 at 11:33 AM, Alexey Perevalov
<[email protected]> wrote:
> On 01/04/2014 04:18 AM, John Stultz wrote:
>>
>> So while the alarm timers are a reasonable precedent, I think they were
>> introduced prior to the timerfd interface, so it seemed at the time
>> having new clockids for the functionality was required to work with the
>> existing syscalls that use the clockid (Though in retrospect, I question
>> if it would have been better to use timer flags to introduce the alarm
>> functionality rather then introducing it via a clockid, as it would
>> simplify the clockid definitions).
>
> As I understood alarm and deferrability it's type of repetition (timer
> trigger condition),
> but REALTIME, BOOTTIME, MONOTONIC it's a type of time representation.
> Mixed it in one clockid, maybe it's a controversially. Which flags do you
> want to use, flags of timerfd_settime?

Well, my first reaction was just to suggest you create a new timer
flag (like TIMER_ABSTIME) TIMER_DEFER, which we could then consider
adapting as a new flag value for all the timer related code
(posix-timers, nanosleep, etc).

Then looking closer at the timerfd code, I see I wasn't aware there's
some additional constraints as the timerfd flags overlap with many of
the file O_ flags, and that the timerfd has its own TFD_TIMER_ABSTIME.
I'm not quite sure I recall the context of that decision, so it might
be good to involve those who recall more of that history. So I'm not
sure which particular bit flag would be best to take there. Anton's
patch took (1<<2), so I'm guessing that's still ok.

Anyway, adding something like a TFD_TIMER_DEFER along with TIMER_DEFER
would seem to me like a reasonable approach.

Addtionally the TFD_CANCEL_ON_SET is interesting in that we don't have
a similar flag for the posix timers interfaces. Do you think there's
any value in looking into unifying that as well?

thanks
-john

2014-01-12 17:16:49

by Alexey Perevalov

[permalink] [raw]
Subject: Re: [PATCH 0/3] Deferrable timers support for timerfd API

On 01/10/2014 12:32 AM, John Stultz wrote:
> On Sun, Jan 5, 2014 at 11:33 AM, Alexey Perevalov
> <[email protected]> wrote:
>> On 01/04/2014 04:18 AM, John Stultz wrote:
>>> So while the alarm timers are a reasonable precedent, I think they were
>>> introduced prior to the timerfd interface, so it seemed at the time
>>> having new clockids for the functionality was required to work with the
>>> existing syscalls that use the clockid (Though in retrospect, I question
>>> if it would have been better to use timer flags to introduce the alarm
>>> functionality rather then introducing it via a clockid, as it would
>>> simplify the clockid definitions).
>> As I understood alarm and deferrability it's type of repetition (timer
>> trigger condition),
>> but REALTIME, BOOTTIME, MONOTONIC it's a type of time representation.
>> Mixed it in one clockid, maybe it's a controversially. Which flags do you
>> want to use, flags of timerfd_settime?
> Well, my first reaction was just to suggest you create a new timer
> flag (like TIMER_ABSTIME) TIMER_DEFER, which we could then consider
> adapting as a new flag value for all the timer related code
> (posix-timers, nanosleep, etc).
>
> Then looking closer at the timerfd code, I see I wasn't aware there's
> some additional constraints as the timerfd flags overlap with many of
> the file O_ flags, and that the timerfd has its own TFD_TIMER_ABSTIME.
> I'm not quite sure I recall the context of that decision, so it might
> be good to involve those who recall more of that history. So I'm not
> sure which particular bit flag would be best to take there. Anton's
> patch took (1<<2), so I'm guessing that's still ok.
>
> Anyway, adding something like a TFD_TIMER_DEFER along with TIMER_DEFER
> would seem to me like a reasonable approach.
>
> Addtionally the TFD_CANCEL_ON_SET is interesting in that we don't have
> a similar flag for the posix timers interfaces. Do you think there's
> any value in looking into unifying that as well?
Posix timer doens't have cancel_list ability right now.
Do you want to add such ability?
With posix timer there is no problem of interference O_ flags,
and flag for posix timer might have the same value as
TFD_TIMER_CANCEL_ON_SET,
and appropriate name.

Version of Anton's patches with flag based interface I'll send tomorrow.
But with little modification in overruns, I think evaluation
based on hrtimer for overrun is not proper way for deferrable timer, because
in most cases number of deferrable ticks is lesser.

> thanks
> -john
>


--
Best regards,
Alexey Perevalov