LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [patch 6/13] signal/timer/event fds v9 - timerfd core ...
@ 2007-03-31 20:09 Davide Libenzi
  2007-04-01  9:05 ` Thomas Gleixner
  0 siblings, 1 reply; 8+ messages in thread
From: Davide Libenzi @ 2007-03-31 20:09 UTC (permalink / raw)
  To: Linux Kernel Mailing List; +Cc: Andrew Morton, Linus Torvalds, Thomas Gleixner

This patch introduces a new system call for timers events delivered
though file descriptors. This allows timer event to be used with
standard POSIX poll(2), select(2) and read(2). As a consequence of
supporting the Linux f_op->poll subsystem, they can be used with
epoll(2) too.
The system call is defined as:

int timerfd(int ufd, int clockid, int flags, const struct itimerspec *utmr);

The "ufd" parameter allows for re-use (re-programming) of an existing
timerfd w/out going through the close/open cycle (same as signalfd).
If "ufd" is -1, s new file descriptor will be created, otherwise the
existing "ufd" will be re-programmed.
The "clockid" parameter is either CLOCK_MONOTONIC or CLOCK_REALTIME.
The time specified in the "utmr->it_value" parameter is the expiry
time for the timer.
If the TFD_TIMER_ABSTIME flag is set in "flags", this is an absolute
time, otherwise it's a relative time.
If the time specified in the "utmr->it_interval" is not zero (.tv_sec == 0,
tv_nsec == 0), this is the period at which the following ticks should
be generated.
The "utmr->it_interval" should be set to zero if only one tick is requested.
Setting the "utmr->it_value" to zero will disable the timer, or will create
a timerfd without the timer enabled.
The function returns the new (or same, in case "ufd" is a valid timerfd
descriptor) file, or -1 in case of error.
As stated before, the timerfd file descriptor supports poll(2), select(2)
and epoll(2). When a timer event happened on the timerfd, a POLLIN mask
will be returned.
The read(2) call can be used, and it will return a u32 variable holding
the number of "ticks" that happened on the interface since the last call
to read(2). The read(2) call supportes the O_NONBLOCK flag too, and EAGAIN
will be returned if no ticks happened.
A quick test program, shows timerfd working correctly on my amd64 box:

http://www.xmailserver.org/timerfd-test.c




Signed-off-by: Davide Libenzi <davidel@xmailserver.org>



- Davide



Index: linux-2.6.21-rc5.fds/fs/timerfd.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.21-rc5.fds/fs/timerfd.c	2007-03-31 12:46:14.000000000 -0700
@@ -0,0 +1,233 @@
+/*
+ *  fs/timerfd.c
+ *
+ *  Copyright (C) 2007  Davide Libenzi <davidel@xmailserver.org>
+ *
+ *
+ *  Thanks to Thomas Gleixner for code reviews and useful comments.
+ *
+ */
+
+#include <linux/file.h>
+#include <linux/poll.h>
+#include <linux/slab.h>
+#include <linux/init.h>
+#include <linux/fs.h>
+#include <linux/mount.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/signal.h>
+#include <linux/list.h>
+#include <linux/spinlock.h>
+#include <linux/time.h>
+#include <linux/hrtimer.h>
+#include <linux/jiffies.h>
+#include <linux/anon_inodes.h>
+#include <linux/timerfd.h>
+
+#include <asm/uaccess.h>
+
+
+struct timerfd_ctx {
+	struct hrtimer tmr;
+	ktime_t tintv;
+	spinlock_t lock;
+	wait_queue_head_t wqh;
+	/*
+	 * Every time a timer triggers, we increase "ticks". A read(2)
+	 * will return the current value, and will reset "ticks" to zero.
+	 */
+	u32 ticks;
+};
+
+
+/*
+ * This gets called when the timer event triggers. We increment the
+ * tick count and wake the possible waiters. If the timer in a
+ * sequential one (->tintv.tv64 != 0), we re-arm it with hrtimer_forward().
+ */
+static enum hrtimer_restart timerfd_tmrproc(struct hrtimer *htmr)
+{
+	struct timerfd_ctx *ctx = container_of(htmr, struct timerfd_ctx, tmr);
+	enum hrtimer_restart rval = HRTIMER_NORESTART;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ctx->lock, flags);
+	ctx->ticks++;
+	wake_up_locked(&ctx->wqh);
+	if (ctx->tintv.tv64 != 0) {
+		hrtimer_forward(htmr, hrtimer_cb_get_time(htmr), ctx->tintv);
+		rval = HRTIMER_RESTART;
+	}
+	spin_unlock_irqrestore(&ctx->lock, flags);
+
+	return rval;
+}
+
+static void timerfd_setup(struct timerfd_ctx *ctx, int clockid, int flags,
+			  const struct itimerspec *ktmr)
+{
+	enum hrtimer_mode htmode;
+	ktime_t texp;
+
+	htmode = (flags & TFD_TIMER_ABSTIME) ?
+		HRTIMER_MODE_ABS: HRTIMER_MODE_REL;
+
+	texp = timespec_to_ktime(ktmr->it_value);
+	ctx->ticks = 0;
+	ctx->tintv = timespec_to_ktime(ktmr->it_interval);
+	hrtimer_init(&ctx->tmr, clockid, htmode);
+	ctx->tmr.expires = texp;
+	ctx->tmr.function = timerfd_tmrproc;
+	if (texp.tv64 != 0)
+		hrtimer_start(&ctx->tmr, texp, htmode);
+}
+
+static int timerfd_release(struct inode *inode, struct file *file)
+{
+	struct timerfd_ctx *ctx = file->private_data;
+
+	hrtimer_cancel(&ctx->tmr);
+	kfree(ctx);
+	return 0;
+}
+
+static unsigned int timerfd_poll(struct file *file, poll_table *wait)
+{
+	struct timerfd_ctx *ctx = file->private_data;
+	unsigned int events = 0;
+	unsigned long flags;
+
+	poll_wait(file, &ctx->wqh, wait);
+
+	spin_lock_irqsave(&ctx->lock, flags);
+	if (ctx->ticks)
+		events |= POLLIN;
+	spin_unlock_irqrestore(&ctx->lock, flags);
+
+	return events;
+}
+
+static ssize_t timerfd_read(struct file *file, char __user *buf, size_t count,
+			    loff_t *ppos)
+{
+	struct timerfd_ctx *ctx = file->private_data;
+	ssize_t res;
+	u32 ticks;
+	DECLARE_WAITQUEUE(wait, current);
+
+	if (count < sizeof(ticks))
+		return -EINVAL;
+	spin_lock_irq(&ctx->lock);
+	res = -EAGAIN;
+	ticks = ctx->ticks;
+	if (ticks == 0 && !(file->f_flags & O_NONBLOCK)) {
+		__add_wait_queue(&ctx->wqh, &wait);
+		for (res = 0;;) {
+			set_current_state(TASK_INTERRUPTIBLE);
+			ticks = ctx->ticks;
+			if (ticks != 0) {
+				res = 0;
+				break;
+			}
+			if (signal_pending(current)) {
+				res = -ERESTARTSYS;
+				break;
+			}
+			spin_unlock_irq(&ctx->lock);
+			schedule();
+			spin_lock_irq(&ctx->lock);
+		}
+		__remove_wait_queue(&ctx->wqh, &wait);
+		__set_current_state(TASK_RUNNING);
+	}
+	if (ticks)
+		ctx->ticks = 0;
+	spin_unlock_irq(&ctx->lock);
+	if (ticks)
+		res = put_user(ticks, buf) ? -EFAULT: sizeof(ticks);
+	return res;
+}
+
+
+static const struct file_operations timerfd_fops = {
+	.release	= timerfd_release,
+	.poll		= timerfd_poll,
+	.read		= timerfd_read,
+};
+
+
+asmlinkage long sys_timerfd(int ufd, int clockid, int flags,
+			    const struct itimerspec __user *utmr)
+{
+	int error;
+	struct timerfd_ctx *ctx;
+	struct file *file;
+	struct inode *inode;
+	struct itimerspec ktmr;
+
+	if (copy_from_user(&ktmr, utmr, sizeof(ktmr)))
+		return -EFAULT;
+
+	if (clockid != CLOCK_MONOTONIC &&
+	    clockid != CLOCK_REALTIME)
+		return -EINVAL;
+	if (!timespec_valid(&ktmr.it_value) ||
+	    !timespec_valid(&ktmr.it_interval))
+		return -EINVAL;
+
+	if (ufd == -1) {
+		ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
+		if (!ctx)
+			return -ENOMEM;
+
+		init_waitqueue_head(&ctx->wqh);
+		spin_lock_init(&ctx->lock);
+
+		timerfd_setup(ctx, clockid, flags, &ktmr);
+
+		/*
+		 * When we call this, the initialization must be complete, since
+		 * aino_getfd() will install the fd.
+		 */
+		error = aino_getfd(&ufd, &inode, &file, "[timerfd]",
+				   &timerfd_fops, ctx);
+		if (error)
+			goto err_tmrcancel;
+	} else {
+		file = fget(ufd);
+		if (!file)
+			return -EBADF;
+		ctx = file->private_data;
+		if (file->f_op != &timerfd_fops) {
+			fput(file);
+			return -EINVAL;
+		}
+		/*
+		 * We need to stop the existing timer before reprogramming
+		 * it to the new values.
+		 */
+		for (;;) {
+			spin_lock_irq(&ctx->lock);
+			if (hrtimer_try_to_cancel(&ctx->tmr) >= 0)
+				break;
+			spin_unlock_irq(&ctx->lock);
+			cpu_relax();
+		}
+		/*
+		 * Re-program the timer to the new value ...
+		 */
+		timerfd_setup(ctx, clockid, flags, &ktmr);
+
+		spin_unlock_irq(&ctx->lock);
+		fput(file);
+	}
+
+	return ufd;
+
+err_tmrcancel:
+	hrtimer_cancel(&ctx->tmr);
+	kfree(ctx);
+	return error;
+}
+
Index: linux-2.6.21-rc5.fds/include/linux/timerfd.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6.21-rc5.fds/include/linux/timerfd.h	2007-03-31 12:30:36.000000000 -0700
@@ -0,0 +1,17 @@
+/*
+ *  include/linux/timerfd.h
+ *
+ *  Copyright (C) 2007  Davide Libenzi <davidel@xmailserver.org>
+ *
+ */
+
+#ifndef _LINUX_TIMERFD_H
+#define _LINUX_TIMERFD_H
+
+
+#define TFD_TIMER_ABSTIME (1 << 0)
+
+
+
+#endif /* _LINUX_TIMERFD_H */
+
Index: linux-2.6.21-rc5.fds/include/linux/syscalls.h
===================================================================
--- linux-2.6.21-rc5.fds.orig/include/linux/syscalls.h	2007-03-31 12:29:28.000000000 -0700
+++ linux-2.6.21-rc5.fds/include/linux/syscalls.h	2007-03-31 12:45:37.000000000 -0700
@@ -603,6 +603,8 @@
 				    size_t len);
 asmlinkage long sys_getcpu(unsigned __user *cpu, unsigned __user *node, struct getcpu_cache __user *cache);
 asmlinkage long sys_signalfd(int ufd, sigset_t __user *user_mask, size_t sizemask);
+asmlinkage long sys_timerfd(int ufd, int clockid, int flags,
+			    const struct itimerspec __user *utmr);
 
 int kernel_execve(const char *filename, char *const argv[], char *const envp[]);
 
Index: linux-2.6.21-rc5.fds/fs/Makefile
===================================================================
--- linux-2.6.21-rc5.fds.orig/fs/Makefile	2007-03-31 12:29:28.000000000 -0700
+++ linux-2.6.21-rc5.fds/fs/Makefile	2007-03-31 12:45:37.000000000 -0700
@@ -24,6 +24,7 @@
 obj-$(CONFIG_EPOLL)		+= eventpoll.o
 obj-$(CONFIG_ANON_INODES)	+= anon_inodes.o
 obj-$(CONFIG_SIGNALFD)		+= signalfd.o
+obj-$(CONFIG_TIMERFD)		+= timerfd.o
 obj-$(CONFIG_COMPAT)		+= compat.o compat_ioctl.o
 
 nfsd-$(CONFIG_NFSD)		:= nfsctl.o
Index: linux-2.6.21-rc5.fds/init/Kconfig
===================================================================
--- linux-2.6.21-rc5.fds.orig/init/Kconfig	2007-03-31 12:29:28.000000000 -0700
+++ linux-2.6.21-rc5.fds/init/Kconfig	2007-03-31 12:45:38.000000000 -0700
@@ -483,6 +483,16 @@
 
 	  If unsure, say Y.
 
+config TIMERFD
+	bool "Eanble timerfd() system call" if EMBEDDED
+	depends on ANON_INODES
+	default y
+	help
+	  Enable the timerfd() system call that allows to receive timer
+	  events on a file descriptor.
+
+	  If unsure, say Y.
+
 config SHMEM
 	bool "Use full shmem filesystem" if EMBEDDED
 	default y


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch 6/13] signal/timer/event fds v9 - timerfd core ...
  2007-03-31 20:09 [patch 6/13] signal/timer/event fds v9 - timerfd core Davide Libenzi
@ 2007-04-01  9:05 ` Thomas Gleixner
  2007-04-01 17:00   ` Davide Libenzi
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Gleixner @ 2007-04-01  9:05 UTC (permalink / raw)
  To: Davide Libenzi; +Cc: Linux Kernel Mailing List, Andrew Morton, Linus Torvalds

On Sat, 2007-03-31 at 13:09 -0700, Davide Libenzi wrote:
> +/*
> + * This gets called when the timer event triggers. We increment the
> + * tick count and wake the possible waiters. If the timer in a
> + * sequential one (->tintv.tv64 != 0), we re-arm it with hrtimer_forward().
> + */
> +static enum hrtimer_restart timerfd_tmrproc(struct hrtimer *htmr)
> +{
> +	struct timerfd_ctx *ctx = container_of(htmr, struct timerfd_ctx, tmr);
> +	enum hrtimer_restart rval = HRTIMER_NORESTART;
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&ctx->lock, flags);
> +	ctx->ticks++;
> +	wake_up_locked(&ctx->wqh);
> +	if (ctx->tintv.tv64 != 0) {
> +		hrtimer_forward(htmr, hrtimer_cb_get_time(htmr), ctx->tintv);
> +		rval = HRTIMER_RESTART;
> +	}
> +	spin_unlock_irqrestore(&ctx->lock, flags);
> +
> +	return rval;
> +}

For periodic timers we probably want to know also about missed ticks,
i.e. when the timer was delayed.

I changed recently the rearm handling code of itimers to prevent DoS
attacks. See commit 8bfd9a7a229b5f3d3eda5d7d45c2eebec5b4ba16. The posix
timer code has a similar mechanism.

Probably we should do the same here. That means that we defer the
restart of the timer to the process context.

	tglx

static enum hrtimer_restart timerfd_tmrproc(struct hrtimer *htmr)
{
       struct timerfd_ctx *ctx = container_of(htmr, struct timerfd_ctx, tmr);
       unsigned long flags;

       spin_lock_irqsave(&ctx->lock, flags);
       ctx->expired = 1;
       wake_up_locked(&ctx->wqh);
       spin_unlock_irqrestore(&ctx->lock, flags);

       return HRTIMER_NORESTART;
}

static ssize_t timerfd_read(struct file *file, char __user *buf, size_t count,
                           loff_t *ppos)
{
       struct timerfd_ctx *ctx = file->private_data;
       ssize_t res;
       u32 ticks = 0;
       DECLARE_WAITQUEUE(wait, current);

       if (count < sizeof(ticks))
               return -EINVAL;
       spin_lock_irq(&ctx->lock);
       res = -EAGAIN;
       if (!ctx->expired && !(file->f_flags & O_NONBLOCK)) {
               __add_wait_queue(&ctx->wqh, &wait);
               for (res = 0;;) {
                       set_current_state(TASK_INTERRUPTIBLE);
                       if (ctx->expired) {
                               res = 0;
                               break;
                       }
                       if (signal_pending(current)) {
                               res = -ERESTARTSYS;
                               break;
                       }
                       spin_unlock_irq(&ctx->lock);
                       schedule();
                       spin_lock_irq(&ctx->lock);
               }
               __remove_wait_queue(&ctx->wqh, &wait);
               __set_current_state(TASK_RUNNING);
       }
       if (ctx->expired) {
               	ctx->expired = 0;
		if (ctx->tintv.tv64 != 0) {
			ticks = hrtimer_forward(&ctx->tmr, ktime_get(),
						ctx->tintv);
			hrtimer_restart(&ctx->tmr);
		} else
			ticks = 1;
	}
       spin_unlock_irq(&ctx->lock);
       if (ticks)
               res = put_user(ticks, buf) ? -EFAULT: sizeof(ticks);
       return res;
}



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch 6/13] signal/timer/event fds v9 - timerfd core ...
  2007-04-01  9:05 ` Thomas Gleixner
