[patch 2/4] Timerfd v3 - new timerfd API

[patch 2/4] Timerfd v3 - new timerfd API

[Davide Libenzi]

This is the new timerfd API as it is implemented by the following patch:

int timerfd_create(int clockid, int flags);
int timerfd_settime(int ufd, int flags,
		    const struct itimerspec *utmr,
		    struct itimerspec *otmr);
int timerfd_gettime(int ufd, struct itimerspec *otmr);

The timerfd_create() API creates an un-programmed timerfd fd. The "clockid"
parameter can be either CLOCK_MONOTONIC or CLOCK_REALTIME.
The timerfd_settime() API give new settings by the timerfd fd, by optionally
retrieving the previous expiration time (in case the "otmr" parameter is not NULL).
The time value specified in "utmr" is absolute, if the TFD_TIMER_ABSTIME bit
is set in the "flags" parameter. Otherwise it's a relative time.
The timerfd_gettime() API returns the next expiration time of the timer, or {0, 0}
if the timerfd has not been set yet.
Like the previous timerfd API implementation, read(2) and poll(2) are supported
(with the same interface).
Here's a simple test program I used to exercise the new timerfd APIs:

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



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


- Davide


---
 fs/compat.c              |   32 ++++++-
 fs/timerfd.c             |  199 ++++++++++++++++++++++++++++++-----------------
 include/linux/compat.h   |    7 +
 include/linux/syscalls.h |    7 +
 4 files changed, 166 insertions(+), 79 deletions(-)

Index: linux-2.6.mod/fs/timerfd.c
===================================================================
--- linux-2.6.mod.orig/fs/timerfd.c	2007-11-23 13:55:16.000000000 -0800
+++ linux-2.6.mod/fs/timerfd.c	2007-11-24 12:49:21.000000000 -0800
@@ -25,13 +25,15 @@
 	struct hrtimer tmr;
 	ktime_t tintv;
 	wait_queue_head_t wqh;
+	u64 ticks;
 	int expired;
+	int clockid;
 };
 
 /*
  * This gets called when the timer event triggers. We set the "expired"
  * flag, but we do not re-arm the timer (in case it's necessary,
- * tintv.tv64 != 0) until the timer is read.
+ * tintv.tv64 != 0) until the timer is accessed.
  */
 static enum hrtimer_restart timerfd_tmrproc(struct hrtimer *htmr)
 {
@@ -40,13 +42,14 @@
 
 	spin_lock_irqsave(&ctx->wqh.lock, flags);
 	ctx->expired = 1;
+	ctx->ticks++;
 	wake_up_locked(&ctx->wqh);
 	spin_unlock_irqrestore(&ctx->wqh.lock, flags);
 
 	return HRTIMER_NORESTART;
 }
 
