Re: [patch 3/3] timerfd: Implement write method

3 messages, 3 authors, 2014-06-10 · open the first message on its own page

Re: [patch 3/3] timerfd: Implement write method

From: Michael Kerrisk (man-pages) <hidden>
Date: 2014-06-10 20:03:37

[CC += linux-api@]

On Tue, Jun 10, 2014 at 6:35 PM, Cyrill Gorcunov [off-list ref] wrote:
quoted hunk
On Thu, May 22, 2014 at 06:58:19AM +0900, Thomas Gleixner wrote:
quoted
quoted
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 <redacted>
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 <redacted>
CC: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
CC: Andrey Vagin <redacted>
CC: Pavel Emelyanov <redacted>
CC: Vladimir Davydov <redacted>
Signed-off-by: Cyrill Gorcunov <redacted>
---
 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/

Re: [patch 3/3] timerfd: Implement write method

From: Andy Lutomirski <luto@amacapital.net>
Date: 2014-06-10 20:05:45

On Tue, Jun 10, 2014 at 1:03 PM, Michael Kerrisk (man-pages)
[off-list ref] wrote:
[CC += linux-api@]

On Tue, Jun 10, 2014 at 6:35 PM, Cyrill Gorcunov [off-list ref] wrote:
quoted
On Thu, May 22, 2014 at 06:58:19AM +0900, Thomas Gleixner wrote:
quoted
quoted
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 <redacted>
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

Re: [patch 3/3] timerfd: Implement write method

From: Cyrill Gorcunov <hidden>
Date: 2014-06-10 20:22:37

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)
[off-list ref] wrote:
quoted
[CC += linux-api@]
Thanks Michael!
quoted
On Tue, Jun 10, 2014 at 6:35 PM, Cyrill Gorcunov [off-list ref] wrote:
quoted
On Thu, May 22, 2014 at 06:58:19AM +0900, Thomas Gleixner wrote:
quoted
quoted
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 <redacted>
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).
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help