@ 2007-04-01 17:00   ` Davide Libenzi
  2007-04-01 17:08     ` Davide Libenzi
  2007-04-02  7:22     ` Thomas Gleixner
  0 siblings, 2 replies; 8+ messages in thread
From: Davide Libenzi @ 2007-04-01 17:00 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Linux Kernel Mailing List, Andrew Morton, Linus Torvalds

On Sun, 1 Apr 2007, Thomas Gleixner wrote:

> On Sat, 2007-03-31 at 13:09 -0700, Davide Libenzi wrote:
> > +/*
> > + * This gets called when the timer event triggers. We increment the
> > + * tick count and wake the possible waiters. If the timer in a
> > + * sequential one (->tintv.tv64 != 0), we re-arm it with hrtimer_forward().
> > + */
> > +static enum hrtimer_restart timerfd_tmrproc(struct hrtimer *htmr)
> > +{
> > +	struct timerfd_ctx *ctx = container_of(htmr, struct timerfd_ctx, tmr);
> > +	enum hrtimer_restart rval = HRTIMER_NORESTART;
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&ctx->lock, flags);
> > +	ctx->ticks++;
> > +	wake_up_locked(&ctx->wqh);
> > +	if (ctx->tintv.tv64 != 0) {
> > +		hrtimer_forward(htmr, hrtimer_cb_get_time(htmr), ctx->tintv);
> > +		rval = HRTIMER_RESTART;
> > +	}
> > +	spin_unlock_irqrestore(&ctx->lock, flags);
> > +
> > +	return rval;
> > +}
> 
> For periodic timers we probably want to know also about missed ticks,
> i.e. when the timer was delayed.
> 
> I changed recently the rearm handling code of itimers to prevent DoS
> attacks. See commit 8bfd9a7a229b5f3d3eda5d7d45c2eebec5b4ba16. The posix
> timer code has a similar mechanism.
> 
> Probably we should do the same here. That means that we defer the
> restart of the timer to the process context.

But timerfd has that implicit, in the ticks counter. So if you get ticks > 1
it means you did not read-out (miss) one or more.
By re-arming the timer whenever you read it, that may be some time after 
it expired. IMO the behaviour is better as it is now, and if you want the 
new timer sequence start at a new timeframe, you just use timerfd with the 
same fd.
I do not know about DoS on timers (did not follow the thread), but I could 
easily implement it here, by capping the counter to some value, and return 
POLLERR in poll().
I'd prefer that instead of the re-arm on read thing.


- Davide



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch 6/13] signal/timer/event fds v9 - timerfd core ...
  2007-04-01 17:00   ` Davide Libenzi