-static void timerfd_setup(struct timerfd_ctx *ctx, int clockid, int flags,
+static void timerfd_setup(struct timerfd_ctx *ctx, int flags,
 			  const struct itimerspec *ktmr)
 {
 	enum hrtimer_mode htmode;
@@ -57,8 +60,9 @@
 
 	texp = timespec_to_ktime(ktmr->it_value);
 	ctx->expired = 0;
+	ctx->ticks = 0;
 	ctx->tintv = timespec_to_ktime(ktmr->it_interval);
-	hrtimer_init(&ctx->tmr, clockid, htmode);
+	hrtimer_init(&ctx->tmr, ctx->clockid, htmode);
 	ctx->tmr.expires = texp;
 	ctx->tmr.function = timerfd_tmrproc;
 	if (texp.tv64 != 0)
@@ -83,7 +87,7 @@
 	poll_wait(file, &ctx->wqh, wait);
 
 	spin_lock_irqsave(&ctx->wqh.lock, flags);
-	if (ctx->expired)
+	if (ctx->ticks)
 		events |= POLLIN;
 	spin_unlock_irqrestore(&ctx->wqh.lock, flags);
 
@@ -102,11 +106,11 @@
 		return -EINVAL;
 	spin_lock_irq(&ctx->wqh.lock);
 	res = -EAGAIN;
-	if (!ctx->expired && !(file->f_flags & O_NONBLOCK)) {
+	if (!ctx->ticks && !(file->f_flags & O_NONBLOCK)) {
 		__add_wait_queue(&ctx->wqh, &wait);
 		for (res = 0;;) {
 			set_current_state(TASK_INTERRUPTIBLE);
-			if (ctx->expired) {
+			if (ctx->ticks) {
 				res = 0;
 				break;
 			}
@@ -121,22 +125,21 @@
 		__remove_wait_queue(&ctx->wqh, &wait);
 		__set_current_state(TASK_RUNNING);
 	}
-	if (ctx->expired) {
-		ctx->expired = 0;
-		if (ctx->tintv.tv64 != 0) {
+	if (ctx->ticks) {
+		ticks = ctx->ticks;
+		if (ctx->expired && ctx->tintv.tv64) {
 			/*
 			 * If tintv.tv64 != 0, this is a periodic timer that
 			 * needs to be re-armed. We avoid doing it in the timer
 			 * callback to avoid DoS attacks specifying a very
 			 * short timer period.
 			 */
-			ticks = (u64)
-				hrtimer_forward(&ctx->tmr,
-						hrtimer_cb_get_time(&ctx->tmr),
-						ctx->tintv);
+			ticks += (u64) hrtimer_forward_now(&ctx->tmr,
+							   ctx->tintv) - 1;
 			hrtimer_restart(&ctx->tmr);
-		} else
-			ticks = 1;
+		}
+		ctx->expired = 0;
+		ctx->ticks = 0;
 	}
 	spin_unlock_irq(&ctx->wqh.lock);
 	if (ticks)
@@ -150,76 +153,132 @@
 	.read		= timerfd_read,
 };
 
-asmlinkage long sys_timerfd(int ufd, int clockid, int flags,
-			    const struct itimerspec __user *utmr)
+static struct file *timerfd_fget(int fd)
+{
+	struct file *file;
+
+	file = fget(fd);
+	if (!file)
+		return ERR_PTR(-EBADF);
+	if (file->f_op != &timerfd_fops) {
+		fput(file);
+		return ERR_PTR(-EINVAL);
+	}
+
+	return file;
+}
+
+asmlinkage long sys_timerfd_create(int clockid, int flags)
 {
-	int error;
+	int error, ufd;
 	struct timerfd_ctx *ctx;
 	struct file *file;
 	struct inode *inode;
-	struct itimerspec ktmr;
-
-	if (copy_from_user(&ktmr, utmr, sizeof(ktmr)))
-		return -EFAULT;
 
+	if (flags)
+		return -EINVAL;
 	if (clockid != CLOCK_MONOTONIC &&
 	    clockid != CLOCK_REALTIME)
 		return -EINVAL;
+
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	init_waitqueue_head(&ctx->wqh);
+	ctx->clockid = clockid;
+	hrtimer_init(&ctx->tmr, clockid, HRTIMER_MODE_ABS);
+
+	error = anon_inode_getfd(&ufd, &inode, &file, "[timerfd]",
+				 &timerfd_fops, ctx);
+	if (error) {
+		kfree(ctx);
+		return error;
+	}
+
+	return ufd;
+}
+
+asmlinkage long sys_timerfd_settime(int ufd, int flags,
+				    const struct itimerspec __user *utmr,
+				    struct itimerspec __user *otmr)
+{
+	struct file *file;
+	struct timerfd_ctx *ctx;
+	struct itimerspec ktmr, kotmr;
+
+	if (copy_from_user(&ktmr, utmr, sizeof(ktmr)))
+		return -EFAULT;
+
 	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);
-
-		timerfd_setup(ctx, clockid, flags, &ktmr);
-
-		/*
-		 * When we call this, the initialization must be complete, since
-		 * anon_inode_getfd() will install the fd.
-		 */
-		error = anon_inode_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->wqh.lock);
-			if (hrtimer_try_to_cancel(&ctx->tmr) >= 0)
-				break;
-			spin_unlock_irq(&ctx->wqh.lock);
-			cpu_relax();
-		}
-		/*
-		 * Re-program the timer to the new value ...
-		 */
-		timerfd_setup(ctx, clockid, flags, &ktmr);
-
+	file = timerfd_fget(ufd);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+	ctx = file->private_data;
+
+	/*
+	 * We need to stop the existing timer before reprogramming
+	 * it to the new values.
+	 */
+	for (;;) {
+		spin_lock_irq(&ctx->wqh.lock);
+		if (hrtimer_try_to_cancel(&ctx->tmr) >= 0)
+			break;
 		spin_unlock_irq(&ctx->wqh.lock);
-		fput(file);
+		cpu_relax();
 	}
 
-	return ufd;
+	/*
+	 * If the timer is expired and it's periodic, we need to advance it
+	 * because the caller may want to know the previous expiration time.
+	 * We do not update "ticks" and "expired" since the timer will be
+	 * re-programmed again in the following timerfd_setup() call.
+	 */
+	if (ctx->expired && ctx->tintv.tv64)
+		hrtimer_forward_now(&ctx->tmr, ctx->tintv);
+
+	kotmr.it_value = ktime_to_timespec(ctx->tmr.expires);
+	kotmr.it_interval = ktime_to_timespec(ctx->tintv);
+
+	/*
+	 * Re-program the timer to the new value ...
+	 */
+	timerfd_setup(ctx, flags, &ktmr);
 
-err_tmrcancel:
-	hrtimer_cancel(&ctx->tmr);
-	kfree(ctx);
-	return error;
+	spin_unlock_irq(&ctx->wqh.lock);
+	fput(file);
+	if (otmr && copy_to_user(otmr, &kotmr, sizeof(kotmr)))
+		return -EFAULT;
+
+	return 0;
+}
+
+asmlinkage long sys_timerfd_gettime(int ufd, struct itimerspec __user *otmr)
+{
+	struct file *file;
+	struct timerfd_ctx *ctx;
+	struct itimerspec kotmr;
+
+	file = timerfd_fget(ufd);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+	ctx = file->private_data;
+
+	spin_lock_irq(&ctx->wqh.lock);
+	if (ctx->expired && ctx->tintv.tv64) {
+		ctx->expired = 0;
+		ctx->ticks += (u64)
+			hrtimer_forward_now(&ctx->tmr, ctx->tintv) - 1;
+		hrtimer_restart(&ctx->tmr);
+	}
+	kotmr.it_value = ktime_to_timespec(ctx->tmr.expires);
+	kotmr.it_interval = ktime_to_timespec(ctx->tintv);
+	spin_unlock_irq(&ctx->wqh.lock);
+	fput(file);
+
+	return copy_to_user(otmr, &kotmr, sizeof(kotmr)) ? -EFAULT: 0;
 }
 
Index: linux-2.6.mod/include/linux/syscalls.h
===================================================================
--- linux-2.6.mod.orig/include/linux/syscalls.h	2007-11-23 13:55:16.000000000 -0800
+++ linux-2.6.mod/include/linux/syscalls.h	2007-11-24 12:48:10.000000000 -0800
@@ -607,8 +607,11 @@
 				    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);
+asmlinkage long sys_timerfd_create(int clockid, int flags);
+asmlinkage long sys_timerfd_settime(int ufd, int flags,
+				    const struct itimerspec __user *utmr,
+				    struct itimerspec __user *otmr);
+asmlinkage long sys_timerfd_gettime(int ufd, struct itimerspec __user *otmr);
 asmlinkage long sys_eventfd(unsigned int count);
 asmlinkage long sys_fallocate(int fd, int mode, loff_t offset, loff_t len);
 
Index: linux-2.6.mod/fs/compat.c
===================================================================
--- linux-2.6.mod.orig/fs/compat.c	2007-11-23 13:55:16.000000000 -0800
+++ linux-2.6.mod/fs/compat.c	2007-11-24 12:48:10.000000000 -0800
@@ -2210,19 +2210,41 @@
 
 #ifdef CONFIG_TIMERFD
 
-asmlinkage long compat_sys_timerfd(int ufd, int clockid, int flags,
-				   const struct compat_itimerspec __user *utmr)
+asmlinkage long compat_sys_timerfd_settime(int ufd, int flags,
+				   const struct compat_itimerspec __user *utmr,
+				   struct compat_itimerspec __user *otmr)
 {
+	int error;
 	struct itimerspec t;
 	struct itimerspec __user *ut;
 
 	if (get_compat_itimerspec(&t, utmr))
 		return -EFAULT;
-	ut = compat_alloc_user_space(sizeof(*ut));
-	if (copy_to_user(ut, &t, sizeof(t)))
+	ut = compat_alloc_user_space(2 * sizeof(struct itimerspec));
+	if (copy_to_user(&ut[0], &t, sizeof(t)))
 		return -EFAULT;
+	error = sys_timerfd_settime(ufd, flags, &ut[0], &ut[1]);
+	if (!error && otmr)
+		error = (copy_from_user(&t, &ut[1], sizeof(struct itimerspec)) ||
+			 put_compat_itimerspec(otmr, &t)) ? -EFAULT: 0;
 
-	return sys_timerfd(ufd, clockid, flags, ut);
+	return error;
+}
+
+asmlinkage long compat_sys_timerfd_gettime(int ufd,
+				   struct compat_itimerspec __user *otmr)
+{
+	int error;
+	struct itimerspec t;
+	struct itimerspec __user *ut;
+
+	ut = compat_alloc_user_space(sizeof(struct itimerspec));
+	error = sys_timerfd_gettime(ufd, ut);
+	if (!error)
+		error = (copy_from_user(&t, ut, sizeof(struct itimerspec)) ||
+			 put_compat_itimerspec(otmr, &t)) ? -EFAULT: 0;
+
+	return error;
 }
 
 #endif /* CONFIG_TIMERFD */
Index: linux-2.6.mod/include/linux/compat.h
===================================================================
--- linux-2.6.mod.orig/include/linux/compat.h	2007-11-23 13:55:16.000000000 -0800
+++ linux-2.6.mod/include/linux/compat.h	2007-11-24 12:48:10.000000000 -0800
@@ -264,8 +264,11 @@
 asmlinkage long compat_sys_signalfd(int ufd,
 				const compat_sigset_t __user *sigmask,
                                 compat_size_t sigsetsize);
-asmlinkage long compat_sys_timerfd(int ufd, int clockid, int flags,
-				const struct compat_itimerspec __user *utmr);
+asmlinkage long compat_sys_timerfd_settime(int ufd, int flags,
+				   const struct compat_itimerspec __user *utmr,
+				   struct compat_itimerspec __user *otmr);
+asmlinkage long compat_sys_timerfd_gettime(int ufd,
+				   struct compat_itimerspec __user *otmr);
 
 #endif /* CONFIG_COMPAT */
 #endif /* _LINUX_COMPAT_H */

Re: [patch 2/4] Timerfd v3 - new timerfd API

[Andrew Morton]

On Sun, 25 Nov 2007 14:14:19 -0800 Davide Libenzi <davidel@xmailserver.org> wrote:

> +static struct file *timerfd_fget(int fd)
> +{
> +	struct file *file;
> +
> +	file = fget(fd);
> +	if (!file)
> +		return ERR_PTR(-EBADF);
> +	if (file->f_op != &timerfd_fops) {
> +		fput(file);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	return file;
> +}

I suppose we could use fget_light() in here sometime.  It is significantly
quicker in microbenchmarks.  Or it was - nobody has checked that in a few
years afaik.

Re: [patch 2/4] Timerfd v3 - new timerfd API

[Davide Libenzi]

On Tue, 27 Nov 2007, Andrew Morton wrote:

> On Sun, 25 Nov 2007 14:14:19 -0800 Davide Libenzi <davidel@xmailserver.org> wrote:
> 
> > +static struct file *timerfd_fget(int fd)
> > +{
> > +	struct file *file;
> > +
> > +	file = fget(fd);
> > +	if (!file)
> > +		return ERR_PTR(-EBADF);
> > +	if (file->f_op != &timerfd_fops) {
> > +		fput(file);
> > +		return ERR_PTR(-EINVAL);
> > +	}
> > +
> > +	return file;
> > +}
> 
> I suppose we could use fget_light() in here sometime.  It is significantly
> quicker in microbenchmarks.  Or it was - nobody has checked that in a few
> years afaik.

Should I now? Half of the internet traffic of the last three month was 
generated by me posting those timerfd patches :)



- Davide

Re: [patch 2/4] Timerfd v3 - new timerfd API

[Andrew Morton]

On Tue, 27 Nov 2007 12:47:46 -0800 (PST)
Davide Libenzi <davidel@xmailserver.org> wrote:

> On Tue, 27 Nov 2007, Andrew Morton wrote:
> 
> > On Sun, 25 Nov 2007 14:14:19 -0800 Davide Libenzi <davidel@xmailserver.org> wrote:
> > 
> > > +static struct file *timerfd_fget(int fd)
> > > +{
> > > +	struct file *file;
> > > +
> > > +	file = fget(fd);
> > > +	if (!file)
> > > +		return ERR_PTR(-EBADF);
> > > +	if (file->f_op != &timerfd_fops) {
> > > +		fput(file);
> > > +		return ERR_PTR(-EINVAL);
> > > +	}
> > > +
> > > +	return file;
> > > +}
> > 
> > I suppose we could use fget_light() in here sometime.  It is significantly
> > quicker in microbenchmarks.  Or it was - nobody has checked that in a few
> > years afaik.
> 
> Should I now?

No great rush.  It'd be fun to see if it actually makes any measurable
difference on modern hardware (hint ;)).

