For checkpoint/restore of timerfd files we need to know how exactly
the timer were armed to be able to handle it. Thus implement show_fdinfo
method which provides enough information for timer re-creation.
One of significant changes I think is addition of
timerfd_ctx::settime_flags member. Currently there are
two flags TFD_TIMER_ABSTIME and TFD_TIMER_CANCEL_ON_SET,
and the second can be found from @might_cancel variable
but in case if the flags will be extended in future we
most probably will have to somehow remember them explicitly
anyway so I guss doing that right now won't hurt.
To not bloat the timerfd_ctx structure I've converted
@expired to short integer and defined @settime_flags
as short as well.
v2 (by avagin@, vdavydov@ and tglx@):
- Add it_value/it_interval fields
- Save flags being used in timerfd_setup in context
CC: Shawn Landden <[email protected]>
CC: Thomas Gleixner <[email protected]>
CC: Andrew Morton <[email protected]>
CC: Andrey Vagin <[email protected]>
CC: Pavel Emelyanov <[email protected]>
CC: Vladimir Davydov <[email protected]>
Signed-off-by: Cyrill Gorcunov <[email protected]>
---
fs/timerfd.c | 30 +++++++++++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
Index: linux-2.6.git/fs/timerfd.c
===================================================================
--- linux-2.6.git.orig/fs/timerfd.c
+++ linux-2.6.git/fs/timerfd.c
@@ -35,8 +35,9 @@ struct timerfd_ctx {
ktime_t moffs;
wait_queue_head_t wqh;
u64 ticks;
- int expired;
int clockid;
+ short unsigned expired;
+ short unsigned settime_flags; /* to show in fdinfo */
struct rcu_head rcu;
struct list_head clist;
bool might_cancel;
@@ -196,6 +197,8 @@ static int timerfd_setup(struct timerfd_
if (timerfd_canceled(ctx))
return -ECANCELED;
}
+
+ ctx->settime_flags = flags & TFD_SETTIME_FLAGS;
return 0;
}
@@ -284,11 +287,36 @@ static ssize_t timerfd_read(struct file
return res;
}
+static int timerfd_show(struct seq_file *m, struct file *file)
+{
+ struct timerfd_ctx *ctx = file->private_data;
+ struct itimerspec t;
+
+ spin_lock_irq(&ctx->wqh.lock);
+ t.it_value = ktime_to_timespec(timerfd_get_remaining(ctx));
+ t.it_interval = ktime_to_timespec(ctx->tintv);
+ spin_unlock_irq(&ctx->wqh.lock);
+
+ return seq_printf(m,
+ "clockid: %d\n"
+ "ticks: %llu\n"
+ "settime flags: 0%o\n"
+ "it_value: (%llu, %llu)\n"
+ "it_interval: (%llu, %llu)\n",
+ ctx->clockid, (unsigned long long)ctx->ticks,
+ ctx->settime_flags,
+ (unsigned long long)t.it_value.tv_sec,
+ (unsigned long long)t.it_value.tv_nsec,
+ (unsigned long long)t.it_interval.tv_sec,
+ (unsigned long long)t.it_interval.tv_nsec);
+}
+
static const struct file_operations timerfd_fops = {
.release = timerfd_release,
.poll = timerfd_poll,
.read = timerfd_read,
.llseek = noop_llseek,
+ .show_fdinfo = timerfd_show,
};
static int timerfd_fget(int fd, struct fd *p)
Hi Cyrill,
On 04/07/2014 09:47 PM, Cyrill Gorcunov wrote:
> For checkpoint/restore of timerfd files we need to know how exactly
> the timer were armed to be able to handle it. Thus implement show_fdinfo
> method which provides enough information for timer re-creation.
>
> One of significant changes I think is addition of
> timerfd_ctx::settime_flags member. Currently there are
> two flags TFD_TIMER_ABSTIME and TFD_TIMER_CANCEL_ON_SET,
> and the second can be found from @might_cancel variable
> but in case if the flags will be extended in future we
> most probably will have to somehow remember them explicitly
> anyway so I guss doing that right now won't hurt.
>
> To not bloat the timerfd_ctx structure I've converted
> @expired to short integer and defined @settime_flags
> as short as well.
>
> v2 (by avagin@, vdavydov@ and tglx@):
>
> - Add it_value/it_interval fields
> - Save flags being used in timerfd_setup in context
>
> CC: Shawn Landden <[email protected]>
> CC: Thomas Gleixner <[email protected]>
> CC: Andrew Morton <[email protected]>
> CC: Andrey Vagin <[email protected]>
> CC: Pavel Emelyanov <[email protected]>
> CC: Vladimir Davydov <[email protected]>
> Signed-off-by: Cyrill Gorcunov <[email protected]>
> ---
> fs/timerfd.c | 30 +++++++++++++++++++++++++++++-
> 1 file changed, 29 insertions(+), 1 deletion(-)
>
> Index: linux-2.6.git/fs/timerfd.c
> ===================================================================
> --- linux-2.6.git.orig/fs/timerfd.c
> +++ linux-2.6.git/fs/timerfd.c
> @@ -35,8 +35,9 @@ struct timerfd_ctx {
> ktime_t moffs;
> wait_queue_head_t wqh;
> u64 ticks;
> - int expired;
> int clockid;
> + short unsigned expired;
> + short unsigned settime_flags; /* to show in fdinfo */
> struct rcu_head rcu;
> struct list_head clist;
> bool might_cancel;
> @@ -196,6 +197,8 @@ static int timerfd_setup(struct timerfd_
> if (timerfd_canceled(ctx))
> return -ECANCELED;
> }
> +
> + ctx->settime_flags = flags & TFD_SETTIME_FLAGS;
> return 0;
> }
>
> @@ -284,11 +287,36 @@ static ssize_t timerfd_read(struct file
> return res;
> }
>
> +static int timerfd_show(struct seq_file *m, struct file *file)
> +{
> + struct timerfd_ctx *ctx = file->private_data;
> + struct itimerspec t;
> +
> + spin_lock_irq(&ctx->wqh.lock);
> + t.it_value = ktime_to_timespec(timerfd_get_remaining(ctx));
> + t.it_interval = ktime_to_timespec(ctx->tintv);
> + spin_unlock_irq(&ctx->wqh.lock);
> +
> + return seq_printf(m,
> + "clockid: %d\n"
> + "ticks: %llu\n"
> + "settime flags: 0%o\n"
> + "it_value: (%llu, %llu)\n"
> + "it_interval: (%llu, %llu)\n",
IMO, one would expect to setup the timer on restore by passing the
values of settime_flags, it_value, and it_interval obtained from the
fdinfo to sys_timerfd_settime, but this will be incorrect, because AFAIU
the it_value you report here is always relative to the current time, no
matter whether TFD_TIMER_ABSTIME was set in settime_flags or not. Is it OK?
Thanks.
> + ctx->clockid, (unsigned long long)ctx->ticks,
> + ctx->settime_flags,
> + (unsigned long long)t.it_value.tv_sec,
> + (unsigned long long)t.it_value.tv_nsec,
> + (unsigned long long)t.it_interval.tv_sec,
> + (unsigned long long)t.it_interval.tv_nsec);
> +}
> +
> static const struct file_operations timerfd_fops = {
> .release = timerfd_release,
> .poll = timerfd_poll,
> .read = timerfd_read,
> .llseek = noop_llseek,
> + .show_fdinfo = timerfd_show,
> };
>
> static int timerfd_fget(int fd, struct fd *p)
On Tue, Apr 08, 2014 at 10:17:14AM +0400, Vladimir Davydov wrote:
...
> > +static int timerfd_show(struct seq_file *m, struct file *file)
> > +{
> > + struct timerfd_ctx *ctx = file->private_data;
> > + struct itimerspec t;
> > +
> > + spin_lock_irq(&ctx->wqh.lock);
> > + t.it_value = ktime_to_timespec(timerfd_get_remaining(ctx));
> > + t.it_interval = ktime_to_timespec(ctx->tintv);
> > + spin_unlock_irq(&ctx->wqh.lock);
> > +
> > + return seq_printf(m,
> > + "clockid: %d\n"
> > + "ticks: %llu\n"
> > + "settime flags: 0%o\n"
> > + "it_value: (%llu, %llu)\n"
> > + "it_interval: (%llu, %llu)\n",
>
> IMO, one would expect to setup the timer on restore by passing the
> values of settime_flags, it_value, and it_interval obtained from the
> fdinfo to sys_timerfd_settime, but this will be incorrect, because AFAIU
> the it_value you report here is always relative to the current time, no
> matter whether TFD_TIMER_ABSTIME was set in settime_flags or not. Is it OK?
Hi Vladimir! Well it_value returns remaining time so it's up to user to
adjust this value when restore with abs time. That said one can examinate
if abs flag was set and restore accordingly. If this is vague I'm open
to change it to more clear way. Ideas?
On 04/08/2014 10:42 AM, Cyrill Gorcunov wrote:
> On Tue, Apr 08, 2014 at 10:17:14AM +0400, Vladimir Davydov wrote:
> ...
>>> +static int timerfd_show(struct seq_file *m, struct file *file)
>>> +{
>>> + struct timerfd_ctx *ctx = file->private_data;
>>> + struct itimerspec t;
>>> +
>>> + spin_lock_irq(&ctx->wqh.lock);
>>> + t.it_value = ktime_to_timespec(timerfd_get_remaining(ctx));
>>> + t.it_interval = ktime_to_timespec(ctx->tintv);
>>> + spin_unlock_irq(&ctx->wqh.lock);
>>> +
>>> + return seq_printf(m,
>>> + "clockid: %d\n"
>>> + "ticks: %llu\n"
>>> + "settime flags: 0%o\n"
>>> + "it_value: (%llu, %llu)\n"
>>> + "it_interval: (%llu, %llu)\n",
>> IMO, one would expect to setup the timer on restore by passing the
>> values of settime_flags, it_value, and it_interval obtained from the
>> fdinfo to sys_timerfd_settime, but this will be incorrect, because AFAIU
>> the it_value you report here is always relative to the current time, no
>> matter whether TFD_TIMER_ABSTIME was set in settime_flags or not. Is it OK?
> Hi Vladimir! Well it_value returns remaining time so it's up to user to
> adjust this value when restore with abs time. That said one can examinate
> if abs flag was set and restore accordingly. If this is vague I'm open
> to change it to more clear way. Ideas?
If it's intended, it should be documented explicitly I think. Currently
in the doc patch I see nothing about whether expiration time is absolute
or relative:
> 'it_value' and 'it_interval' are the expiration time and interval for the timer.
Thanks.
On Tue, Apr 08, 2014 at 10:54:45AM +0400, Vladimir Davydov wrote:
> > Hi Vladimir! Well it_value returns remaining time so it's up to user to
> > adjust this value when restore with abs time. That said one can examinate
> > if abs flag was set and restore accordingly. If this is vague I'm open
> > to change it to more clear way. Ideas?
>
> If it's intended, it should be documented explicitly I think. Currently
> in the doc patch I see nothing about whether expiration time is absolute
> or relative:
>
> > 'it_value' and 'it_interval' are the expiration time and interval for the timer.
I'll update, thanks!