@ 2007-04-01 17:08     ` Davide Libenzi
  2007-04-02  7:22     ` Thomas Gleixner
  1 sibling, 0 replies; 8+ messages in thread
From: Davide Libenzi @ 2007-04-01 17:08 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Linux Kernel Mailing List, Andrew Morton, Linus Torvalds

On Sun, 1 Apr 2007, Davide Libenzi wrote:

> I do not know about DoS on timers (did not follow the thread), but I could 
> easily implement it here, by capping the counter to some value, and return 
> POLLERR in poll().

For capping I mean, I won't re-arm the timer if ticks exceeded it ...


- Davide



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch 6/13] signal/timer/event fds v9 - timerfd core ...
  2007-04-01 17:00   ` Davide Libenzi
  2007-04-01 17:08     ` Davide Libenzi
@ 2007-04-02  7:22     ` Thomas Gleixner
  2007-04-02 17:30       ` Davide Libenzi
  1 sibling, 1 reply; 8+ messages in thread
From: Thomas Gleixner @ 2007-04-02  7:22 UTC (permalink / raw)
  To: Davide Libenzi; +Cc: Linux Kernel Mailing List, Andrew Morton, Linus Torvalds

On Sun, 2007-04-01 at 10:00 -0700, Davide Libenzi wrote:
> > For periodic timers we probably want to know also about missed ticks,
> > i.e. when the timer was delayed.
> > 
> > I changed recently the rearm handling code of itimers to prevent DoS
> > attacks. See commit 8bfd9a7a229b5f3d3eda5d7d45c2eebec5b4ba16. The posix
> > timer code has a similar mechanism.
> > 
> > Probably we should do the same here. That means that we defer the
> > restart of the timer to the process context.
> 
> But timerfd has that implicit, in the ticks counter. So if you get ticks > 1
> it means you did not read-out (miss) one or more.
> By re-arming the timer whenever you read it, that may be some time after 
> it expired. IMO the behaviour is better as it is now, and if you want the 
> new timer sequence start at a new timeframe, you just use timerfd with the 
> same fd.
> I do not know about DoS on timers (did not follow the thread), but I could 
> easily implement it here, by capping the counter to some value, and return 
> POLLERR in poll().
> I'd prefer that instead of the re-arm on read thing.