> Half of the internet traffic of the last three month was 
> generated by me posting those timerfd patches :)

Which means my inbox gets 3000% of the internet.  Thought so.

Re: [patch 2/4] Timerfd v3 - new timerfd API

[Davide Libenzi]

On Tue, 27 Nov 2007, Andrew Morton wrote:

> On Tue, 27 Nov 2007 12:47:46 -0800 (PST)
> Davide Libenzi <davidel@xmailserver.org> wrote:
> 
> > On Tue, 27 Nov 2007, Andrew Morton wrote:
> > 
> > > On Sun, 25 Nov 2007 14:14:19 -0800 Davide Libenzi <davidel@xmailserver.org> wrote:
> > > 
> > > > +static struct file *timerfd_fget(int fd)
> > > > +{
> > > > +	struct file *file;
> > > > +
> > > > +	file = fget(fd);
> > > > +	if (!file)
> > > > +		return ERR_PTR(-EBADF);
> > > > +	if (file->f_op != &timerfd_fops) {
> > > > +		fput(file);
> > > > +		return ERR_PTR(-EINVAL);
> > > > +	}
> > > > +
> > > > +	return file;
> > > > +}
> > > 
> > > I suppose we could use fget_light() in here sometime.  It is significantly
> > > quicker in microbenchmarks.  Or it was - nobody has checked that in a few
> > > years afaik.
> > 
> > Should I now?
> 
> No great rush.  It'd be fun to see if it actually makes any measurable
> difference on modern hardware (hint ;)).

