timerfd -- implement missing parts to checkpoint and restore timerfd state, v2
Subsystems (3)
cover timerfd -- implement missing parts to checkpoint and restore timerfd state, v2
Hi, here is an updated series, changes mostly in show_fdinfo code which now produces a way more information (and documentation update). Please take a look once time permit. Thanks!
PATCH 1/3 timerfd: Implement show_fdinfo method
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
--- fs/timerfd.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-)
fs/timerfd.c +29 -1
@@ -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 filereturn 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)
Re: [patch 1/3] timerfd: Implement show_fdinfo method
Hi Cyrill,
Quoted 66 lines · Cyrill Gorcunov
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_fdinfomethod which provides enough information for timer re-creation.One of significant changes I think is addition of
timerfd_ctx::settime_flagsmember. Currently there are two flagsTFD_TIMER_ABSTIMEandTFD_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_ctxstructure 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 intimerfd_setupin contextCC 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
===================================================================fs/timerfd.c +10
-1@@ -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 filereturn 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);
fs/timerfd.c static ssize_t timerfd_read(struct file · line 305
Quoted 10 lines
fs/timerfd.c +10
-0+ 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.
Quoted 17 lines
fs/timerfd.c +9
-0+ 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)
Re: [patch 1/3] timerfd: Implement show_fdinfo method
On Tue, Apr 08, 2014 at 10:17:14AM +0400, Vladimir Davydov wrote:
...
Quoted 14 lines
Quoted 14 lines
fs/timerfd.c +14
-0+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"
Quoted 8 lines
fs/timerfd.c static ssize_t timerfd_read(struct file · line 305
Quoted 2 lines
fs/timerfd.c +2
-0+"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, andit_intervalobtained from the fdinfo tosys_timerfd_settime, but this will be incorrect, because AFAIU theit_valueyou report here is always relative to the current time, no matter whetherTFD_TIMER_ABSTIMEwas set insettime_flagsor 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?
Re: [patch 1/3] timerfd: Implement show_fdinfo method
Quoted 23 lines · Cyrill Gorcunov
On Tue, Apr 08, 2014 at 10:17:14AM +0400, Vladimir Davydov wrote:
...Quoted 13 lines
Quoted 13 lines
fs/timerfd.c +13
-0+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"Quoted 8 lines
fs/timerfd.c static ssize_t timerfd_read(struct file · line 305
Quoted 3 lines
fs/timerfd.c +3
-0+"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, andit_intervalobtained from the fdinfo tosys_timerfd_settime, but this will be incorrect, because AFAIU theit_valueyou report here is always relative to the current time, no matter whetherTFD_TIMER_ABSTIMEwas set insettime_flagsor not. Is it OK?
Quoted 4 lines
Hi Vladimir! Well
it_valuereturns 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:
Quoted 1 line
'
it_value' and 'it_interval' are the expiration time and interval for the timer.
Thanks.
Re: [patch 1/3] timerfd: Implement show_fdinfo method
Quoted 10 lines · Vladimir Davydov
Quoted 4 lines
Hi Vladimir! Well
it_valuereturns 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:
Quoted 1 line
'
it_value' and 'it_interval' are the expiration time and interval for the timer.
I'll update, thanks!
PATCH 2/3 docs: procfs -- Document timerfd output
In reply to Cyrill Gorcunov--- Documentation/filesystems/proc.txt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
Documentation/filesystems/proc.txt +17 -0
@@ -1741,6 +1741,23 @@ pair provide additional information part While the first three lines are mandatory and always printed, the rest is optional and may be omitted if no marks created yet. + Timerfd files + ~~~~~~~~~~~~~ + + pos: 0 + flags: 02 + mnt_id: 9 + clockid: 0 + ticks: 0 + settime flags: 01 + it_value: (0, 49406829) + it_interval: (1, 0) + + where 'clockid' is the clock type and 'ticks' is the number of the timer expirations + that have occurred [see timerfd_create(2) for details]. 'settime flags' are + flags in octal form used to setup the timer [see timerfd_settime(2) for + details]. 'it_value' and 'it_interval' are the expiration time and interval + for the timer. ------------------------------------------------------------------------------ Configuring procfs
PATCH 2/3 docs: procfs -- Document timerfd output, v2
---
Vladimir, would the below text be more clear?
Documentation/filesystems/proc.txt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
Documentation/filesystems/proc.txt +17 -0
@@ -1741,6 +1741,23 @@ pair provide additional information part While the first three lines are mandatory and always printed, the rest is optional and may be omitted if no marks created yet. + Timerfd files + ~~~~~~~~~~~~~ + + pos: 0 + flags: 02 + mnt_id: 9 + clockid: 0 + ticks: 0 + settime flags: 01 + it_value: (0, 49406829) + it_interval: (1, 0) + + where 'clockid' is the clock type and 'ticks' is the number of the timer expirations + that have occurred [see timerfd_create(2) for details]. 'settime flags' are + flags in octal form used to setup the timer [see timerfd_settime(2) for + details]. 'it_value' is remaining time until the timer exiration. + 'it_interval' is the interval for the timer. ------------------------------------------------------------------------------ Configuring procfs
Re: [patch 2/3] docs: procfs -- Document timerfd output, v2
Quoted 10 lines · Cyrill Gorcunov
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]>---
Vladimir, would the below text be more clear?
It looks good to me.
Thanks.
Quoted 32 lines
Documentation/filesystems/proc.txt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)Index:
linux-2.6.git/Documentation/filesystems/proc.txt
===================================================================Documentation/filesystems/proc.txt +17
-0@@ -1741,6 +1741,23 @@ pair provide additional information part While the first three lines are mandatory and always printed, the rest is optional and may be omitted if no marks created yet. + Timerfd files + ~~~~~~~~~~~~~ + + pos: 0 + flags: 02 + mnt_id: 9 + clockid: 0 + ticks: 0 + settime flags: 01 + it_value: (0, 49406829) + it_interval: (1, 0) + + where 'clockid' is the clock type and 'ticks' is the number of the timer expirations + that have occurred [see timerfd_create(2) for details]. 'settime flags' are + flags in octal form used to setup the timer [see timerfd_settime(2) for + details]. 'it_value' is remaining time until the timer exiration. + 'it_interval' is the interval for the timer. ------------------------------------------------------------------------------ Configuring procfs
PATCH 3/3 timerfd: Implement write method
In reply to Cyrill GorcunovThe read() of timerfd files allows to fetch the number of timer ticks while there is no way to set it back from userspace.
To restore the timer state as it was at checkpoint moment we need a way to setup ticks back. So as a counterpart of read() the write() takes ticks number from the userspace and updates internal timer ticks accordingly.
--- fs/timerfd.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
fs/timerfd.c +21 -0
@@ -311,10 +311,31 @@ static int timerfd_show(struct seq_file (unsigned long long)t.it_interval.tv_nsec); } + +static ssize_t timerfd_write (struct file *file,const char __user *buf, +size_t count,loff_t *ppos) +{ +struct timerfd_ctx *ctx = file->private_data; +u64 ticks =0 ; + +if (count <sizeof (ticks)) +return -EINVAL ; + +if (get_user (ticks, (u64 __user *) buf)) +return -EFAULT ; + +spin_lock_irq (&ctx->wqh.lock); + ctx->ticks = ticks; +spin_unlock_irq (&ctx->wqh.lock); + +return sizeof (ticks); +} +static const struct file_operations timerfd_fops = { .release = timerfd_release, .poll = timerfd_poll, .read = timerfd_read, + .write = timerfd_write, .llseek = noop_llseek, .show_fdinfo = timerfd_show, };
Quoted 3 lines · Cyrill Gorcunov
Hi, here is an updated series, changes mostly in
show_fdinfocode which now produces a way more information (and documentation update). Please take a look once time permit. Thanks!
Hi, any new comments on this series?
Quoted 6 lines · Cyrill Gorcunov
Quoted 3 lines · Cyrill Gorcunov
Hi, here is an updated series, changes mostly in
show_fdinfocode which now produces a way more information (and documentation update). Please take a look once time permit. Thanks!Hi, any new comments on this series?
Looks ok and I did not do anything about it, as I saw a "will update in next version" from you in the discussion.
Thanks,
tglx
Quoted 11 lines · Thomas Gleixner
Quoted 6 lines · Cyrill Gorcunov
Quoted 3 lines · Cyrill Gorcunov
Hi, here is an updated series, changes mostly in
show_fdinfocode which now produces a way more information (and documentation update). Please take a look once time permit. Thanks!Hi, any new comments on this series?
Looks ok and I did not do anything about it, as I saw a "will update in next version" from you in the discussion.
Silly me :( I promised to update the series a bit, and forgot. Thanks, Thomas!