The DoS is simple: create a bunch of periodic timers with 10usec period.
This can be done by any user.

There is no inaccuracy when you rearm the timer on read: hrtimer_forward
takes care, that the period is accurate. It does not start the timer out
of the periodic order, i.e. on a different time frame.

Where is the win of keeping the timer running, when nobody cares about
the expiry at all ? It just generates interrupts and events for nothing.

	tglx





^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch 6/13] signal/timer/event fds v9 - timerfd core ...
  2007-04-02  7:22     ` Thomas Gleixner
@ 2007-04-02 17:30       ` Davide Libenzi
  2007-04-02 17:47         ` Thomas Gleixner
  0 siblings, 1 reply; 8+ messages in thread
From: Davide Libenzi @ 2007-04-02 17:30 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Linux Kernel Mailing List, Andrew Morton, Linus Torvalds

On Mon, 2 Apr 2007, Thomas Gleixner wrote:

> The DoS is simple: create a bunch of periodic timers with 10usec period.
> This can be done by any user.

Ok, that's what I immagined. Agreed, that's a problem.



> There is no inaccuracy when you rearm the timer on read: hrtimer_forward
> takes care, that the period is accurate. It does not start the timer out
> of the periodic order, i.e. on a different time frame.
> 
> Where is the win of keeping the timer running, when nobody cares about
> the expiry at all ? It just generates interrupts and events for nothing.

Then you'd lose the ability to know if you lost one or more (yes, you 
could figure it out by reading the time and with a few calculations). I 
think that the capping (to a sane value) idea solves the DoS issue and at 
the same time have the ability to report you missed ticks. What are your 
strong points against that solution?



- Davide



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch 6/13] signal/timer/event fds v9 - timerfd core ...
  2007-04-02 17:30       ` Davide Libenzi