I was going to say BS, given that at the time of the tests, the files 
struct was protected by an rwlock whereas now there's a rcu one.
But that seems not the case:

http://www.xmailserver.org/fget-light-test.c


$ fget-light-test -r 900000000   
warming up ...
testing non-shared ...
time = 314.354000 ms
testing shared ...
time = 390.781000 ms



And here is the oprofile output:

[SHARED CASE]
samples  %        app name                 symbol name
7436     28.9339  vmlinux                  __clear_user
2369      9.2179  vmlinux                  system_call
1710      6.6537  vmlinux                  fget_light
1244      4.8405  vmlinux                  inotify_dentry_parent_queue_event
1128      4.3891  vmlinux                  sys_read
1041      4.0506  libpthread-2.6.1.so      __read_nocancel
1027      3.9961  Xorg                     (no symbols)
978       3.8054  vmlinux                  read_zero
755       2.9377  vmlinux                  vfs_read
545       2.1206  vmlinux                  inotify_inode_queue_event
507       1.9728  vmlinux                  sysret_check
414       1.6109  vmlinux                  rw_verify_area
405       1.5759  vmlinux                  unix_poll
371       1.4436  nvidia                   (no symbols)
333       1.2957  vmlinux                  acpi_pm_read
311       1.2101  nvidia_drv.so            (no symbols)
290       1.1284  vmlinux                  do_select
288       1.1206  vmlinux                  dnotify_parent
253       0.9844  libc-2.6.1.so            (no symbols)
232       0.9027  bash                     (no symbols)
216       0.8405  libpthread-2.6.1.so      __pthread_enable_asynccancel


