The 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.
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 | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
Index: linux-2.6.git/fs/timerfd.c
===================================================================
--- linux-2.6.git.orig/fs/timerfd.c
+++ linux-2.6.git/fs/timerfd.c
@@ -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,
};
On Thu, May 22, 2014 at 06:58:19AM +0900, Thomas Gleixner wrote:
> >
> > So what wakes a potential waiter in read/poll?
>
> And who is updating timerfd_create(2) ?
Thomas, could you please take a look if the approach below is acceptable?
If it will be fine I update manpage then.
---
From: Cyrill Gorcunov <[email protected]>
Subject: timerfd: Implement timerfd_ioctl method to restore timerfd_ctx::ticks
The 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's state as it was at checkpoint moment we need
a path to bring @ticks back. Initially I thought about writing ticks
back via write() interface but it seems such API is somehow obscure.
Instead implement timerfd_ioctl() method with TFD_IOC_SET_TICKS
command which requires CAP_SYS_RESOURCE capability to be able to
set @ticks into arbitrary value. Note this command doesn't wake
up readers/waiters and its purpose only to serve C/R needs
(for same sake I wrapped code with CONFIG_CHECKPOINT_RESTORE).
Still if needed the ioctl may be extended for new commands
and CONFIG_CHECKPOINT_RESTORE dropped off.
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 | 31 +++++++++++++++++++++++++++++++
include/linux/timerfd.h | 5 +++++
2 files changed, 36 insertions(+)
Index: linux-2.6.git/fs/timerfd.c
===================================================================
--- linux-2.6.git.orig/fs/timerfd.c
+++ linux-2.6.git/fs/timerfd.c
@@ -313,11 +313,42 @@ static int timerfd_show(struct seq_file
}
#endif
+#ifdef CONFIG_CHECKPOINT_RESTORE
+static long timerfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+ struct timerfd_ctx *ctx = file->private_data;
+ int ret = 0;
+
+ switch (cmd) {
+ case TFD_IOC_SET_TICKS: {
+ u64 ticks;
+
+ if (!capable(CAP_SYS_RESOURCE))
+ return -EPERM;
+ if (get_user(ticks, (u64 __user *)arg))
+ return -EFAULT;
+ spin_lock_irq(&ctx->wqh.lock);
+ ctx->ticks = ticks;
+ spin_unlock_irq(&ctx->wqh.lock);
+ break;
+ }
+ default:
+ ret = -ENOTTY;
+ break;
+ }
+
+ return ret;
+}
+#endif
+
static const struct file_operations timerfd_fops = {
.release = timerfd_release,
.poll = timerfd_poll,
.read = timerfd_read,
.llseek = noop_llseek,
+#ifdef CONFIG_CHECKPOINT_RESTORE
+ .unlocked_ioctl = timerfd_ioctl,
+#endif
#ifdef CONFIG_PROC_FS
.show_fdinfo = timerfd_show,
#endif
Index: linux-2.6.git/include/linux/timerfd.h
===================================================================
--- linux-2.6.git.orig/include/linux/timerfd.h
+++ linux-2.6.git/include/linux/timerfd.h
@@ -11,6 +11,9 @@
/* For O_CLOEXEC and O_NONBLOCK */
#include <linux/fcntl.h>
+/* For _IO helpers */
+#include <linux/ioctl.h>
+
/*
* CAREFUL: Check include/asm-generic/fcntl.h when defining
* new flags, since they might collide with O_* ones. We want
@@ -29,4 +32,6 @@
/* Flags for timerfd_settime. */
#define TFD_SETTIME_FLAGS (TFD_TIMER_ABSTIME | TFD_TIMER_CANCEL_ON_SET)
+#define TFD_IOC_SET_TICKS _IOW('T', 0, u64)
+
#endif /* _LINUX_TIMERFD_H */
[CC += linux-api@]
On Tue, Jun 10, 2014 at 6:35 PM, Cyrill Gorcunov <[email protected]> wrote:
> On Thu, May 22, 2014 at 06:58:19AM +0900, Thomas Gleixner wrote:
>> >
>> > So what wakes a potential waiter in read/poll?
>>
>> And who is updating timerfd_create(2) ?
>
> Thomas, could you please take a look if the approach below is acceptable?
> If it will be fine I update manpage then.
> ---
> From: Cyrill Gorcunov <[email protected]>
> Subject: timerfd: Implement timerfd_ioctl method to restore timerfd_ctx::ticks
>
> The 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's state as it was at checkpoint moment we need
> a path to bring @ticks back. Initially I thought about writing ticks
> back via write() interface but it seems such API is somehow obscure.
>
> Instead implement timerfd_ioctl() method with TFD_IOC_SET_TICKS
> command which requires CAP_SYS_RESOURCE capability to be able to
> set @ticks into arbitrary value. Note this command doesn't wake
> up readers/waiters and its purpose only to serve C/R needs
> (for same sake I wrapped code with CONFIG_CHECKPOINT_RESTORE).
> Still if needed the ioctl may be extended for new commands
> and CONFIG_CHECKPOINT_RESTORE dropped off.
>
> 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 | 31 +++++++++++++++++++++++++++++++
> include/linux/timerfd.h | 5 +++++
> 2 files changed, 36 insertions(+)
>
> Index: linux-2.6.git/fs/timerfd.c
> ===================================================================
> --- linux-2.6.git.orig/fs/timerfd.c
> +++ linux-2.6.git/fs/timerfd.c
> @@ -313,11 +313,42 @@ static int timerfd_show(struct seq_file
> }
> #endif
>
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> +static long timerfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> + struct timerfd_ctx *ctx = file->private_data;
> + int ret = 0;
> +
> + switch (cmd) {
> + case TFD_IOC_SET_TICKS: {
> + u64 ticks;
> +
> + if (!capable(CAP_SYS_RESOURCE))
> + return -EPERM;
> + if (get_user(ticks, (u64 __user *)arg))
> + return -EFAULT;
> + spin_lock_irq(&ctx->wqh.lock);
> + ctx->ticks = ticks;
> + spin_unlock_irq(&ctx->wqh.lock);
> + break;
> + }
> + default:
> + ret = -ENOTTY;
> + break;
> + }
> +
> + return ret;
> +}
> +#endif
> +
> static const struct file_operations timerfd_fops = {
> .release = timerfd_release,
> .poll = timerfd_poll,
> .read = timerfd_read,
> .llseek = noop_llseek,
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> + .unlocked_ioctl = timerfd_ioctl,
> +#endif
> #ifdef CONFIG_PROC_FS
> .show_fdinfo = timerfd_show,
> #endif
> Index: linux-2.6.git/include/linux/timerfd.h
> ===================================================================
> --- linux-2.6.git.orig/include/linux/timerfd.h
> +++ linux-2.6.git/include/linux/timerfd.h
> @@ -11,6 +11,9 @@
> /* For O_CLOEXEC and O_NONBLOCK */
> #include <linux/fcntl.h>
>
> +/* For _IO helpers */
> +#include <linux/ioctl.h>
> +
> /*
> * CAREFUL: Check include/asm-generic/fcntl.h when defining
> * new flags, since they might collide with O_* ones. We want
> @@ -29,4 +32,6 @@
> /* Flags for timerfd_settime. */
> #define TFD_SETTIME_FLAGS (TFD_TIMER_ABSTIME | TFD_TIMER_CANCEL_ON_SET)
>
> +#define TFD_IOC_SET_TICKS _IOW('T', 0, u64)
> +
> #endif /* _LINUX_TIMERFD_H */
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
On Tue, Jun 10, 2014 at 1:03 PM, Michael Kerrisk (man-pages)
<[email protected]> wrote:
> [CC += linux-api@]
>
> On Tue, Jun 10, 2014 at 6:35 PM, Cyrill Gorcunov <[email protected]> wrote:
>> On Thu, May 22, 2014 at 06:58:19AM +0900, Thomas Gleixner wrote:
>>> >
>>> > So what wakes a potential waiter in read/poll?
>>>
>>> And who is updating timerfd_create(2) ?
>>
>> Thomas, could you please take a look if the approach below is acceptable?
>> If it will be fine I update manpage then.
>> ---
>> From: Cyrill Gorcunov <[email protected]>
>> Subject: timerfd: Implement timerfd_ioctl method to restore timerfd_ctx::ticks
>>
>> The 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's state as it was at checkpoint moment we need
>> a path to bring @ticks back. Initially I thought about writing ticks
>> back via write() interface but it seems such API is somehow obscure.
>>
>> Instead implement timerfd_ioctl() method with TFD_IOC_SET_TICKS
>> command which requires CAP_SYS_RESOURCE capability to be able to
>> set @ticks into arbitrary value. Note this command doesn't wake
>> up readers/waiters and its purpose only to serve C/R needs
>> (for same sake I wrapped code with CONFIG_CHECKPOINT_RESTORE).
>> Still if needed the ioctl may be extended for new commands
>> and CONFIG_CHECKPOINT_RESTORE dropped off.
Why does this need CAP_SYS_RESOURCE?
--Andy
On Tue, Jun 10, 2014 at 01:05:22PM -0700, Andy Lutomirski wrote:
> On Tue, Jun 10, 2014 at 1:03 PM, Michael Kerrisk (man-pages)
> <[email protected]> wrote:
> > [CC += linux-api@]
Thanks Michael!
> > On Tue, Jun 10, 2014 at 6:35 PM, Cyrill Gorcunov <[email protected]> wrote:
> >> On Thu, May 22, 2014 at 06:58:19AM +0900, Thomas Gleixner wrote:
> >>> >
> >>> > So what wakes a potential waiter in read/poll?
> >>>
> >>> And who is updating timerfd_create(2) ?
> >>
> >> Thomas, could you please take a look if the approach below is acceptable?
> >> If it will be fine I update manpage then.
> >> ---
> >> From: Cyrill Gorcunov <[email protected]>
> >> Subject: timerfd: Implement timerfd_ioctl method to restore timerfd_ctx::ticks
> >>
> >> The 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's state as it was at checkpoint moment we need
> >> a path to bring @ticks back. Initially I thought about writing ticks
> >> back via write() interface but it seems such API is somehow obscure.
> >>
> >> Instead implement timerfd_ioctl() method with TFD_IOC_SET_TICKS
> >> command which requires CAP_SYS_RESOURCE capability to be able to
> >> set @ticks into arbitrary value. Note this command doesn't wake
> >> up readers/waiters and its purpose only to serve C/R needs
> >> (for same sake I wrapped code with CONFIG_CHECKPOINT_RESTORE).
> >> Still if needed the ioctl may be extended for new commands
> >> and CONFIG_CHECKPOINT_RESTORE dropped off.
>
> Why does this need CAP_SYS_RESOURCE?
Because I think this interface should not be used by a regular
applications, the only purpose is to restore the @ticks after
checkpoint. Requiring CAP_SYS_RESOURCE means that at least
program which use it knows what it's doing.
Still if someone has a scenarion where we might need this
intarface out of this cap requirement -- we always can
drop it of without breaking existing users, but not the
reverse.
P.S. I remember Thomas' words about existence of the other
word out of c/r, still I treat this ioctl as exception
(as in prctl codes we use for c/r).
On Tue, Jun 10, 2014 at 08:35:30PM +0400, Cyrill Gorcunov wrote:
> On Thu, May 22, 2014 at 06:58:19AM +0900, Thomas Gleixner wrote:
> > >
> > > So what wakes a potential waiter in read/poll?
> >
> > And who is updating timerfd_create(2) ?
>
> Thomas, could you please take a look if the approach below is acceptable?
> If it will be fine I update manpage then.
> ---
> From: Cyrill Gorcunov <[email protected]>
> Subject: timerfd: Implement timerfd_ioctl method to restore timerfd_ctx::ticks
>
> The 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's state as it was at checkpoint moment we need
> a path to bring @ticks back. Initially I thought about writing ticks
> back via write() interface but it seems such API is somehow obscure.
>
> Instead implement timerfd_ioctl() method with TFD_IOC_SET_TICKS
> command which requires CAP_SYS_RESOURCE capability to be able to
> set @ticks into arbitrary value. Note this command doesn't wake
> up readers/waiters and its purpose only to serve C/R needs
> (for same sake I wrapped code with CONFIG_CHECKPOINT_RESTORE).
> Still if needed the ioctl may be extended for new commands
> and CONFIG_CHECKPOINT_RESTORE dropped off.
>
> 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 | 31 +++++++++++++++++++++++++++++++
> include/linux/timerfd.h | 5 +++++
> 2 files changed, 36 insertions(+)
>
> Index: linux-2.6.git/fs/timerfd.c
> ===================================================================
> --- linux-2.6.git.orig/fs/timerfd.c
> +++ linux-2.6.git/fs/timerfd.c
> @@ -313,11 +313,42 @@ static int timerfd_show(struct seq_file
> }
> #endif
>
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> +static long timerfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> + struct timerfd_ctx *ctx = file->private_data;
> + int ret = 0;
> +
> + switch (cmd) {
> + case TFD_IOC_SET_TICKS: {
> + u64 ticks;
> +
> + if (!capable(CAP_SYS_RESOURCE))
> + return -EPERM;
I think it is too strong. It will not work in userns.
Why do we need to check CAP_SYS_RESOURCE here?
Can we replace capable on ns_capable?
> + if (get_user(ticks, (u64 __user *)arg))
> + return -EFAULT;
> + spin_lock_irq(&ctx->wqh.lock);
> + ctx->ticks = ticks;
I think we need to wakt up readers here if ctx->ticks isn't zero.
> + spin_unlock_irq(&ctx->wqh.lock);
> + break;
> + }
> + default:
> + ret = -ENOTTY;
> + break;
> + }
> +
> + return ret;
> +}
> +#endif
> +
> static const struct file_operations timerfd_fops = {
> .release = timerfd_release,
> .poll = timerfd_poll,
> .read = timerfd_read,
> .llseek = noop_llseek,
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> + .unlocked_ioctl = timerfd_ioctl,
> +#endif
> #ifdef CONFIG_PROC_FS
> .show_fdinfo = timerfd_show,
> #endif
> Index: linux-2.6.git/include/linux/timerfd.h
> ===================================================================
> --- linux-2.6.git.orig/include/linux/timerfd.h
> +++ linux-2.6.git/include/linux/timerfd.h
> @@ -11,6 +11,9 @@
> /* For O_CLOEXEC and O_NONBLOCK */
> #include <linux/fcntl.h>
>
> +/* For _IO helpers */
> +#include <linux/ioctl.h>
> +
> /*
> * CAREFUL: Check include/asm-generic/fcntl.h when defining
> * new flags, since they might collide with O_* ones. We want
> @@ -29,4 +32,6 @@
> /* Flags for timerfd_settime. */
> #define TFD_SETTIME_FLAGS (TFD_TIMER_ABSTIME | TFD_TIMER_CANCEL_ON_SET)
>
> +#define TFD_IOC_SET_TICKS _IOW('T', 0, u64)
> +
> #endif /* _LINUX_TIMERFD_H */
On Wed, Jun 11, 2014 at 11:27:43AM +0400, Andrew Vagin wrote:
> > + case TFD_IOC_SET_TICKS: {
> > + u64 ticks;
> > +
> > + if (!capable(CAP_SYS_RESOURCE))
> > + return -EPERM;
>
> I think it is too strong. It will not work in userns.
>
> Why do we need to check CAP_SYS_RESOURCE here?
> Can we replace capable on ns_capable?
>
> > + if (get_user(ticks, (u64 __user *)arg))
> > + return -EFAULT;
> > + spin_lock_irq(&ctx->wqh.lock);
> > + ctx->ticks = ticks;
>
> I think we need to wakt up readers here if ctx->ticks isn't zero.
I used this caps to prevent arbitrary changing the ticks from a
regular programs because i didn't wake up readers. But after thinking
more i think indeed this case (write ticks but not wake up anyone)
leads to strange situation -- currently (without the patch) there can't
be scenario when we have a waiter not woken up with nonzero @ticks,
but with this patch applied the timerfd logic become vague. One can
set up @ticks to nonzero value and still have waiter looping around.
Thanks Andrew! How about the patch below? We allow to modify @ticks
and wake up waiter on non-zero @ticks assignment. I think this should
fit current timerfd logic.
(once such or anything else approach get approved -- I'll resend new
series with man updated).
---
From: Cyrill Gorcunov <[email protected]>
Subject: timerfd: Implement timerfd_ioctl method to restore timerfd_ctx::ticks
The 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's state as it was at checkpoint moment we need
a path to bring @ticks back. Initially I thought about writing ticks
back via write() interface but it seems such API is somehow obscure.
Instead implement timerfd_ioctl() method with TFD_IOC_SET_TICKS
command which allows to adjust @ticks into arbitrary value. In
case if new value is non-zero we are waking up the waiters.
I wrapped code with CONFIG_CHECKPOINT_RESTORE which can be
dropped off if there users except c/r camp appear.
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 | 31 +++++++++++++++++++++++++++++++
include/linux/timerfd.h | 5 +++++
2 files changed, 36 insertions(+)
Index: linux-2.6.git/fs/timerfd.c
===================================================================
--- linux-2.6.git.orig/fs/timerfd.c
+++ linux-2.6.git/fs/timerfd.c
@@ -313,11 +313,42 @@ static int timerfd_show(struct seq_file
}
#endif
+#ifdef CONFIG_CHECKPOINT_RESTORE
+static long timerfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+ struct timerfd_ctx *ctx = file->private_data;
+ int ret = 0;
+
+ switch (cmd) {
+ case TFD_IOC_SET_TICKS: {
+ u64 ticks;
+
+ if (get_user(ticks, (u64 __user *)arg))
+ return -EFAULT;
+ spin_lock_irq(&ctx->wqh.lock);
+ ctx->ticks = ticks;
+ if (ticks)
+ wake_up_locked(&ctx->wqh);
+ spin_unlock_irq(&ctx->wqh.lock);
+ break;
+ }
+ default:
+ ret = -ENOTTY;
+ break;
+ }
+
+ return ret;
+}
+#endif
+
static const struct file_operations timerfd_fops = {
.release = timerfd_release,
.poll = timerfd_poll,
.read = timerfd_read,
.llseek = noop_llseek,
+#ifdef CONFIG_CHECKPOINT_RESTORE
+ .unlocked_ioctl = timerfd_ioctl,
+#endif
#ifdef CONFIG_PROC_FS
.show_fdinfo = timerfd_show,
#endif
Index: linux-2.6.git/include/linux/timerfd.h
===================================================================
--- linux-2.6.git.orig/include/linux/timerfd.h
+++ linux-2.6.git/include/linux/timerfd.h
@@ -11,6 +11,9 @@
/* For O_CLOEXEC and O_NONBLOCK */
#include <linux/fcntl.h>
+/* For _IO helpers */
+#include <linux/ioctl.h>
+
/*
* CAREFUL: Check include/asm-generic/fcntl.h when defining
* new flags, since they might collide with O_* ones. We want
@@ -29,4 +32,6 @@
/* Flags for timerfd_settime. */
#define TFD_SETTIME_FLAGS (TFD_TIMER_ABSTIME | TFD_TIMER_CANCEL_ON_SET)
+#define TFD_IOC_SET_TICKS _IOW('T', 0, u64)
+
#endif /* _LINUX_TIMERFD_H */
On Wed, Jun 11, 2014 at 11:51:25AM +0400, Cyrill Gorcunov wrote:
> On Wed, Jun 11, 2014 at 11:27:43AM +0400, Andrew Vagin wrote:
...
> +#ifdef CONFIG_CHECKPOINT_RESTORE
> +static long timerfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> + struct timerfd_ctx *ctx = file->private_data;
> + int ret = 0;
> +
> + switch (cmd) {
> + case TFD_IOC_SET_TICKS: {
> + u64 ticks;
> +
> + if (get_user(ticks, (u64 __user *)arg))
> + return -EFAULT;
> + spin_lock_irq(&ctx->wqh.lock);
> + ctx->ticks = ticks;
> + if (ticks)
> + wake_up_locked(&ctx->wqh);
Setting ticks to zero is equivalent to timerfd_read(), isn't it?
So do we need to re-arme the timer, if it's periodic?
> + spin_unlock_irq(&ctx->wqh.lock);
> + break;
> + }
> + default:
> + ret = -ENOTTY;
> + break;
> + }
> +
> +V return ret;
> +}
> +#endif
On Wed, Jun 11, 2014 at 01:09:15PM +0400, Andrew Vagin wrote:
> On Wed, Jun 11, 2014 at 11:51:25AM +0400, Cyrill Gorcunov wrote:
> > On Wed, Jun 11, 2014 at 11:27:43AM +0400, Andrew Vagin wrote:
>
> ...
>
> > +#ifdef CONFIG_CHECKPOINT_RESTORE
> > +static long timerfd_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> > +{
> > + struct timerfd_ctx *ctx = file->private_data;
> > + int ret = 0;
> > +
> > + switch (cmd) {
> > + case TFD_IOC_SET_TICKS: {
> > + u64 ticks;
> > +
> > + if (get_user(ticks, (u64 __user *)arg))
> > + return -EFAULT;
> > + spin_lock_irq(&ctx->wqh.lock);
> > + ctx->ticks = ticks;
> > + if (ticks)
> > + wake_up_locked(&ctx->wqh);
>
> Setting ticks to zero is equivalent to timerfd_read(), isn't it?
> So do we need to re-arme the timer, if it's periodic?
I must admit I'm not really sure if we should rearm it in such
case. In general @ticks are zeroified in case of timer-setup/cancel/read.
- lets consider someone armed the timer it triggered but no read done
yet, instead ioctl called and @ticks are set to zero, then call for
read() and it returns zero to caller not rearming the timer (in
current patch approach and non-block read)
- in turn if we rearm timer on @ticks = 0 in ioctl this makes it
close to behaviour of read() function (which in turn look to
me as a duplication of read() interface).
That said, I'm not sure yet...
On Wed, Jun 11, 2014 at 01:52:46PM +0400, Cyrill Gorcunov wrote:
> On Wed, Jun 11, 2014 at 01:09:15PM +0400, Andrew Vagin wrote:
> > On Wed, Jun 11, 2014 at 11:51:25AM +0400, Cyrill Gorcunov wrote:
> > > On Wed, Jun 11, 2014 at 11:27:43AM +0400, Andrew Vagin wrote:
> >
> > Setting ticks to zero is equivalent to timerfd_read(), isn't it?
> > So do we need to re-arme the timer, if it's periodic?
>
> I must admit I'm not really sure if we should rearm it in such
> case. In general @ticks are zeroified in case of timer-setup/cancel/read.
>
> - lets consider someone armed the timer it triggered but no read done
> yet, instead ioctl called and @ticks are set to zero, then call for
> read() and it returns zero to caller not rearming the timer (in
> current patch approach and non-block read)
>
> - in turn if we rearm timer on @ticks = 0 in ioctl this makes it
> close to behaviour of read() function (which in turn look to
> me as a duplication of read() interface).
>
> That said, I'm not sure yet...
What if we prohibit setting non-zero values here? @ticks are set to
zero on timerfd_setup thus there is always a way to create a timer
with fields zeroified. Something like
case TFD_IOC_SET_TICKS: {
u64 ticks;
if (get_user(ticks, (u64 __user *)arg))
return -EFAULT;
if (!ticks)
return -EINVAL;
spin_lock_irq(&ctx->wqh.lock);
if (!timerfd_canceled(ctx)) {
ctx->ticks = ticks;
if (ticks)
wake_up_locked(&ctx->wqh);
else
} else
ret = -ECANCELED;
spin_unlock_irq(&ctx->wqh.lock);
break;
}
?