@ 2007-04-02 17:47         ` Thomas Gleixner
  2007-04-02 17:52           ` Davide Libenzi
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Gleixner @ 2007-04-02 17:47 UTC (permalink / raw)
  To: Davide Libenzi; +Cc: Linux Kernel Mailing List, Andrew Morton, Linus Torvalds

On Mon, 2007-04-02 at 10:30 -0700, Davide Libenzi wrote:

> > There is no inaccuracy when you rearm the timer on read: hrtimer_forward
> > takes care, that the period is accurate. It does not start the timer out
> > of the periodic order, i.e. on a different time frame.
> > 
> > Where is the win of keeping the timer running, when nobody cares about
> > the expiry at all ? It just generates interrupts and events for nothing.
> 
> Then you'd lose the ability to know if you lost one or more (yes, you 
> could figure it out by reading the time and with a few calculations). I 
> think that the capping (to a sane value) idea solves the DoS issue and at 
> the same time have the ability to report you missed ticks. What are your 
> strong points against that solution?

Err, the read function 

	ticks = hrtimer_forward(&ctx->tmr, ktime_get(),
                                ctx->tintv);

does give you the number of (lost) ticks.

tmr->expires holds the absolute expiry time of the last event.
hrtimer_forward() adds N intervals to tmr->expires, so that the new
tmr->expires value is greater than now (ktime_get()). It returns N.