[UNSHARED CASE]
samples  %        app name                 symbol name
6542     27.6922  vmlinux                  __clear_user
4074     17.2452  vmlinux                  vfs_read
1266      5.3590  vmlinux                  inotify_inode_queue_event
1091      4.6182  Xorg                     (no symbols)
1059      4.4827  vmlinux                  system_call
937       3.9663  libpthread-2.6.1.so      __read_nocancel
544       2.3027  vmlinux                  clear_user
484       2.0488  vmlinux                  dnotify_parent
445       1.8837  vmlinux                  read_zero
438       1.8540  vmlinux                  sysret_check
414       1.7525  vmlinux                  unix_poll
407       1.7228  nvidia                   (no symbols)
389       1.6466  vmlinux                  acpi_pm_read
315       1.3334  nvidia_drv.so            (no symbols)
312       1.3207  vmlinux                  inotify_dentry_parent_queue_event
312       1.3207  vmlinux                  sys_read
305       1.2911  vmlinux                  rw_verify_area
304       1.2868  vmlinux                  do_select
222       0.9397  libc-2.6.1.so            (no symbols)
214       0.9059  bash                     (no symbols)
196       0.8297  fget-light-test          read_test
185       0.7831  vmlinux                  fget_light



You can clearly notice the fget_light() drop out of the relevance window.
BTW, are all those "notify" noises supposed to be there?




- Davide