So the number of lost ticks is N - 1. No time reading and no magic
math :)

	tglx



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [patch 6/13] signal/timer/event fds v9 - timerfd core ...
  2007-04-02 17:47         ` Thomas Gleixner
@ 2007-04-02 17:52           ` Davide Libenzi
  0 siblings, 0 replies; 8+ messages in thread
From: Davide Libenzi @ 2007-04-02 17:52 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Linux Kernel Mailing List, Andrew Morton, Linus Torvalds

On Mon, 2 Apr 2007, Thomas Gleixner wrote:

> On Mon, 2007-04-02 at 10:30 -0700, Davide Libenzi wrote:
> 
> > > There is no inaccuracy when you rearm the timer on read: hrtimer_forward
> > > takes care, that the period is accurate. It does not start the timer out
> > > of the periodic order, i.e. on a different time frame.
> > > 
> > > Where is the win of keeping the timer running, when nobody cares about
> > > the expiry at all ? It just generates interrupts and events for nothing.
> > 
> > Then you'd lose the ability to know if you lost one or more (yes, you 
> > could figure it out by reading the time and with a few calculations). I 
> > think that the capping (to a sane value) idea solves the DoS issue and at 
> > the same time have the ability to report you missed ticks. What are your 
> > strong points against that solution?
> 
> Err, the read function 
> 
> 	ticks = hrtimer_forward(&ctx->tmr, ktime_get(),
>                                 ctx->tintv);
> 
> does give you the number of (lost) ticks.
> 
> tmr->expires holds the absolute expiry time of the last event.
> hrtimer_forward() adds N intervals to tmr->expires, so that the new
> tmr->expires value is greater than now (ktime_get()). It returns N.
> 
> So the number of lost ticks is N - 1. No time reading and no magic
> math :)

Yack, I missed that part :) Sounds fine then.



- Davide



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2007-04-02 17:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-03-31 20:09 [patch 6/13] signal/timer/event fds v9 - timerfd core Davide Libenzi
2007-04-01  9:05 ` Thomas Gleixner
2007-04-01 17:00   ` Davide Libenzi
2007-04-01 17:08     ` Davide Libenzi
2007-04-02  7:22     ` Thomas Gleixner
2007-04-02 17:30       ` Davide Libenzi
2007-04-02 17:47         ` Thomas Gleixner
2007-04-02 17:52           ` Davide Libenzi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).