[PATCH v3 0/1] Generalize poll events from eventfd

[PATCH v3 0/1] Generalize poll events from eventfd

[Damian Hobson-Garcia]

Using eventfd user space can generate POLLIN/POLLOUT events but some
applications may want to generate POLLPRI/POLLERR events as well.
This patch submission aims to generalize the events generated by an
eventfd. This is a resubmission of a patch from Feb 2013[1]. The original
discussion trailed off without any conclusion, but the original author
has recently confirmed[2] that this functionality is still useful, so I
volunteered to rebase and resubmit the patch for discussion.

[1] https://lkml.org/lkml/2013/2/18/147
[2] https://lkml.org/lkml/2015/7/9/153

Changes in v3
-------------
* replace efd_mask structure with scalar 'events' variable.

Changes in v2
-------------

* rebased on Linux v4.3-rc1
* Move file operation implementations for EFD_MASK to a seperate structure
* Remove 'data' element from efd_mask structure
* read() is no longer supported when EFD_MASK is set (fails with EINVAL)
* eventfd_ctx_fileget() now returns EINVAL when EFD_MASK is set, eliminating
  the possibility of triggering the orginal BUG_ON() macros which have now
  been removed.

Thank you,
Damian

Martin Sustrik (1):
  eventfd: implementation of EFD_MASK flag

 fs/eventfd.c                 | 91 ++++++++++++++++++++++++++++++++++++++------
 include/linux/eventfd.h      | 16 +-------
 include/uapi/linux/eventfd.h | 40 +++++++++++++++++++
 3 files changed, 121 insertions(+), 26 deletions(-)
 create mode 100644 include/uapi/linux/eventfd.h

-- 
1.9.1

[PATCH v3 1/1] eventfd: implementation of EFD_MASK flag

[Damian Hobson-Garcia]

From: Martin Sustrik <sustrik@250bpm.com>

When implementing network protocols in user space, one has to implement
fake file descriptors to represent the sockets for the protocol.

Polling on such fake file descriptors is a problem (poll/select/epoll
accept only true file descriptors) and forces protocol implementers to use
various workarounds resulting in complex, non-standard and convoluted APIs.

More generally, ability to create full-blown file descriptors for
userspace-to-userspace signalling is missing. While eventfd(2) goes half
the way towards this goal it has follwoing shorcomings:

I.  There's no way to signal POLLPRI, POLLHUP etc.
II. There's no way to signal arbitrary combination of POLL* flags. Most
    notably, simultaneous !POLLIN and !POLLOUT, which is a perfectly valid
    combination for a network protocol (rx buffer is empty and tx buffer is
    full), cannot be signaled using eventfd.

This patch implements new EFD_MASK flag which solves the above problems.

The semantics of EFD_MASK are as follows:

eventfd(2):

If eventfd is created with EFD_MASK flag set, it is initialised in such a
way as to signal no events on the file descriptor when it is polled on.
The 'initval' argument is ignored.

write(2):

User is allowed to write only buffers containing a 32-bit value
representing any combination of event flags as defined by the poll(2)
function (POLLIN, POLLOUT, POLLERR, POLLHUP etc.). Specified events
will be signaled when polling (select, poll, epoll) on the eventfd is
done later on.

read(2):

read is not supported and will fail with EINVAL.

select(2), poll(2) and similar:

When polling on the eventfd marked by EFD_MASK flag, all the events
specified in last written event flags shall be signaled.

Signed-off-by: Martin Sustrik <sustrik@250bpm.com>

[dhobsong@igel.co.jp: Rebased, and resubmitted for Linux 4.3]
Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
---
 fs/eventfd.c                 | 102 ++++++++++++++++++++++++++++++++++++++-----
 include/linux/eventfd.h      |  16 +------
 include/uapi/linux/eventfd.h |  33 ++++++++++++++
 3 files changed, 126 insertions(+), 25 deletions(-)
 create mode 100644 include/uapi/linux/eventfd.h

diff --git a/fs/eventfd.c b/fs/eventfd.c
index 8d0c0df..1310779 100644
--- a/fs/eventfd.c
+++ b/fs/eventfd.c
@@ -2,6 +2,7 @@
  *  fs/eventfd.c
  *
  *  Copyright (C) 2007  Davide Libenzi <davidel@xmailserver.org>
+ *  Copyright (C) 2013  Martin Sustrik <sustrik@250bpm.com>
  *
  */
 
@@ -22,18 +23,31 @@
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 
+#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
+#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE | EFD_MASK)
+#define EFD_MASK_VALID_EVENTS (POLLIN | POLLPRI | POLLOUT | POLLERR | POLLHUP)
+
 struct eventfd_ctx {
 	struct kref kref;
 	wait_queue_head_t wqh;
-	/*
-	 * Every time that a write(2) is performed on an eventfd, the
-	 * value of the __u64 being written is added to "count" and a
-	 * wakeup is performed on "wqh". A read(2) will return the "count"
-	 * value to userspace, and will reset "count" to zero. The kernel
-	 * side eventfd_signal() also, adds to the "count" counter and
-	 * issue a wakeup.
-	 */
-	__u64 count;
+	union {
+		/*
+		 * Every time that a write(2) is performed on an eventfd, the
+		 * value of the __u64 being written is added to "count" and a
+		 * wakeup is performed on "wqh". A read(2) will return the
+		 * "count" value to userspace, and will reset "count" to zero.
+		 * The kernel side eventfd_signal() also, adds to the "count"
+		 * counter and issue a wakeup.
+		 */
+		__u64 count;
+
+		/*
+		 * When using eventfd in EFD_MASK mode this stracture stores the
+		 * current events to be signaled on the eventfd (events member)
+		 * along with opaque user-defined data (data member).
+		 */
+		__u32 events;
+	};
 	unsigned int flags;
 };
 
@@ -134,6 +148,14 @@ static unsigned int eventfd_poll(struct file *file, poll_table *wait)
 	return events;
 }
 
+static unsigned int eventfd_mask_poll(struct file *file, poll_table *wait)
+{
+	struct eventfd_ctx *ctx = file->private_data;
+
+	poll_wait(file, &ctx->wqh, wait);
+	return ctx->events;
+}
+
 static void eventfd_ctx_do_read(struct eventfd_ctx *ctx, __u64 *cnt)
 {
 	*cnt = (ctx->flags & EFD_SEMAPHORE) ? 1 : ctx->count;
@@ -239,6 +261,14 @@ static ssize_t eventfd_read(struct file *file, char __user *buf, size_t count,
 	return put_user(cnt, (__u64 __user *) buf) ? -EFAULT : sizeof(cnt);
 }
 
+static ssize_t eventfd_mask_read(struct file *file, char __user *buf,
+			    size_t count,
+			    loff_t *ppos)
+{
+	return -EINVAL;
+}
+
+
 static ssize_t eventfd_write(struct file *file, const char __user *buf, size_t count,
 			     loff_t *ppos)
 {
@@ -286,6 +316,28 @@ static ssize_t eventfd_write(struct file *file, const char __user *buf, size_t c
 	return res;
 }
 
+static ssize_t eventfd_mask_write(struct file *file, const char __user *buf,
+			     size_t count,
+			     loff_t *ppos)
+{
+	struct eventfd_ctx *ctx = file->private_data;
+	__u32 events;
+
+	if (count < sizeof(events))
+		return -EINVAL;
+	if (copy_from_user(&events, buf, sizeof(events)))
+		return -EFAULT;
+	if (events & ~EFD_MASK_VALID_EVENTS)
+		return -EINVAL;
+	spin_lock_irq(&ctx->wqh.lock);
+	memcpy(&ctx->events, &events, sizeof(ctx->events));
+	if (waitqueue_active(&ctx->wqh))
+		wake_up_locked_poll(&ctx->wqh,
+			(unsigned long)ctx->events);
+	spin_unlock_irq(&ctx->wqh.lock);
+	return sizeof(ctx->events);
+}
+
 #ifdef CONFIG_PROC_FS
 static void eventfd_show_fdinfo(struct seq_file *m, struct file *f)
 {
@@ -296,6 +348,16 @@ static void eventfd_show_fdinfo(struct seq_file *m, struct file *f)
 		   (unsigned long long)ctx->count);
 	spin_unlock_irq(&ctx->wqh.lock);
 }
+
+static void eventfd_mask_show_fdinfo(struct seq_file *m, struct file *f)
+{
+	struct eventfd_ctx *ctx = f->private_data;
+
+	spin_lock_irq(&ctx->wqh.lock);
+	seq_printf(m, "eventfd-mask: %x\n",
+		ctx->events);
+	spin_unlock_irq(&ctx->wqh.lock);
+}
 #endif
 
 static const struct file_operations eventfd_fops = {
@@ -309,6 +371,17 @@ static const struct file_operations eventfd_fops = {
 	.llseek		= noop_llseek,
 };
 
+static const struct file_operations eventfd_mask_fops = {
+#ifdef CONFIG_PROC_FS
+	.show_fdinfo	= eventfd_mask_show_fdinfo,
+#endif
+	.release	= eventfd_release,
+	.poll		= eventfd_mask_poll,
+	.read		= eventfd_mask_read,
+	.write		= eventfd_mask_write,
+	.llseek		= noop_llseek,
+};
+
 /**
  * eventfd_fget - Acquire a reference of an eventfd file descriptor.
  * @fd: [in] Eventfd file descriptor.
@@ -392,6 +465,7 @@ struct file *eventfd_file_create(unsigned int count, int flags)
 {
 	struct file *file;
 	struct eventfd_ctx *ctx;
+	const struct file_operations *fops;
 
 	/* Check the EFD_* constants for consistency.  */
 	BUILD_BUG_ON(EFD_CLOEXEC != O_CLOEXEC);
@@ -406,10 +480,16 @@ struct file *eventfd_file_create(unsigned int count, int flags)
 
 	kref_init(&ctx->kref);
 	init_waitqueue_head(&ctx->wqh);
-	ctx->count = count;
+	if (flags & EFD_MASK) {
+		ctx->events = 0;
+		fops = &eventfd_mask_fops;
+	} else {
+		ctx->count = count;
+		fops = &eventfd_fops;
+	}
 	ctx->flags = flags;
 
-	file = anon_inode_getfile("[eventfd]", &eventfd_fops, ctx,
+	file = anon_inode_getfile("[eventfd]", fops, ctx,
 				  O_RDWR | (flags & EFD_SHARED_FCNTL_FLAGS));
 	if (IS_ERR(file))
 		eventfd_free_ctx(ctx);
diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h
index ff0b981..87de343 100644
--- a/include/linux/eventfd.h
+++ b/include/linux/eventfd.h
@@ -8,23 +8,11 @@
 #ifndef _LINUX_EVENTFD_H
 #define _LINUX_EVENTFD_H
 
+#include <uapi/linux/eventfd.h>
+
 #include <linux/fcntl.h>
 #include <linux/wait.h>
 
-/*
- * CAREFUL: Check include/uapi/asm-generic/fcntl.h when defining
- * new flags, since they might collide with O_* ones. We want
- * to re-use O_* flags that couldn't possibly have a meaning
- * from eventfd, in order to leave a free define-space for
- * shared O_* flags.
- */
-#define EFD_SEMAPHORE (1 << 0)
-#define EFD_CLOEXEC O_CLOEXEC
-#define EFD_NONBLOCK O_NONBLOCK
-
-#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
-#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
-
 struct file;
 
 #ifdef CONFIG_EVENTFD
diff --git a/include/uapi/linux/eventfd.h b/include/uapi/linux/eventfd.h
new file mode 100644
index 0000000..097dcad
--- /dev/null
+++ b/include/uapi/linux/eventfd.h
@@ -0,0 +1,33 @@
+/*
+ *  Copyright (C) 2013 Martin Sustrik <sustrik@250bpm.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ */
+
+#ifndef _UAPI_LINUX_EVENTFD_H
+#define _UAPI_LINUX_EVENTFD_H
+
+/* For O_CLOEXEC */
+#include <linux/fcntl.h>
+#include <linux/types.h>
+
+/*
+ * CAREFUL: Check include/asm-generic/fcntl.h when defining
+ * new flags, since they might collide with O_* ones. We want
+ * to re-use O_* flags that couldn't possibly have a meaning
+ * from eventfd, in order to leave a free define-space for
+ * shared O_* flags.
+ */
+
+/* Provide semaphore-like semantics for reads from the eventfd. */
+#define EFD_SEMAPHORE (1 << 0)
+/* Provide event mask semantics for the eventfd. */
+#define EFD_MASK (1 << 1)
+/*  Set the close-on-exec (FD_CLOEXEC) flag on the eventfd. */
+#define EFD_CLOEXEC O_CLOEXEC
+/*  Create the eventfd in non-blocking mode. */
+#define EFD_NONBLOCK O_NONBLOCK
+#endif /* _UAPI_LINUX_EVENTFD_H */
-- 
1.9.1

Re: [PATCH v3 1/1] eventfd: implementation of EFD_MASK flag

[Damian Hobson-Garcia]

Hello,

> diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h
> index ff0b981..87de343 100644
> --- a/include/linux/eventfd.h
> +++ b/include/linux/eventfd.h
<snip>
>  
> -/*
> - * CAREFUL: Check include/uapi/asm-generic/fcntl.h when defining
> - * new flags, since they might collide with O_* ones. We want
> - * to re-use O_* flags that couldn't possibly have a meaning
> - * from eventfd, in order to leave a free define-space for
> - * shared O_* flags.
> - */
> -#define EFD_SEMAPHORE (1 << 0)
> -#define EFD_CLOEXEC O_CLOEXEC
> -#define EFD_NONBLOCK O_NONBLOCK
> -
> -#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
> -#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
> -
>  struct file;
>  
>  #ifdef CONFIG_EVENTFD
> diff --git a/include/uapi/linux/eventfd.h b/include/uapi/linux/eventfd.h
> new file mode 100644
> index 0000000..097dcad
> --- /dev/null
> +++ b/include/uapi/linux/eventfd.h
> @@ -0,0 +1,33 @@
<snip>
> +
> +/*
> + * CAREFUL: Check include/asm-generic/fcntl.h when defining
> + * new flags, since they might collide with O_* ones. We want
> + * to re-use O_* flags that couldn't possibly have a meaning
> + * from eventfd, in order to leave a free define-space for
> + * shared O_* flags.
> + */
> +
> +/* Provide semaphore-like semantics for reads from the eventfd. */
> +#define EFD_SEMAPHORE (1 << 0)
> +/* Provide event mask semantics for the eventfd. */
> +#define EFD_MASK (1 << 1)
> +/*  Set the close-on-exec (FD_CLOEXEC) flag on the eventfd. */
> +#define EFD_CLOEXEC O_CLOEXEC
> +/*  Create the eventfd in non-blocking mode. */
> +#define EFD_NONBLOCK O_NONBLOCK
> +#endif /* _UAPI_LINUX_EVENTFD_H */
> 

Since the latest version of this patch adds only the EFD_MASK definition
to the eventfd header, I was wondering if it was really
necessary/recommended to move the definitions from linux/eventfd.h to
linux/uapi/eventfd.h.  From my understanding, the EFD_SEMAPHORE (and now
EFD_MASK) define(s) are provided to user space from the libc headers
only. Any advice would be greatly appreciated.

Thank you,
Damian

Re: [PATCH v3 1/1] eventfd: implementation of EFD_MASK flag

[Paul Elder]

Hello Damian, Martin, and all,

I came across this (quite old by now) patch to extend eventfd's polling
functionality. I was wondering what happened to it (why it hasn't been
merged yet) and if we could, or what is needed to, move it forward.

I was thinking to use it for V4L2 events support via polling in the V4L2
compatibility layer for libcamera [1]. We can signal V4L2 buffer
availability POLLOUT via write(), but there is no way to signal V4L2
events, as they are signaled via POLLPRI.


Thank you,

Paul

[1] https://libcamera.org/docs.html#id1

On Thu, Oct 15, 2015 at 10:42:08AM +0900, Damian Hobson-Garcia wrote:
> From: Martin Sustrik <sustrik@250bpm.com>
> 
> When implementing network protocols in user space, one has to implement
> fake file descriptors to represent the sockets for the protocol.
> 
> Polling on such fake file descriptors is a problem (poll/select/epoll
> accept only true file descriptors) and forces protocol implementers to use
> various workarounds resulting in complex, non-standard and convoluted APIs.
> 
> More generally, ability to create full-blown file descriptors for
> userspace-to-userspace signalling is missing. While eventfd(2) goes half
> the way towards this goal it has follwoing shorcomings:
> 
> I.  There's no way to signal POLLPRI, POLLHUP etc.
> II. There's no way to signal arbitrary combination of POLL* flags. Most
>     notably, simultaneous !POLLIN and !POLLOUT, which is a perfectly valid
>     combination for a network protocol (rx buffer is empty and tx buffer is
>     full), cannot be signaled using eventfd.
> 
> This patch implements new EFD_MASK flag which solves the above problems.
> 
> The semantics of EFD_MASK are as follows:
> 
> eventfd(2):
> 
> If eventfd is created with EFD_MASK flag set, it is initialised in such a
> way as to signal no events on the file descriptor when it is polled on.
> The 'initval' argument is ignored.
> 
> write(2):
> 
> User is allowed to write only buffers containing a 32-bit value
> representing any combination of event flags as defined by the poll(2)
> function (POLLIN, POLLOUT, POLLERR, POLLHUP etc.). Specified events
> will be signaled when polling (select, poll, epoll) on the eventfd is
> done later on.
> 
> read(2):
> 
> read is not supported and will fail with EINVAL.
> 
> select(2), poll(2) and similar:
> 
> When polling on the eventfd marked by EFD_MASK flag, all the events
> specified in last written event flags shall be signaled.
> 
> Signed-off-by: Martin Sustrik <sustrik@250bpm.com>
> 
> [dhobsong@igel.co.jp: Rebased, and resubmitted for Linux 4.3]
> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> ---
>  fs/eventfd.c                 | 102 ++++++++++++++++++++++++++++++++++++++-----
>  include/linux/eventfd.h      |  16 +------
>  include/uapi/linux/eventfd.h |  33 ++++++++++++++
>  3 files changed, 126 insertions(+), 25 deletions(-)
>  create mode 100644 include/uapi/linux/eventfd.h
> 
> diff --git a/fs/eventfd.c b/fs/eventfd.c
> index 8d0c0df..1310779 100644
> --- a/fs/eventfd.c
> +++ b/fs/eventfd.c
> @@ -2,6 +2,7 @@
>   *  fs/eventfd.c
>   *
>   *  Copyright (C) 2007  Davide Libenzi <davidel@xmailserver.org>
> + *  Copyright (C) 2013  Martin Sustrik <sustrik@250bpm.com>
>   *
>   */
>  
> @@ -22,18 +23,31 @@
>  #include <linux/proc_fs.h>
>  #include <linux/seq_file.h>
>  
> +#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
> +#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE | EFD_MASK)
> +#define EFD_MASK_VALID_EVENTS (POLLIN | POLLPRI | POLLOUT | POLLERR | POLLHUP)
> +
>  struct eventfd_ctx {
>  	struct kref kref;
>  	wait_queue_head_t wqh;
> -	/*
> -	 * Every time that a write(2) is performed on an eventfd, the
> -	 * value of the __u64 being written is added to "count" and a
> -	 * wakeup is performed on "wqh". A read(2) will return the "count"
> -	 * value to userspace, and will reset "count" to zero. The kernel
> -	 * side eventfd_signal() also, adds to the "count" counter and
> -	 * issue a wakeup.
> -	 */
> -	__u64 count;
> +	union {
> +		/*
> +		 * Every time that a write(2) is performed on an eventfd, the
> +		 * value of the __u64 being written is added to "count" and a
> +		 * wakeup is performed on "wqh". A read(2) will return the
> +		 * "count" value to userspace, and will reset "count" to zero.
> +		 * The kernel side eventfd_signal() also, adds to the "count"
> +		 * counter and issue a wakeup.
> +		 */
> +		__u64 count;
> +
> +		/*
> +		 * When using eventfd in EFD_MASK mode this stracture stores the
> +		 * current events to be signaled on the eventfd (events member)
> +		 * along with opaque user-defined data (data member).
> +		 */
> +		__u32 events;
> +	};
>  	unsigned int flags;
>  };
>  
> @@ -134,6 +148,14 @@ static unsigned int eventfd_poll(struct file *file, poll_table *wait)
>  	return events;
>  }
>  
> +static unsigned int eventfd_mask_poll(struct file *file, poll_table *wait)
> +{
> +	struct eventfd_ctx *ctx = file->private_data;
> +
> +	poll_wait(file, &ctx->wqh, wait);
> +	return ctx->events;
> +}
> +
>  static void eventfd_ctx_do_read(struct eventfd_ctx *ctx, __u64 *cnt)
>  {
>  	*cnt = (ctx->flags & EFD_SEMAPHORE) ? 1 : ctx->count;
> @@ -239,6 +261,14 @@ static ssize_t eventfd_read(struct file *file, char __user *buf, size_t count,
>  	return put_user(cnt, (__u64 __user *) buf) ? -EFAULT : sizeof(cnt);
>  }
>  
> +static ssize_t eventfd_mask_read(struct file *file, char __user *buf,
> +			    size_t count,
> +			    loff_t *ppos)
> +{
> +	return -EINVAL;
> +}
> +
> +
>  static ssize_t eventfd_write(struct file *file, const char __user *buf, size_t count,
>  			     loff_t *ppos)
>  {
> @@ -286,6 +316,28 @@ static ssize_t eventfd_write(struct file *file, const char __user *buf, size_t c
>  	return res;
>  }
>  
> +static ssize_t eventfd_mask_write(struct file *file, const char __user *buf,
> +			     size_t count,
> +			     loff_t *ppos)
> +{
> +	struct eventfd_ctx *ctx = file->private_data;
> +	__u32 events;
> +
> +	if (count < sizeof(events))
> +		return -EINVAL;
> +	if (copy_from_user(&events, buf, sizeof(events)))
> +		return -EFAULT;
> +	if (events & ~EFD_MASK_VALID_EVENTS)
> +		return -EINVAL;
> +	spin_lock_irq(&ctx->wqh.lock);
> +	memcpy(&ctx->events, &events, sizeof(ctx->events));
> +	if (waitqueue_active(&ctx->wqh))
> +		wake_up_locked_poll(&ctx->wqh,
> +			(unsigned long)ctx->events);
> +	spin_unlock_irq(&ctx->wqh.lock);
> +	return sizeof(ctx->events);
> +}
> +
>  #ifdef CONFIG_PROC_FS
>  static void eventfd_show_fdinfo(struct seq_file *m, struct file *f)
>  {
> @@ -296,6 +348,16 @@ static void eventfd_show_fdinfo(struct seq_file *m, struct file *f)
>  		   (unsigned long long)ctx->count);
>  	spin_unlock_irq(&ctx->wqh.lock);
>  }
> +
> +static void eventfd_mask_show_fdinfo(struct seq_file *m, struct file *f)
> +{
> +	struct eventfd_ctx *ctx = f->private_data;
> +
> +	spin_lock_irq(&ctx->wqh.lock);
> +	seq_printf(m, "eventfd-mask: %x\n",
> +		ctx->events);
> +	spin_unlock_irq(&ctx->wqh.lock);
> +}
>  #endif
>  
>  static const struct file_operations eventfd_fops = {
> @@ -309,6 +371,17 @@ static const struct file_operations eventfd_fops = {
>  	.llseek		= noop_llseek,
>  };
>  
> +static const struct file_operations eventfd_mask_fops = {
> +#ifdef CONFIG_PROC_FS
> +	.show_fdinfo	= eventfd_mask_show_fdinfo,
> +#endif
> +	.release	= eventfd_release,
> +	.poll		= eventfd_mask_poll,
> +	.read		= eventfd_mask_read,
> +	.write		= eventfd_mask_write,
> +	.llseek		= noop_llseek,
> +};
> +
>  /**
>   * eventfd_fget - Acquire a reference of an eventfd file descriptor.
>   * @fd: [in] Eventfd file descriptor.
> @@ -392,6 +465,7 @@ struct file *eventfd_file_create(unsigned int count, int flags)
>  {
>  	struct file *file;
>  	struct eventfd_ctx *ctx;
> +	const struct file_operations *fops;
>  
>  	/* Check the EFD_* constants for consistency.  */
>  	BUILD_BUG_ON(EFD_CLOEXEC != O_CLOEXEC);
> @@ -406,10 +480,16 @@ struct file *eventfd_file_create(unsigned int count, int flags)
>  
>  	kref_init(&ctx->kref);
>  	init_waitqueue_head(&ctx->wqh);
> -	ctx->count = count;
> +	if (flags & EFD_MASK) {
> +		ctx->events = 0;
> +		fops = &eventfd_mask_fops;
> +	} else {
> +		ctx->count = count;
> +		fops = &eventfd_fops;
> +	}
>  	ctx->flags = flags;
>  
> -	file = anon_inode_getfile("[eventfd]", &eventfd_fops, ctx,
> +	file = anon_inode_getfile("[eventfd]", fops, ctx,
>  				  O_RDWR | (flags & EFD_SHARED_FCNTL_FLAGS));
>  	if (IS_ERR(file))
>  		eventfd_free_ctx(ctx);
> diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h
> index ff0b981..87de343 100644
> --- a/include/linux/eventfd.h
> +++ b/include/linux/eventfd.h
> @@ -8,23 +8,11 @@
>  #ifndef _LINUX_EVENTFD_H
>  #define _LINUX_EVENTFD_H
>  
> +#include <uapi/linux/eventfd.h>
> +
>  #include <linux/fcntl.h>
>  #include <linux/wait.h>
>  
> -/*
> - * CAREFUL: Check include/uapi/asm-generic/fcntl.h when defining
> - * new flags, since they might collide with O_* ones. We want
> - * to re-use O_* flags that couldn't possibly have a meaning
> - * from eventfd, in order to leave a free define-space for
> - * shared O_* flags.
> - */
> -#define EFD_SEMAPHORE (1 << 0)
> -#define EFD_CLOEXEC O_CLOEXEC
> -#define EFD_NONBLOCK O_NONBLOCK
> -
> -#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
> -#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
> -
>  struct file;
>  
>  #ifdef CONFIG_EVENTFD
> diff --git a/include/uapi/linux/eventfd.h b/include/uapi/linux/eventfd.h
> new file mode 100644
> index 0000000..097dcad
> --- /dev/null
> +++ b/include/uapi/linux/eventfd.h
> @@ -0,0 +1,33 @@
> +/*
> + *  Copyright (C) 2013 Martin Sustrik <sustrik@250bpm.com>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + */
> +
> +#ifndef _UAPI_LINUX_EVENTFD_H
> +#define _UAPI_LINUX_EVENTFD_H
> +
> +/* For O_CLOEXEC */
> +#include <linux/fcntl.h>
> +#include <linux/types.h>
> +
> +/*
> + * CAREFUL: Check include/asm-generic/fcntl.h when defining
> + * new flags, since they might collide with O_* ones. We want
> + * to re-use O_* flags that couldn't possibly have a meaning
> + * from eventfd, in order to leave a free define-space for
> + * shared O_* flags.
> + */
> +
> +/* Provide semaphore-like semantics for reads from the eventfd. */
> +#define EFD_SEMAPHORE (1 << 0)
> +/* Provide event mask semantics for the eventfd. */
> +#define EFD_MASK (1 << 1)
> +/*  Set the close-on-exec (FD_CLOEXEC) flag on the eventfd. */
> +#define EFD_CLOEXEC O_CLOEXEC
> +/*  Create the eventfd in non-blocking mode. */
> +#define EFD_NONBLOCK O_NONBLOCK
> +#endif /* _UAPI_LINUX_EVENTFD_H */
> -- 
> 1.9.1

Re: [PATCH v3 1/1] eventfd: implementation of EFD_MASK flag

[Damian Hobson-Garcia]

Hi Paul,

On 2020-06-19 7:16 p.m., Paul Elder wrote:
> Hello Damian, Martin, and all,
> 
> I came across this (quite old by now) patch to extend eventfd's polling
> functionality. I was wondering what happened to it (why it hasn't been
> merged yet) and if we could, or what is needed to, move it forward.

I think there was an open question about whether it was
best to move the definitions of EFD_SEMAPHORE, etc out of
/include/linux/eventfd.h and into a newly created
/include/uapi/linux/eventfd.h as this patch does.

I don't know if the maintainers have any concerns on this matter, or the
patch in general, that would prevent this from moving forward.

Thank you,
Damian

> 
> I was thinking to use it for V4L2 events support via polling in the V4L2
> compatibility layer for libcamera [1]. We can signal V4L2 buffer
> availability POLLOUT via write(), but there is no way to signal V4L2
> events, as they are signaled via POLLPRI.
> 
> 
> Thank you,
> 
> Paul
> 
> [1] https://libcamera.org/docs.html#id1
> 
> On Thu, Oct 15, 2015 at 10:42:08AM +0900, Damian Hobson-Garcia wrote:
>> From: Martin Sustrik <sustrik@250bpm.com>
>>
>> When implementing network protocols in user space, one has to implement
>> fake file descriptors to represent the sockets for the protocol.
>>
>> Polling on such fake file descriptors is a problem (poll/select/epoll
>> accept only true file descriptors) and forces protocol implementers to use
>> various workarounds resulting in complex, non-standard and convoluted APIs.
>>
>> More generally, ability to create full-blown file descriptors for
>> userspace-to-userspace signalling is missing. While eventfd(2) goes half
>> the way towards this goal it has follwoing shorcomings:
>>
>> I.  There's no way to signal POLLPRI, POLLHUP etc.
>> II. There's no way to signal arbitrary combination of POLL* flags. Most
>>     notably, simultaneous !POLLIN and !POLLOUT, which is a perfectly valid
>>     combination for a network protocol (rx buffer is empty and tx buffer is
>>     full), cannot be signaled using eventfd.
>>
>> This patch implements new EFD_MASK flag which solves the above problems.
>>
>> The semantics of EFD_MASK are as follows:
>>
>> eventfd(2):
>>
>> If eventfd is created with EFD_MASK flag set, it is initialised in such a
>> way as to signal no events on the file descriptor when it is polled on.
>> The 'initval' argument is ignored.
>>
>> write(2):
>>
>> User is allowed to write only buffers containing a 32-bit value
>> representing any combination of event flags as defined by the poll(2)
>> function (POLLIN, POLLOUT, POLLERR, POLLHUP etc.). Specified events
>> will be signaled when polling (select, poll, epoll) on the eventfd is
>> done later on.
>>
>> read(2):
>>
>> read is not supported and will fail with EINVAL.
>>
>> select(2), poll(2) and similar:
>>
>> When polling on the eventfd marked by EFD_MASK flag, all the events
>> specified in last written event flags shall be signaled.
>>
>> Signed-off-by: Martin Sustrik <sustrik@250bpm.com>
>>
>> [dhobsong@igel.co.jp: Rebased, and resubmitted for Linux 4.3]
>> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
>> ---
>>  fs/eventfd.c                 | 102 ++++++++++++++++++++++++++++++++++++++-----
>>  include/linux/eventfd.h      |  16 +------
>>  include/uapi/linux/eventfd.h |  33 ++++++++++++++
>>  3 files changed, 126 insertions(+), 25 deletions(-)
>>  create mode 100644 include/uapi/linux/eventfd.h
>>
>> diff --git a/fs/eventfd.c b/fs/eventfd.c
>> index 8d0c0df..1310779 100644
>> --- a/fs/eventfd.c
>> +++ b/fs/eventfd.c
>> @@ -2,6 +2,7 @@
>>   *  fs/eventfd.c
>>   *
>>   *  Copyright (C) 2007  Davide Libenzi <davidel@xmailserver.org>
>> + *  Copyright (C) 2013  Martin Sustrik <sustrik@250bpm.com>
>>   *
>>   */
>>  
>> @@ -22,18 +23,31 @@
>>  #include <linux/proc_fs.h>
>>  #include <linux/seq_file.h>
>>  
>> +#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
>> +#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE | EFD_MASK)
>> +#define EFD_MASK_VALID_EVENTS (POLLIN | POLLPRI | POLLOUT | POLLERR | POLLHUP)
>> +
>>  struct eventfd_ctx {
>>  	struct kref kref;
>>  	wait_queue_head_t wqh;
>> -	/*
>> -	 * Every time that a write(2) is performed on an eventfd, the
>> -	 * value of the __u64 being written is added to "count" and a
>> -	 * wakeup is performed on "wqh". A read(2) will return the "count"
>> -	 * value to userspace, and will reset "count" to zero. The kernel
>> -	 * side eventfd_signal() also, adds to the "count" counter and
>> -	 * issue a wakeup.
>> -	 */
>> -	__u64 count;
>> +	union {
>> +		/*
>> +		 * Every time that a write(2) is performed on an eventfd, the
>> +		 * value of the __u64 being written is added to "count" and a
>> +		 * wakeup is performed on "wqh". A read(2) will return the
>> +		 * "count" value to userspace, and will reset "count" to zero.
>> +		 * The kernel side eventfd_signal() also, adds to the "count"
>> +		 * counter and issue a wakeup.
>> +		 */
>> +		__u64 count;
>> +
>> +		/*
>> +		 * When using eventfd in EFD_MASK mode this stracture stores the
>> +		 * current events to be signaled on the eventfd (events member)
>> +		 * along with opaque user-defined data (data member).
>> +		 */
>> +		__u32 events;
>> +	};
>>  	unsigned int flags;
>>  };
>>  
>> @@ -134,6 +148,14 @@ static unsigned int eventfd_poll(struct file *file, poll_table *wait)
>>  	return events;
>>  }
>>  
>> +static unsigned int eventfd_mask_poll(struct file *file, poll_table *wait)
>> +{
>> +	struct eventfd_ctx *ctx = file->private_data;
>> +
>> +	poll_wait(file, &ctx->wqh, wait);
>> +	return ctx->events;
>> +}
>> +
>>  static void eventfd_ctx_do_read(struct eventfd_ctx *ctx, __u64 *cnt)
>>  {
>>  	*cnt = (ctx->flags & EFD_SEMAPHORE) ? 1 : ctx->count;
>> @@ -239,6 +261,14 @@ static ssize_t eventfd_read(struct file *file, char __user *buf, size_t count,
>>  	return put_user(cnt, (__u64 __user *) buf) ? -EFAULT : sizeof(cnt);
>>  }
>>  
>> +static ssize_t eventfd_mask_read(struct file *file, char __user *buf,
>> +			    size_t count,
>> +			    loff_t *ppos)
>> +{
>> +	return -EINVAL;
>> +}
>> +
>> +
>>  static ssize_t eventfd_write(struct file *file, const char __user *buf, size_t count,
>>  			     loff_t *ppos)
>>  {
>> @@ -286,6 +316,28 @@ static ssize_t eventfd_write(struct file *file, const char __user *buf, size_t c
>>  	return res;
>>  }
>>  
>> +static ssize_t eventfd_mask_write(struct file *file, const char __user *buf,
>> +			     size_t count,
>> +			     loff_t *ppos)
>> +{
>> +	struct eventfd_ctx *ctx = file->private_data;
>> +	__u32 events;
>> +
>> +	if (count < sizeof(events))
>> +		return -EINVAL;
>> +	if (copy_from_user(&events, buf, sizeof(events)))
>> +		return -EFAULT;
>> +	if (events & ~EFD_MASK_VALID_EVENTS)
>> +		return -EINVAL;
>> +	spin_lock_irq(&ctx->wqh.lock);
>> +	memcpy(&ctx->events, &events, sizeof(ctx->events));
>> +	if (waitqueue_active(&ctx->wqh))
>> +		wake_up_locked_poll(&ctx->wqh,
>> +			(unsigned long)ctx->events);
>> +	spin_unlock_irq(&ctx->wqh.lock);
>> +	return sizeof(ctx->events);
>> +}
>> +
>>  #ifdef CONFIG_PROC_FS
>>  static void eventfd_show_fdinfo(struct seq_file *m, struct file *f)
>>  {
>> @@ -296,6 +348,16 @@ static void eventfd_show_fdinfo(struct seq_file *m, struct file *f)
>>  		   (unsigned long long)ctx->count);
>>  	spin_unlock_irq(&ctx->wqh.lock);
>>  }
>> +
>> +static void eventfd_mask_show_fdinfo(struct seq_file *m, struct file *f)
>> +{
>> +	struct eventfd_ctx *ctx = f->private_data;
>> +
>> +	spin_lock_irq(&ctx->wqh.lock);
>> +	seq_printf(m, "eventfd-mask: %x\n",
>> +		ctx->events);
>> +	spin_unlock_irq(&ctx->wqh.lock);
>> +}
>>  #endif
>>  
>>  static const struct file_operations eventfd_fops = {
>> @@ -309,6 +371,17 @@ static const struct file_operations eventfd_fops = {
>>  	.llseek		= noop_llseek,
>>  };
>>  
>> +static const struct file_operations eventfd_mask_fops = {
>> +#ifdef CONFIG_PROC_FS
>> +	.show_fdinfo	= eventfd_mask_show_fdinfo,
>> +#endif
>> +	.release	= eventfd_release,
>> +	.poll		= eventfd_mask_poll,
>> +	.read		= eventfd_mask_read,
>> +	.write		= eventfd_mask_write,
>> +	.llseek		= noop_llseek,
>> +};
>> +
>>  /**
>>   * eventfd_fget - Acquire a reference of an eventfd file descriptor.
>>   * @fd: [in] Eventfd file descriptor.
>> @@ -392,6 +465,7 @@ struct file *eventfd_file_create(unsigned int count, int flags)
>>  {
>>  	struct file *file;
>>  	struct eventfd_ctx *ctx;
>> +	const struct file_operations *fops;
>>  
>>  	/* Check the EFD_* constants for consistency.  */
>>  	BUILD_BUG_ON(EFD_CLOEXEC != O_CLOEXEC);
>> @@ -406,10 +480,16 @@ struct file *eventfd_file_create(unsigned int count, int flags)
>>  
>>  	kref_init(&ctx->kref);
>>  	init_waitqueue_head(&ctx->wqh);
>> -	ctx->count = count;
>> +	if (flags & EFD_MASK) {
>> +		ctx->events = 0;
>> +		fops = &eventfd_mask_fops;
>> +	} else {
>> +		ctx->count = count;
>> +		fops = &eventfd_fops;
>> +	}
>>  	ctx->flags = flags;
>>  
>> -	file = anon_inode_getfile("[eventfd]", &eventfd_fops, ctx,
>> +	file = anon_inode_getfile("[eventfd]", fops, ctx,
>>  				  O_RDWR | (flags & EFD_SHARED_FCNTL_FLAGS));
>>  	if (IS_ERR(file))
>>  		eventfd_free_ctx(ctx);
>> diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h
>> index ff0b981..87de343 100644
>> --- a/include/linux/eventfd.h
>> +++ b/include/linux/eventfd.h
>> @@ -8,23 +8,11 @@
>>  #ifndef _LINUX_EVENTFD_H
>>  #define _LINUX_EVENTFD_H
>>  
>> +#include <uapi/linux/eventfd.h>
>> +
>>  #include <linux/fcntl.h>
>>  #include <linux/wait.h>
>>  
>> -/*
>> - * CAREFUL: Check include/uapi/asm-generic/fcntl.h when defining
>> - * new flags, since they might collide with O_* ones. We want
>> - * to re-use O_* flags that couldn't possibly have a meaning
>> - * from eventfd, in order to leave a free define-space for
>> - * shared O_* flags.
>> - */
>> -#define EFD_SEMAPHORE (1 << 0)
>> -#define EFD_CLOEXEC O_CLOEXEC
>> -#define EFD_NONBLOCK O_NONBLOCK
>> -
>> -#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
>> -#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
>> -
>>  struct file;
>>  
>>  #ifdef CONFIG_EVENTFD
>> diff --git a/include/uapi/linux/eventfd.h b/include/uapi/linux/eventfd.h
>> new file mode 100644
>> index 0000000..097dcad
>> --- /dev/null
>> +++ b/include/uapi/linux/eventfd.h
>> @@ -0,0 +1,33 @@
>> +/*
>> + *  Copyright (C) 2013 Martin Sustrik <sustrik@250bpm.com>
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation; either version 2 of the License, or
>> + *  (at your option) any later version.
>> + */
>> +
>> +#ifndef _UAPI_LINUX_EVENTFD_H
>> +#define _UAPI_LINUX_EVENTFD_H
>> +
>> +/* For O_CLOEXEC */
>> +#include <linux/fcntl.h>
>> +#include <linux/types.h>
>> +
>> +/*
>> + * CAREFUL: Check include/asm-generic/fcntl.h when defining
>> + * new flags, since they might collide with O_* ones. We want
>> + * to re-use O_* flags that couldn't possibly have a meaning
>> + * from eventfd, in order to leave a free define-space for
>> + * shared O_* flags.
>> + */
>> +
>> +/* Provide semaphore-like semantics for reads from the eventfd. */
>> +#define EFD_SEMAPHORE (1 << 0)
>> +/* Provide event mask semantics for the eventfd. */
>> +#define EFD_MASK (1 << 1)
>> +/*  Set the close-on-exec (FD_CLOEXEC) flag on the eventfd. */
>> +#define EFD_CLOEXEC O_CLOEXEC
>> +/*  Create the eventfd in non-blocking mode. */
>> +#define EFD_NONBLOCK O_NONBLOCK
>> +#endif /* _UAPI_LINUX_EVENTFD_H */
>> -- 
>> 1.9.1

Re: [PATCH v3 1/1] eventfd: implementation of EFD_MASK flag

[Laurent Pinchart]

Hi Damian,

On Tue, Jun 23, 2020 at 06:21:28PM +0900, Damian Hobson-Garcia wrote:
> On 2020-06-19 7:16 p.m., Paul Elder wrote:
> > Hello Damian, Martin, and all,
> > 
> > I came across this (quite old by now) patch to extend eventfd's polling
> > functionality. I was wondering what happened to it (why it hasn't been
> > merged yet) and if we could, or what is needed to, move it forward.
> 
> I think there was an open question about whether it was
> best to move the definitions of EFD_SEMAPHORE, etc out of
> /include/linux/eventfd.h and into a newly created
> /include/uapi/linux/eventfd.h as this patch does.

I would have thought that defining EFD_SEMAPHORE in a public API header
would be best, but it seems that glibc has its own definition in
bits/eventfd.h. I don't know what is usually preferred in these cases.

> I don't know if the maintainers have any concerns on this matter, or the
> patch in general, that would prevent this from moving forward.

Thanks for your reply. It seems a good way forward would be to resubmit
the patch then.

> > I was thinking to use it for V4L2 events support via polling in the V4L2
> > compatibility layer for libcamera [1]. We can signal V4L2 buffer
> > availability POLLOUT via write(), but there is no way to signal V4L2
> > events, as they are signaled via POLLPRI.
> > 
> > [1] https://libcamera.org/docs.html#id1
> > 
> > On Thu, Oct 15, 2015 at 10:42:08AM +0900, Damian Hobson-Garcia wrote:
> >> From: Martin Sustrik <sustrik@250bpm.com>
> >>
> >> When implementing network protocols in user space, one has to implement
> >> fake file descriptors to represent the sockets for the protocol.
> >>
> >> Polling on such fake file descriptors is a problem (poll/select/epoll
> >> accept only true file descriptors) and forces protocol implementers to use
> >> various workarounds resulting in complex, non-standard and convoluted APIs.
> >>
> >> More generally, ability to create full-blown file descriptors for
> >> userspace-to-userspace signalling is missing. While eventfd(2) goes half
> >> the way towards this goal it has follwoing shorcomings:
> >>
> >> I.  There's no way to signal POLLPRI, POLLHUP etc.
> >> II. There's no way to signal arbitrary combination of POLL* flags. Most
> >>     notably, simultaneous !POLLIN and !POLLOUT, which is a perfectly valid
> >>     combination for a network protocol (rx buffer is empty and tx buffer is
> >>     full), cannot be signaled using eventfd.
> >>
> >> This patch implements new EFD_MASK flag which solves the above problems.
> >>
> >> The semantics of EFD_MASK are as follows:
> >>
> >> eventfd(2):
> >>
> >> If eventfd is created with EFD_MASK flag set, it is initialised in such a
> >> way as to signal no events on the file descriptor when it is polled on.
> >> The 'initval' argument is ignored.
> >>
> >> write(2):
> >>
> >> User is allowed to write only buffers containing a 32-bit value
> >> representing any combination of event flags as defined by the poll(2)
> >> function (POLLIN, POLLOUT, POLLERR, POLLHUP etc.). Specified events
> >> will be signaled when polling (select, poll, epoll) on the eventfd is
> >> done later on.
> >>
> >> read(2):
> >>
> >> read is not supported and will fail with EINVAL.
> >>
> >> select(2), poll(2) and similar:
> >>
> >> When polling on the eventfd marked by EFD_MASK flag, all the events
> >> specified in last written event flags shall be signaled.
> >>
> >> Signed-off-by: Martin Sustrik <sustrik@250bpm.com>
> >>
> >> [dhobsong@igel.co.jp: Rebased, and resubmitted for Linux 4.3]
> >> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> >> ---
> >>  fs/eventfd.c                 | 102 ++++++++++++++++++++++++++++++++++++++-----
> >>  include/linux/eventfd.h      |  16 +------
> >>  include/uapi/linux/eventfd.h |  33 ++++++++++++++
> >>  3 files changed, 126 insertions(+), 25 deletions(-)
> >>  create mode 100644 include/uapi/linux/eventfd.h
> >>
> >> diff --git a/fs/eventfd.c b/fs/eventfd.c
> >> index 8d0c0df..1310779 100644
> >> --- a/fs/eventfd.c
> >> +++ b/fs/eventfd.c
> >> @@ -2,6 +2,7 @@
> >>   *  fs/eventfd.c
> >>   *
> >>   *  Copyright (C) 2007  Davide Libenzi <davidel@xmailserver.org>
> >> + *  Copyright (C) 2013  Martin Sustrik <sustrik@250bpm.com>
> >>   *
> >>   */
> >>  
> >> @@ -22,18 +23,31 @@
> >>  #include <linux/proc_fs.h>
> >>  #include <linux/seq_file.h>
> >>  
> >> +#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
> >> +#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE | EFD_MASK)
> >> +#define EFD_MASK_VALID_EVENTS (POLLIN | POLLPRI | POLLOUT | POLLERR | POLLHUP)
> >> +
> >>  struct eventfd_ctx {
> >>  	struct kref kref;
> >>  	wait_queue_head_t wqh;
> >> -	/*
> >> -	 * Every time that a write(2) is performed on an eventfd, the
> >> -	 * value of the __u64 being written is added to "count" and a
> >> -	 * wakeup is performed on "wqh". A read(2) will return the "count"
> >> -	 * value to userspace, and will reset "count" to zero. The kernel
> >> -	 * side eventfd_signal() also, adds to the "count" counter and
> >> -	 * issue a wakeup.
> >> -	 */
> >> -	__u64 count;
> >> +	union {
> >> +		/*
> >> +		 * Every time that a write(2) is performed on an eventfd, the
> >> +		 * value of the __u64 being written is added to "count" and a
> >> +		 * wakeup is performed on "wqh". A read(2) will return the
> >> +		 * "count" value to userspace, and will reset "count" to zero.
> >> +		 * The kernel side eventfd_signal() also, adds to the "count"
> >> +		 * counter and issue a wakeup.
> >> +		 */
> >> +		__u64 count;
> >> +
> >> +		/*
> >> +		 * When using eventfd in EFD_MASK mode this stracture stores the
> >> +		 * current events to be signaled on the eventfd (events member)
> >> +		 * along with opaque user-defined data (data member).
> >> +		 */
> >> +		__u32 events;
> >> +	};
> >>  	unsigned int flags;
> >>  };
> >>  
> >> @@ -134,6 +148,14 @@ static unsigned int eventfd_poll(struct file *file, poll_table *wait)
> >>  	return events;
> >>  }
> >>  
> >> +static unsigned int eventfd_mask_poll(struct file *file, poll_table *wait)
> >> +{
> >> +	struct eventfd_ctx *ctx = file->private_data;
> >> +
> >> +	poll_wait(file, &ctx->wqh, wait);
> >> +	return ctx->events;
> >> +}
> >> +
> >>  static void eventfd_ctx_do_read(struct eventfd_ctx *ctx, __u64 *cnt)
> >>  {
> >>  	*cnt = (ctx->flags & EFD_SEMAPHORE) ? 1 : ctx->count;
> >> @@ -239,6 +261,14 @@ static ssize_t eventfd_read(struct file *file, char __user *buf, size_t count,
> >>  	return put_user(cnt, (__u64 __user *) buf) ? -EFAULT : sizeof(cnt);
> >>  }
> >>  
> >> +static ssize_t eventfd_mask_read(struct file *file, char __user *buf,
> >> +			    size_t count,
> >> +			    loff_t *ppos)
> >> +{
> >> +	return -EINVAL;
> >> +}
> >> +
> >> +
> >>  static ssize_t eventfd_write(struct file *file, const char __user *buf, size_t count,
> >>  			     loff_t *ppos)
> >>  {
> >> @@ -286,6 +316,28 @@ static ssize_t eventfd_write(struct file *file, const char __user *buf, size_t c
> >>  	return res;
> >>  }
> >>  
> >> +static ssize_t eventfd_mask_write(struct file *file, const char __user *buf,
> >> +			     size_t count,
> >> +			     loff_t *ppos)
> >> +{
> >> +	struct eventfd_ctx *ctx = file->private_data;
> >> +	__u32 events;
> >> +
> >> +	if (count < sizeof(events))
> >> +		return -EINVAL;
> >> +	if (copy_from_user(&events, buf, sizeof(events)))
> >> +		return -EFAULT;
> >> +	if (events & ~EFD_MASK_VALID_EVENTS)
> >> +		return -EINVAL;
> >> +	spin_lock_irq(&ctx->wqh.lock);
> >> +	memcpy(&ctx->events, &events, sizeof(ctx->events));
> >> +	if (waitqueue_active(&ctx->wqh))
> >> +		wake_up_locked_poll(&ctx->wqh,
> >> +			(unsigned long)ctx->events);
> >> +	spin_unlock_irq(&ctx->wqh.lock);
> >> +	return sizeof(ctx->events);
> >> +}
> >> +
> >>  #ifdef CONFIG_PROC_FS
> >>  static void eventfd_show_fdinfo(struct seq_file *m, struct file *f)
> >>  {
> >> @@ -296,6 +348,16 @@ static void eventfd_show_fdinfo(struct seq_file *m, struct file *f)
> >>  		   (unsigned long long)ctx->count);
> >>  	spin_unlock_irq(&ctx->wqh.lock);
> >>  }
> >> +
> >> +static void eventfd_mask_show_fdinfo(struct seq_file *m, struct file *f)
> >> +{
> >> +	struct eventfd_ctx *ctx = f->private_data;
> >> +
> >> +	spin_lock_irq(&ctx->wqh.lock);
> >> +	seq_printf(m, "eventfd-mask: %x\n",
> >> +		ctx->events);
> >> +	spin_unlock_irq(&ctx->wqh.lock);
> >> +}
> >>  #endif
> >>  
> >>  static const struct file_operations eventfd_fops = {
> >> @@ -309,6 +371,17 @@ static const struct file_operations eventfd_fops = {
> >>  	.llseek		= noop_llseek,
> >>  };
> >>  
> >> +static const struct file_operations eventfd_mask_fops = {
> >> +#ifdef CONFIG_PROC_FS
> >> +	.show_fdinfo	= eventfd_mask_show_fdinfo,
> >> +#endif
> >> +	.release	= eventfd_release,
> >> +	.poll		= eventfd_mask_poll,
> >> +	.read		= eventfd_mask_read,
> >> +	.write		= eventfd_mask_write,
> >> +	.llseek		= noop_llseek,
> >> +};
> >> +
> >>  /**
> >>   * eventfd_fget - Acquire a reference of an eventfd file descriptor.
> >>   * @fd: [in] Eventfd file descriptor.
> >> @@ -392,6 +465,7 @@ struct file *eventfd_file_create(unsigned int count, int flags)
> >>  {
> >>  	struct file *file;
> >>  	struct eventfd_ctx *ctx;
> >> +	const struct file_operations *fops;
> >>  
> >>  	/* Check the EFD_* constants for consistency.  */
> >>  	BUILD_BUG_ON(EFD_CLOEXEC != O_CLOEXEC);
> >> @@ -406,10 +480,16 @@ struct file *eventfd_file_create(unsigned int count, int flags)
> >>  
> >>  	kref_init(&ctx->kref);
> >>  	init_waitqueue_head(&ctx->wqh);
> >> -	ctx->count = count;
> >> +	if (flags & EFD_MASK) {
> >> +		ctx->events = 0;
> >> +		fops = &eventfd_mask_fops;
> >> +	} else {
> >> +		ctx->count = count;
> >> +		fops = &eventfd_fops;
> >> +	}
> >>  	ctx->flags = flags;
> >>  
> >> -	file = anon_inode_getfile("[eventfd]", &eventfd_fops, ctx,
> >> +	file = anon_inode_getfile("[eventfd]", fops, ctx,
> >>  				  O_RDWR | (flags & EFD_SHARED_FCNTL_FLAGS));
> >>  	if (IS_ERR(file))
> >>  		eventfd_free_ctx(ctx);
> >> diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h
> >> index ff0b981..87de343 100644
> >> --- a/include/linux/eventfd.h
> >> +++ b/include/linux/eventfd.h
> >> @@ -8,23 +8,11 @@
> >>  #ifndef _LINUX_EVENTFD_H
> >>  #define _LINUX_EVENTFD_H
> >>  
> >> +#include <uapi/linux/eventfd.h>
> >> +
> >>  #include <linux/fcntl.h>
> >>  #include <linux/wait.h>
> >>  
> >> -/*
> >> - * CAREFUL: Check include/uapi/asm-generic/fcntl.h when defining
> >> - * new flags, since they might collide with O_* ones. We want
> >> - * to re-use O_* flags that couldn't possibly have a meaning
> >> - * from eventfd, in order to leave a free define-space for
> >> - * shared O_* flags.
> >> - */
> >> -#define EFD_SEMAPHORE (1 << 0)
> >> -#define EFD_CLOEXEC O_CLOEXEC
> >> -#define EFD_NONBLOCK O_NONBLOCK
> >> -
> >> -#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
> >> -#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
> >> -
> >>  struct file;
> >>  
> >>  #ifdef CONFIG_EVENTFD
> >> diff --git a/include/uapi/linux/eventfd.h b/include/uapi/linux/eventfd.h
> >> new file mode 100644
> >> index 0000000..097dcad
> >> --- /dev/null
> >> +++ b/include/uapi/linux/eventfd.h
> >> @@ -0,0 +1,33 @@
> >> +/*
> >> + *  Copyright (C) 2013 Martin Sustrik <sustrik@250bpm.com>
> >> + *
> >> + *  This program is free software; you can redistribute it and/or modify
> >> + *  it under the terms of the GNU General Public License as published by
> >> + *  the Free Software Foundation; either version 2 of the License, or
> >> + *  (at your option) any later version.
> >> + */
> >> +
> >> +#ifndef _UAPI_LINUX_EVENTFD_H
> >> +#define _UAPI_LINUX_EVENTFD_H
> >> +
> >> +/* For O_CLOEXEC */
> >> +#include <linux/fcntl.h>
> >> +#include <linux/types.h>
> >> +
> >> +/*
> >> + * CAREFUL: Check include/asm-generic/fcntl.h when defining
> >> + * new flags, since they might collide with O_* ones. We want
> >> + * to re-use O_* flags that couldn't possibly have a meaning
> >> + * from eventfd, in order to leave a free define-space for
> >> + * shared O_* flags.
> >> + */
> >> +
> >> +/* Provide semaphore-like semantics for reads from the eventfd. */
> >> +#define EFD_SEMAPHORE (1 << 0)
> >> +/* Provide event mask semantics for the eventfd. */
> >> +#define EFD_MASK (1 << 1)
> >> +/*  Set the close-on-exec (FD_CLOEXEC) flag on the eventfd. */
> >> +#define EFD_CLOEXEC O_CLOEXEC
> >> +/*  Create the eventfd in non-blocking mode. */
> >> +#define EFD_NONBLOCK O_NONBLOCK
> >> +#endif /* _UAPI_LINUX_EVENTFD_H */

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 1/1] eventfd: implementation of EFD_MASK flag

[Martin Sustrik]

On 2015-07-09 11:06, Damian Hobson-Garcia wrote:
> Hi Martin,
> 
> On 2015-07-09 5:41 PM, Martin Sustrik wrote:
>> Hi Damian,
>> 
>> Yes, this patch would be geneally useful for implementing stuff in 
>> user
>> space that otherwise would have to live in kernelspace.
>> 
>> Unfortunately, I have no cycles left to pursue getting it to the
>> mainline. If you feel like you can take care of it, that would be 
>> great.
> I'll see what I can do. I'll rebase it and resubmit it.
> Unless there are major changes (which I highly doubt), I'll keep you as
> the author, if that's ok with you.

Sure. No problem.

>> 
>> I can help with the documentation, if needed.
> Thank you, that would be very much appreciated.

Ok, ping me when it's needed.

Thanks!
Martin

Re: [PATCH v3 1/1] eventfd: implementation of EFD_MASK flag

[Damian Hobson-Garcia]

Hi Martin,

On 2015-07-09 5:41 PM, Martin Sustrik wrote:
> Hi Damian,
> 
> Yes, this patch would be geneally useful for implementing stuff in user
> space that otherwise would have to live in kernelspace.
> 
> Unfortunately, I have no cycles left to pursue getting it to the
> mainline. If you feel like you can take care of it, that would be great.
I'll see what I can do. I'll rebase it and resubmit it.
Unless there are major changes (which I highly doubt), I'll keep you as
the author, if that's ok with you.
> 
> I can help with the documentation, if needed.
Thank you, that would be very much appreciated.

Damian

Re: [PATCH v3 1/1] eventfd: implementation of EFD_MASK flag

[Martin Sustrik]

Hi Damian,

Yes, this patch would be geneally useful for implementing stuff in user 
space that otherwise would have to live in kernelspace.

Unfortunately, I have no cycles left to pursue getting it to the 
mainline. If you feel like you can take care of it, that would be great.

I can help with the documentation, if needed.

Martin

On 2015-07-09 09:57, Damian Hobson-Garcia wrote:
> Hello Martin and all,
> 
> I recently came across this (quite old by now) patch submission for an
> extension to the functionality of eventfd and I noticed that the
> discussion seems to have fizzled out.  Is this functionality still of
> use for user space network protocols?  It seems like it would be usable
> for other notification use cases as well.  In particular I'm thinking 
> of
> poll/select/epoll support for a user space video codec via libv4l.
> Is there value in re-examining this patch?
> 
> Thank you,
> Damian
> 
> On 2013-02-18 8:34 PM, Martin Sustrik wrote:
>> When implementing network protocols in user space, one has to 
>> implement
>> fake file descriptors to represent the sockets for the protocol.
>> 
>> Polling on such fake file descriptors is a problem (poll/select/epoll 
>> accept
>> only true file descriptors) and forces protocol implementers to use 
>> various
>> workarounds resulting in complex, non-standard and convoluted APIs.
>> 
>> More generally, ability to create full-blown file descriptors for
>> userspace-to-userspace signalling is missing. While eventfd(2) goes 
>> half the way
>> towards this goal it has follwoing shorcomings:
>> 
>> I.  There's no way to signal POLLPRI, POLLHUP etc.
>> II. There's no way to signal arbitrary combination of POLL* flags. 
>> Most notably,
>>     simultaneous !POLLIN and !POLLOUT, which is a perfectly valid 
>> combination
>>     for a network protocol (rx buffer is empty and tx buffer is full), 
>> cannot be
>>     signaled using eventfd.
>> 
>> This patch implements new EFD_MASK flag which solves the above 
>> problems.
>> 
>> Additionally, to provide a way to associate user-space state with 
>> eventfd
>> object, it allows to attach user-space data to the file descriptor.
>> 
>> The semantics of EFD_MASK are as follows:
>> 
>> eventfd(2):
>> 
>> If eventfd is created with EFD_MASK flag set, it is initialised in 
>> such a way
>> as to signal no events on the file descriptor when it is polled on. 
>> 'initval'
>> argument is ignored.
>> 
>> write(2):
>> 
>> User is allowed to write only buffers containing the following 
>> structure:
>> 
>> struct efd_mask {
>>   short events;
>>   union {
>>     void    *ptr;
>>     uint32_t u32;
>>     uint64_t u64;
>>   };
>> };
>> 
>> The value of 'events' should be any combination of event flags as 
>> defined by
>> poll(2) function (POLLIN, POLLOUT, POLLERR, POLLHUP etc.) Specified 
>> events will
>> be signaled when polling (select, poll, epoll) on the eventfd is done 
>> later on.
>> 'ptr', 'u32' or 'u64' are opaque data that are not interpreted by 
>> eventfd
>> object.
>> 
>> read(2):
>> 
>> User is allowed to read an efd_mask structure from the eventfd marked 
>> by
>> EFD_MASK. Returned value shall be the last one written to the eventfd.
>> 
>> select(2), poll(2) and similar:
>> 
>> When polling on the eventfd marked by EFD_MASK flag, all the events 
>> specified
>> in last written 'events' field shall be signaled.
>> 
>> Signed-off-by: Martin Sustrik <sustrik@250bpm.com>
>> ---
>> Following changes were made to the patch since v2:
>> 
>> - eventfd_mask structure renamed to efd_mask to keep user-space 
>> prefixes
>>   consistent
>> - efd_mask is __packed for all architectures
>> - eventfd.h header file added to uapi; given there wasn't one so far, 
>> in
>>   addition to moving efd_mask there, I've moved also all the eventfd 
>> flags that
>>   are meant to be visible from the user space
>> - comment for 'mask' member eventfd_ctx added
>> - synchronisation bugs in eventfd_read fixed
>> - several variable declarations moved from the beginning of the 
>> function to
>>   the blocks where they are used
>> - changelog text made simpler and more up to the point
>> 
>> There was also a request to explain why this functionality is needed. 
>> I've
>> written an article explaining the problems user-space network protocol
>> implementers face here: http://www.250bpm.com/blog:16
>> ---
>>  fs/eventfd.c                 |  194 
>> ++++++++++++++++++++++++++++--------------
>>  include/linux/eventfd.h      |   17 +---
>>  include/uapi/linux/eventfd.h |   40 +++++++++
>>  3 files changed, 172 insertions(+), 79 deletions(-)
>>  create mode 100644 include/uapi/linux/eventfd.h
>> 
>> diff --git a/fs/eventfd.c b/fs/eventfd.c
>> index 35470d9..8f821b1 100644
>> --- a/fs/eventfd.c
>> +++ b/fs/eventfd.c
>> @@ -2,6 +2,7 @@
>>   *  fs/eventfd.c
>>   *
>>   *  Copyright (C) 2007  Davide Libenzi <davidel@xmailserver.org>
>> + *  Copyright (C) 2013  Martin Sustrik <sustrik@250bpm.com>
>>   *
>>   */
>> 
>> @@ -22,18 +23,31 @@
>>  #include <linux/proc_fs.h>
>>  #include <linux/seq_file.h>
>> 
>> +#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
>> +#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE | 
>> EFD_MASK)
>> +#define EFD_MASK_VALID_EVENTS (POLLIN | POLLPRI | POLLOUT | POLLERR | 
>> POLLHUP)
>> +
>>  struct eventfd_ctx {
>>  	struct kref kref;
>>  	wait_queue_head_t wqh;
>> -	/*
>> -	 * Every time that a write(2) is performed on an eventfd, the
>> -	 * value of the __u64 being written is added to "count" and a
>> -	 * wakeup is performed on "wqh". A read(2) will return the "count"
>> -	 * value to userspace, and will reset "count" to zero. The kernel
>> -	 * side eventfd_signal() also, adds to the "count" counter and
>> -	 * issue a wakeup.
>> -	 */
>> -	__u64 count;
>> +	union {
>> +		/*
>> +		 * Every time that a write(2) is performed on an eventfd, the
>> +		 * value of the __u64 being written is added to "count" and a
>> +		 * wakeup is performed on "wqh". A read(2) will return the
>> +		 * "count" value to userspace, and will reset "count" to zero.
>> +		 * The kernel side eventfd_signal() also, adds to the "count"
>> +		 * counter and issue a wakeup.
>> +		 */
>> +		__u64 count;
>> +
>> +		/*
>> +		 * When using eventfd in EFD_MASK mode this stracture stores the
>> +		 * current events to be signaled on the eventfd (events member)
>> +		 * along with opaque user-defined data (data member).
>> +		 */
>> +		struct efd_mask mask;
>> +	};
>>  	unsigned int flags;
>>  };
>> 
>> @@ -55,6 +69,9 @@ __u64 eventfd_signal(struct eventfd_ctx *ctx, __u64 
>> n)
>>  {
>>  	unsigned long flags;
>> 
>> +	/* This function should never be used with eventfd in the mask mode. 
>> */
>> +	BUG_ON(ctx->flags & EFD_MASK);
>> +
>>  	spin_lock_irqsave(&ctx->wqh.lock, flags);
>>  	if (ULLONG_MAX - ctx->count < n)
>>  		n = ULLONG_MAX - ctx->count;
>> @@ -123,12 +140,16 @@ static unsigned int eventfd_poll(struct file 
>> *file, poll_table *wait)
>>  	poll_wait(file, &ctx->wqh, wait);
>> 
>>  	spin_lock_irqsave(&ctx->wqh.lock, flags);
>> -	if (ctx->count > 0)
>> -		events |= POLLIN;
>> -	if (ctx->count == ULLONG_MAX)
>> -		events |= POLLERR;
>> -	if (ULLONG_MAX - 1 > ctx->count)
>> -		events |= POLLOUT;
>> +	if (ctx->flags & EFD_MASK) {
>> +		events = ctx->mask.events;
>> +	} else {
>> +		if (ctx->count > 0)
>> +			events |= POLLIN;
>> +		if (ctx->count == ULLONG_MAX)
>> +			events |= POLLERR;
>> +		if (ULLONG_MAX - 1 > ctx->count)
>> +			events |= POLLOUT;
>> +	}
>>  	spin_unlock_irqrestore(&ctx->wqh.lock, flags);
>> 
>>  	return events;
>> @@ -158,6 +179,9 @@ int eventfd_ctx_remove_wait_queue(struct 
>> eventfd_ctx *ctx, wait_queue_t *wait,
>>  {
>>  	unsigned long flags;
>> 
>> +	/* This function should never be used with eventfd in the mask mode. 
>> */
>> +	BUG_ON(ctx->flags & EFD_MASK);
>> +
>>  	spin_lock_irqsave(&ctx->wqh.lock, flags);
>>  	eventfd_ctx_do_read(ctx, cnt);
>>  	__remove_wait_queue(&ctx->wqh, wait);
>> @@ -188,6 +212,9 @@ ssize_t eventfd_ctx_read(struct eventfd_ctx *ctx, 
>> int no_wait, __u64 *cnt)
>>  	ssize_t res;
>>  	DECLARE_WAITQUEUE(wait, current);
>> 
>> +	/* This function should never be used with eventfd in the mask mode. 
>> */
>> +	BUG_ON(ctx->flags & EFD_MASK);
>> +
>>  	spin_lock_irq(&ctx->wqh.lock);
>>  	*cnt = 0;
>>  	res = -EAGAIN;
>> @@ -227,63 +254,92 @@ static ssize_t eventfd_read(struct file *file, 
>> char __user *buf, size_t count,
>>  			    loff_t *ppos)
>>  {
>>  	struct eventfd_ctx *ctx = file->private_data;
>> -	ssize_t res;
>> -	__u64 cnt;
>> 
>> -	if (count < sizeof(cnt))
>> -		return -EINVAL;
>> -	res = eventfd_ctx_read(ctx, file->f_flags & O_NONBLOCK, &cnt);
>> -	if (res < 0)
>> -		return res;
>> -
>> -	return put_user(cnt, (__u64 __user *) buf) ? -EFAULT : sizeof(cnt);
>> +	if (ctx->flags & EFD_MASK) {
>> +		struct efd_mask mask;
>> +		if (count < sizeof(mask))
>> +			return -EINVAL;
>> +		spin_lock_irq(&ctx->wqh.lock);
>> +		mask = ctx->mask;
>> +		spin_unlock_irq(&ctx->wqh.lock);
>> +		if (copy_to_user(buf, &mask, sizeof(mask)))
>> +			return -EFAULT;
>> +		return sizeof(mask);
>> +	} else {
>> +		ssize_t res;
>> +		__u64 cnt;
>> +		if (count < sizeof(cnt))
>> +			return -EINVAL;
>> +		res = eventfd_ctx_read(ctx, file->f_flags & O_NONBLOCK, &cnt);
>> +		if (res < 0)
>> +			return res;
>> +		return put_user(cnt, (__u64 __user *) buf) ?
>> +			-EFAULT : sizeof(cnt);
>> +	}
>>  }
>> 
>>  static ssize_t eventfd_write(struct file *file, const char __user 
>> *buf, size_t count,
>>  			     loff_t *ppos)
>>  {
>>  	struct eventfd_ctx *ctx = file->private_data;
>> -	ssize_t res;
>> -	__u64 ucnt;
>> -	DECLARE_WAITQUEUE(wait, current);
>> 
>> -	if (count < sizeof(ucnt))
>> -		return -EINVAL;
>> -	if (copy_from_user(&ucnt, buf, sizeof(ucnt)))
>> -		return -EFAULT;
>> -	if (ucnt == ULLONG_MAX)
>> -		return -EINVAL;
>> -	spin_lock_irq(&ctx->wqh.lock);
>> -	res = -EAGAIN;
>> -	if (ULLONG_MAX - ctx->count > ucnt)
>> -		res = sizeof(ucnt);
>> -	else if (!(file->f_flags & O_NONBLOCK)) {
>> -		__add_wait_queue(&ctx->wqh, &wait);
>> -		for (res = 0;;) {
>> -			set_current_state(TASK_INTERRUPTIBLE);
>> -			if (ULLONG_MAX - ctx->count > ucnt) {
>> -				res = sizeof(ucnt);
>> -				break;
>> -			}
>> -			if (signal_pending(current)) {
>> -				res = -ERESTARTSYS;
>> -				break;
>> +	if (ctx->flags & EFD_MASK) {
>> +		struct efd_mask mask;
>> +		if (count < sizeof(mask))
>> +			return -EINVAL;
>> +		if (copy_from_user(&mask, buf, sizeof(mask)))
>> +			return -EFAULT;
>> +		if (mask.events & ~EFD_MASK_VALID_EVENTS)
>> +			return -EINVAL;
>> +		spin_lock_irq(&ctx->wqh.lock);
>> +		memcpy(&ctx->mask, &mask, sizeof(ctx->mask));
>> +		if (waitqueue_active(&ctx->wqh))
>> +			wake_up_locked_poll(&ctx->wqh,
>> +				(unsigned long)ctx->mask.events);
>> +		spin_unlock_irq(&ctx->wqh.lock);
>> +		return sizeof(ctx->mask);
>> +	} else {
>> +		ssize_t res;
>> +		__u64 ucnt;
>> +		DECLARE_WAITQUEUE(wait, current);
>> +		if (count < sizeof(ucnt))
>> +			return -EINVAL;
>> +		if (copy_from_user(&ucnt, buf, sizeof(ucnt)))
>> +			return -EFAULT;
>> +		if (ucnt == ULLONG_MAX)
>> +			return -EINVAL;
>> +		spin_lock_irq(&ctx->wqh.lock);
>> +		res = -EAGAIN;
>> +		if (ULLONG_MAX - ctx->count > ucnt)
>> +			res = sizeof(ucnt);
>> +		else if (!(file->f_flags & O_NONBLOCK)) {
>> +			__add_wait_queue(&ctx->wqh, &wait);
>> +			for (res = 0;;) {
>> +				set_current_state(TASK_INTERRUPTIBLE);
>> +				if (ULLONG_MAX - ctx->count > ucnt) {
>> +					res = sizeof(ucnt);
>> +					break;
>> +				}
>> +				if (signal_pending(current)) {
>> +					res = -ERESTARTSYS;
>> +					break;
>> +				}
>> +				spin_unlock_irq(&ctx->wqh.lock);
>> +				schedule();
>> +				spin_lock_irq(&ctx->wqh.lock);
>>  			}
>> -			spin_unlock_irq(&ctx->wqh.lock);
>> -			schedule();
>> -			spin_lock_irq(&ctx->wqh.lock);
>> +			__remove_wait_queue(&ctx->wqh, &wait);
>> +			__set_current_state(TASK_RUNNING);
>>  		}
>> -		__remove_wait_queue(&ctx->wqh, &wait);
>> -		__set_current_state(TASK_RUNNING);
>> -	}
>> -	if (likely(res > 0)) {
>> -		ctx->count += ucnt;
>> -		if (waitqueue_active(&ctx->wqh))
>> -			wake_up_locked_poll(&ctx->wqh, POLLIN);
>> -	}
>> -	spin_unlock_irq(&ctx->wqh.lock);
>> +		if (likely(res > 0)) {
>> +			ctx->count += ucnt;
>> +			if (waitqueue_active(&ctx->wqh))
>> +				wake_up_locked_poll(&ctx->wqh, POLLIN);
>> +		}
>> +		spin_unlock_irq(&ctx->wqh.lock);
>> 
>> -	return res;
>> +		return res;
>> +	}
>>  }
>> 
>>  #ifdef CONFIG_PROC_FS
>> @@ -293,8 +349,13 @@ static int eventfd_show_fdinfo(struct seq_file 
>> *m, struct file *f)
>>  	int ret;
>> 
>>  	spin_lock_irq(&ctx->wqh.lock);
>> -	ret = seq_printf(m, "eventfd-count: %16llx\n",
>> -			 (unsigned long long)ctx->count);
>> +	if (ctx->flags & EFD_MASK) {
>> +		ret = seq_printf(m, "eventfd-mask: %x\n",
>> +				 (unsigned)ctx->mask.events);
>> +	} else {
>> +		ret = seq_printf(m, "eventfd-count: %16llx\n",
>> +				 (unsigned long long)ctx->count);
>> +	}
>>  	spin_unlock_irq(&ctx->wqh.lock);
>> 
>>  	return ret;
>> @@ -412,7 +473,12 @@ struct file *eventfd_file_create(unsigned int 
>> count, int flags)
>> 
>>  	kref_init(&ctx->kref);
>>  	init_waitqueue_head(&ctx->wqh);
>> -	ctx->count = count;
>> +	if (flags & EFD_MASK) {
>> +		ctx->mask.events = 0;
>> +		ctx->mask.data = 0;
>> +	} else {
>> +		ctx->count = count;
>> +	}
>>  	ctx->flags = flags;
>> 
>>  	file = anon_inode_getfile("[eventfd]", &eventfd_fops, ctx,
>> diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h
>> index 3c3ef19..218aba6 100644
>> --- a/include/linux/eventfd.h
>> +++ b/include/linux/eventfd.h
>> @@ -8,24 +8,11 @@
>>  #ifndef _LINUX_EVENTFD_H
>>  #define _LINUX_EVENTFD_H
>> 
>> -#include <linux/fcntl.h>
>> +#include <uapi/linux/eventfd.h>
>> +
>>  #include <linux/file.h>
>>  #include <linux/wait.h>
>> 
>> -/*
>> - * CAREFUL: Check include/asm-generic/fcntl.h when defining
>> - * new flags, since they might collide with O_* ones. We want
>> - * to re-use O_* flags that couldn't possibly have a meaning
>> - * from eventfd, in order to leave a free define-space for
>> - * shared O_* flags.
>> - */
>> -#define EFD_SEMAPHORE (1 << 0)
>> -#define EFD_CLOEXEC O_CLOEXEC
>> -#define EFD_NONBLOCK O_NONBLOCK
>> -
>> -#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
>> -#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
>> -
>>  #ifdef CONFIG_EVENTFD
>> 
>>  struct file *eventfd_file_create(unsigned int count, int flags);
>> diff --git a/include/uapi/linux/eventfd.h 
>> b/include/uapi/linux/eventfd.h
>> new file mode 100644
>> index 0000000..03057a5
>> --- /dev/null
>> +++ b/include/uapi/linux/eventfd.h
>> @@ -0,0 +1,40 @@
>> +/*
>> + *  Copyright (C) 2013 Martin Sustrik <sustrik@250bpm.com>
>> + *
>> + *  This program is free software; you can redistribute it and/or 
>> modify
>> + *  it under the terms of the GNU General Public License as published 
>> by
>> + *  the Free Software Foundation; either version 2 of the License, or
>> + *  (at your option) any later version.
>> + */
>> +
>> +#ifndef _UAPI_LINUX_EVENTFD_H
>> +#define _UAPI_LINUX_EVENTFD_H
>> +
>> +/* For O_CLOEXEC */
>> +#include <linux/fcntl.h>
>> +#include <linux/types.h>
>> +
>> +/*
>> + * CAREFUL: Check include/asm-generic/fcntl.h when defining
>> + * new flags, since they might collide with O_* ones. We want
>> + * to re-use O_* flags that couldn't possibly have a meaning
>> + * from eventfd, in order to leave a free define-space for
>> + * shared O_* flags.
>> + */
>> +
>> +/* Provide semaphore-like semantics for reads from the eventfd. */
>> +#define EFD_SEMAPHORE (1 << 0)
>> +/* Provide event mask semantics for the eventfd. */
>> +#define EFD_MASK (1 << 1)
>> +/*  Set the close-on-exec (FD_CLOEXEC) flag on the eventfd. */
>> +#define EFD_CLOEXEC O_CLOEXEC
>> +/*  Create the eventfd in non-blocking mode. */
>> +#define EFD_NONBLOCK O_NONBLOCK
>> +
>> +/* Structure to read/write to eventfd in EFD_MASK mode. */
>> +struct efd_mask {
>> +	__u32 events;
>> +	__u64 data;
>> +} __packed;
>> +
>> +#endif /* _UAPI_LINUX_EVENTFD_H */
>> 

Re: [PATCH v3 1/1] eventfd: implementation of EFD_MASK flag

[Damian Hobson-Garcia]

Hello Martin and all,

I recently came across this (quite old by now) patch submission for an
extension to the functionality of eventfd and I noticed that the
discussion seems to have fizzled out.  Is this functionality still of
use for user space network protocols?  It seems like it would be usable
for other notification use cases as well.  In particular I'm thinking of
poll/select/epoll support for a user space video codec via libv4l.
Is there value in re-examining this patch?

Thank you,
Damian

On 2013-02-18 8:34 PM, Martin Sustrik wrote:
> When implementing network protocols in user space, one has to implement
> fake file descriptors to represent the sockets for the protocol.
> 
> Polling on such fake file descriptors is a problem (poll/select/epoll accept
> only true file descriptors) and forces protocol implementers to use various
> workarounds resulting in complex, non-standard and convoluted APIs.
> 
> More generally, ability to create full-blown file descriptors for
> userspace-to-userspace signalling is missing. While eventfd(2) goes half the way
> towards this goal it has follwoing shorcomings:
> 
> I.  There's no way to signal POLLPRI, POLLHUP etc.
> II. There's no way to signal arbitrary combination of POLL* flags. Most notably,
>     simultaneous !POLLIN and !POLLOUT, which is a perfectly valid combination
>     for a network protocol (rx buffer is empty and tx buffer is full), cannot be
>     signaled using eventfd.
> 
> This patch implements new EFD_MASK flag which solves the above problems.
> 
> Additionally, to provide a way to associate user-space state with eventfd
> object, it allows to attach user-space data to the file descriptor.
> 
> The semantics of EFD_MASK are as follows:
> 
> eventfd(2):
> 
> If eventfd is created with EFD_MASK flag set, it is initialised in such a way
> as to signal no events on the file descriptor when it is polled on. 'initval'
> argument is ignored.
> 
> write(2):
> 
> User is allowed to write only buffers containing the following structure:
> 
> struct efd_mask {
>   short events;
>   union {
>     void    *ptr;
>     uint32_t u32;
>     uint64_t u64;
>   };
> };
> 
> The value of 'events' should be any combination of event flags as defined by
> poll(2) function (POLLIN, POLLOUT, POLLERR, POLLHUP etc.) Specified events will
> be signaled when polling (select, poll, epoll) on the eventfd is done later on.
> 'ptr', 'u32' or 'u64' are opaque data that are not interpreted by eventfd
> object.
> 
> read(2):
> 
> User is allowed to read an efd_mask structure from the eventfd marked by
> EFD_MASK. Returned value shall be the last one written to the eventfd.
> 
> select(2), poll(2) and similar:
> 
> When polling on the eventfd marked by EFD_MASK flag, all the events specified
> in last written 'events' field shall be signaled.
> 
> Signed-off-by: Martin Sustrik <sustrik@250bpm.com>
> ---
> Following changes were made to the patch since v2:
> 
> - eventfd_mask structure renamed to efd_mask to keep user-space prefixes
>   consistent
> - efd_mask is __packed for all architectures
> - eventfd.h header file added to uapi; given there wasn't one so far, in
>   addition to moving efd_mask there, I've moved also all the eventfd flags that
>   are meant to be visible from the user space
> - comment for 'mask' member eventfd_ctx added 
> - synchronisation bugs in eventfd_read fixed
> - several variable declarations moved from the beginning of the function to
>   the blocks where they are used
> - changelog text made simpler and more up to the point
> 
> There was also a request to explain why this functionality is needed. I've
> written an article explaining the problems user-space network protocol
> implementers face here: http://www.250bpm.com/blog:16
> ---
>  fs/eventfd.c                 |  194 ++++++++++++++++++++++++++++--------------
>  include/linux/eventfd.h      |   17 +---
>  include/uapi/linux/eventfd.h |   40 +++++++++
>  3 files changed, 172 insertions(+), 79 deletions(-)
>  create mode 100644 include/uapi/linux/eventfd.h
> 
> diff --git a/fs/eventfd.c b/fs/eventfd.c
> index 35470d9..8f821b1 100644
> --- a/fs/eventfd.c
> +++ b/fs/eventfd.c
> @@ -2,6 +2,7 @@
>   *  fs/eventfd.c
>   *
>   *  Copyright (C) 2007  Davide Libenzi <davidel@xmailserver.org>
> + *  Copyright (C) 2013  Martin Sustrik <sustrik@250bpm.com>
>   *
>   */
>  
> @@ -22,18 +23,31 @@
>  #include <linux/proc_fs.h>
>  #include <linux/seq_file.h>
>  
> +#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
> +#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE | EFD_MASK)
> +#define EFD_MASK_VALID_EVENTS (POLLIN | POLLPRI | POLLOUT | POLLERR | POLLHUP)
> +
>  struct eventfd_ctx {
>  	struct kref kref;
>  	wait_queue_head_t wqh;
> -	/*
> -	 * Every time that a write(2) is performed on an eventfd, the
> -	 * value of the __u64 being written is added to "count" and a
> -	 * wakeup is performed on "wqh". A read(2) will return the "count"
> -	 * value to userspace, and will reset "count" to zero. The kernel
> -	 * side eventfd_signal() also, adds to the "count" counter and
> -	 * issue a wakeup.
> -	 */
> -	__u64 count;
> +	union {
> +		/*
> +		 * Every time that a write(2) is performed on an eventfd, the
> +		 * value of the __u64 being written is added to "count" and a
> +		 * wakeup is performed on "wqh". A read(2) will return the
> +		 * "count" value to userspace, and will reset "count" to zero.
> +		 * The kernel side eventfd_signal() also, adds to the "count"
> +		 * counter and issue a wakeup.
> +		 */
> +		__u64 count;
> +
> +		/*
> +		 * When using eventfd in EFD_MASK mode this stracture stores the
> +		 * current events to be signaled on the eventfd (events member)
> +		 * along with opaque user-defined data (data member).
> +		 */
> +		struct efd_mask mask;
> +	};
>  	unsigned int flags;
>  };
>  
> @@ -55,6 +69,9 @@ __u64 eventfd_signal(struct eventfd_ctx *ctx, __u64 n)
>  {
>  	unsigned long flags;
>  
> +	/* This function should never be used with eventfd in the mask mode. */
> +	BUG_ON(ctx->flags & EFD_MASK);
> +
>  	spin_lock_irqsave(&ctx->wqh.lock, flags);
>  	if (ULLONG_MAX - ctx->count < n)
>  		n = ULLONG_MAX - ctx->count;
> @@ -123,12 +140,16 @@ static unsigned int eventfd_poll(struct file *file, poll_table *wait)
>  	poll_wait(file, &ctx->wqh, wait);
>  
>  	spin_lock_irqsave(&ctx->wqh.lock, flags);
> -	if (ctx->count > 0)
> -		events |= POLLIN;
> -	if (ctx->count == ULLONG_MAX)
> -		events |= POLLERR;
> -	if (ULLONG_MAX - 1 > ctx->count)
> -		events |= POLLOUT;
> +	if (ctx->flags & EFD_MASK) {
> +		events = ctx->mask.events;
> +	} else {
> +		if (ctx->count > 0)
> +			events |= POLLIN;
> +		if (ctx->count == ULLONG_MAX)
> +			events |= POLLERR;
> +		if (ULLONG_MAX - 1 > ctx->count)
> +			events |= POLLOUT;
> +	}
>  	spin_unlock_irqrestore(&ctx->wqh.lock, flags);
>  
>  	return events;
> @@ -158,6 +179,9 @@ int eventfd_ctx_remove_wait_queue(struct eventfd_ctx *ctx, wait_queue_t *wait,
>  {
>  	unsigned long flags;
>  
> +	/* This function should never be used with eventfd in the mask mode. */
> +	BUG_ON(ctx->flags & EFD_MASK);
> +
>  	spin_lock_irqsave(&ctx->wqh.lock, flags);
>  	eventfd_ctx_do_read(ctx, cnt);
>  	__remove_wait_queue(&ctx->wqh, wait);
> @@ -188,6 +212,9 @@ ssize_t eventfd_ctx_read(struct eventfd_ctx *ctx, int no_wait, __u64 *cnt)
>  	ssize_t res;
>  	DECLARE_WAITQUEUE(wait, current);
>  
> +	/* This function should never be used with eventfd in the mask mode. */
> +	BUG_ON(ctx->flags & EFD_MASK);
> +
>  	spin_lock_irq(&ctx->wqh.lock);
>  	*cnt = 0;
>  	res = -EAGAIN;
> @@ -227,63 +254,92 @@ static ssize_t eventfd_read(struct file *file, char __user *buf, size_t count,
>  			    loff_t *ppos)
>  {
>  	struct eventfd_ctx *ctx = file->private_data;
> -	ssize_t res;
> -	__u64 cnt;
>  
> -	if (count < sizeof(cnt))
> -		return -EINVAL;
> -	res = eventfd_ctx_read(ctx, file->f_flags & O_NONBLOCK, &cnt);
> -	if (res < 0)
> -		return res;
> -
> -	return put_user(cnt, (__u64 __user *) buf) ? -EFAULT : sizeof(cnt);
> +	if (ctx->flags & EFD_MASK) {
> +		struct efd_mask mask;
> +		if (count < sizeof(mask))
> +			return -EINVAL;
> +		spin_lock_irq(&ctx->wqh.lock);
> +		mask = ctx->mask;
> +		spin_unlock_irq(&ctx->wqh.lock);
> +		if (copy_to_user(buf, &mask, sizeof(mask)))
> +			return -EFAULT;
> +		return sizeof(mask);
> +	} else {
> +		ssize_t res;
> +		__u64 cnt;
> +		if (count < sizeof(cnt))
> +			return -EINVAL;
> +		res = eventfd_ctx_read(ctx, file->f_flags & O_NONBLOCK, &cnt);
> +		if (res < 0)
> +			return res;
> +		return put_user(cnt, (__u64 __user *) buf) ?
> +			-EFAULT : sizeof(cnt);
> +	}
>  }
>  
>  static ssize_t eventfd_write(struct file *file, const char __user *buf, size_t count,
>  			     loff_t *ppos)
>  {
>  	struct eventfd_ctx *ctx = file->private_data;
> -	ssize_t res;
> -	__u64 ucnt;
> -	DECLARE_WAITQUEUE(wait, current);
>  
> -	if (count < sizeof(ucnt))
> -		return -EINVAL;
> -	if (copy_from_user(&ucnt, buf, sizeof(ucnt)))
> -		return -EFAULT;
> -	if (ucnt == ULLONG_MAX)
> -		return -EINVAL;
> -	spin_lock_irq(&ctx->wqh.lock);
> -	res = -EAGAIN;
> -	if (ULLONG_MAX - ctx->count > ucnt)
> -		res = sizeof(ucnt);
> -	else if (!(file->f_flags & O_NONBLOCK)) {
> -		__add_wait_queue(&ctx->wqh, &wait);
> -		for (res = 0;;) {
> -			set_current_state(TASK_INTERRUPTIBLE);
> -			if (ULLONG_MAX - ctx->count > ucnt) {
> -				res = sizeof(ucnt);
> -				break;
> -			}
> -			if (signal_pending(current)) {
> -				res = -ERESTARTSYS;
> -				break;
> +	if (ctx->flags & EFD_MASK) {
> +		struct efd_mask mask;
> +		if (count < sizeof(mask))
> +			return -EINVAL;
> +		if (copy_from_user(&mask, buf, sizeof(mask)))
> +			return -EFAULT;
> +		if (mask.events & ~EFD_MASK_VALID_EVENTS)
> +			return -EINVAL;
> +		spin_lock_irq(&ctx->wqh.lock);
> +		memcpy(&ctx->mask, &mask, sizeof(ctx->mask));
> +		if (waitqueue_active(&ctx->wqh))
> +			wake_up_locked_poll(&ctx->wqh,
> +				(unsigned long)ctx->mask.events);
> +		spin_unlock_irq(&ctx->wqh.lock);
> +		return sizeof(ctx->mask);
> +	} else {
> +		ssize_t res;
> +		__u64 ucnt;
> +		DECLARE_WAITQUEUE(wait, current);
> +		if (count < sizeof(ucnt))
> +			return -EINVAL;
> +		if (copy_from_user(&ucnt, buf, sizeof(ucnt)))
> +			return -EFAULT;
> +		if (ucnt == ULLONG_MAX)
> +			return -EINVAL;
> +		spin_lock_irq(&ctx->wqh.lock);
> +		res = -EAGAIN;
> +		if (ULLONG_MAX - ctx->count > ucnt)
> +			res = sizeof(ucnt);
> +		else if (!(file->f_flags & O_NONBLOCK)) {
> +			__add_wait_queue(&ctx->wqh, &wait);
> +			for (res = 0;;) {
> +				set_current_state(TASK_INTERRUPTIBLE);
> +				if (ULLONG_MAX - ctx->count > ucnt) {
> +					res = sizeof(ucnt);
> +					break;
> +				}
> +				if (signal_pending(current)) {
> +					res = -ERESTARTSYS;
> +					break;
> +				}
> +				spin_unlock_irq(&ctx->wqh.lock);
> +				schedule();
> +				spin_lock_irq(&ctx->wqh.lock);
>  			}
> -			spin_unlock_irq(&ctx->wqh.lock);
> -			schedule();
> -			spin_lock_irq(&ctx->wqh.lock);
> +			__remove_wait_queue(&ctx->wqh, &wait);
> +			__set_current_state(TASK_RUNNING);
>  		}
> -		__remove_wait_queue(&ctx->wqh, &wait);
> -		__set_current_state(TASK_RUNNING);
> -	}
> -	if (likely(res > 0)) {
> -		ctx->count += ucnt;
> -		if (waitqueue_active(&ctx->wqh))
> -			wake_up_locked_poll(&ctx->wqh, POLLIN);
> -	}
> -	spin_unlock_irq(&ctx->wqh.lock);
> +		if (likely(res > 0)) {
> +			ctx->count += ucnt;
> +			if (waitqueue_active(&ctx->wqh))
> +				wake_up_locked_poll(&ctx->wqh, POLLIN);
> +		}
> +		spin_unlock_irq(&ctx->wqh.lock);
>  
> -	return res;
> +		return res;
> +	}
>  }
>  
>  #ifdef CONFIG_PROC_FS
> @@ -293,8 +349,13 @@ static int eventfd_show_fdinfo(struct seq_file *m, struct file *f)
>  	int ret;
>  
>  	spin_lock_irq(&ctx->wqh.lock);
> -	ret = seq_printf(m, "eventfd-count: %16llx\n",
> -			 (unsigned long long)ctx->count);
> +	if (ctx->flags & EFD_MASK) {
> +		ret = seq_printf(m, "eventfd-mask: %x\n",
> +				 (unsigned)ctx->mask.events);
> +	} else {
> +		ret = seq_printf(m, "eventfd-count: %16llx\n",
> +				 (unsigned long long)ctx->count);
> +	}
>  	spin_unlock_irq(&ctx->wqh.lock);
>  
>  	return ret;
> @@ -412,7 +473,12 @@ struct file *eventfd_file_create(unsigned int count, int flags)
>  
>  	kref_init(&ctx->kref);
>  	init_waitqueue_head(&ctx->wqh);
> -	ctx->count = count;
> +	if (flags & EFD_MASK) {
> +		ctx->mask.events = 0;
> +		ctx->mask.data = 0;
> +	} else {
> +		ctx->count = count;
> +	}
>  	ctx->flags = flags;
>  
>  	file = anon_inode_getfile("[eventfd]", &eventfd_fops, ctx,
> diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h
> index 3c3ef19..218aba6 100644
> --- a/include/linux/eventfd.h
> +++ b/include/linux/eventfd.h
> @@ -8,24 +8,11 @@
>  #ifndef _LINUX_EVENTFD_H
>  #define _LINUX_EVENTFD_H
>  
> -#include <linux/fcntl.h>
> +#include <uapi/linux/eventfd.h>
> +
>  #include <linux/file.h>
>  #include <linux/wait.h>
>  
> -/*
> - * CAREFUL: Check include/asm-generic/fcntl.h when defining
> - * new flags, since they might collide with O_* ones. We want
> - * to re-use O_* flags that couldn't possibly have a meaning
> - * from eventfd, in order to leave a free define-space for
> - * shared O_* flags.
> - */
> -#define EFD_SEMAPHORE (1 << 0)
> -#define EFD_CLOEXEC O_CLOEXEC
> -#define EFD_NONBLOCK O_NONBLOCK
> -
> -#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
> -#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
> -
>  #ifdef CONFIG_EVENTFD
>  
>  struct file *eventfd_file_create(unsigned int count, int flags);
> diff --git a/include/uapi/linux/eventfd.h b/include/uapi/linux/eventfd.h
> new file mode 100644
> index 0000000..03057a5
> --- /dev/null
> +++ b/include/uapi/linux/eventfd.h
> @@ -0,0 +1,40 @@
> +/*
> + *  Copyright (C) 2013 Martin Sustrik <sustrik@250bpm.com>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + */
> +
> +#ifndef _UAPI_LINUX_EVENTFD_H
> +#define _UAPI_LINUX_EVENTFD_H
> +
> +/* For O_CLOEXEC */
> +#include <linux/fcntl.h>
> +#include <linux/types.h>
> +
> +/*
> + * CAREFUL: Check include/asm-generic/fcntl.h when defining
> + * new flags, since they might collide with O_* ones. We want
> + * to re-use O_* flags that couldn't possibly have a meaning
> + * from eventfd, in order to leave a free define-space for
> + * shared O_* flags.
> + */
> +
> +/* Provide semaphore-like semantics for reads from the eventfd. */
> +#define EFD_SEMAPHORE (1 << 0)
> +/* Provide event mask semantics for the eventfd. */
> +#define EFD_MASK (1 << 1)
> +/*  Set the close-on-exec (FD_CLOEXEC) flag on the eventfd. */
> +#define EFD_CLOEXEC O_CLOEXEC
> +/*  Create the eventfd in non-blocking mode. */
> +#define EFD_NONBLOCK O_NONBLOCK
> +
> +/* Structure to read/write to eventfd in EFD_MASK mode. */
> +struct efd_mask {
> +	__u32 events;
> +	__u64 data;
> +} __packed;
> +
> +#endif /* _UAPI_LINUX_EVENTFD_H */
> 

[PATCH v3 1/1] eventfd: implementation of EFD_MASK flag

[Martin Sustrik]

When implementing network protocols in user space, one has to implement
fake file descriptors to represent the sockets for the protocol.

Polling on such fake file descriptors is a problem (poll/select/epoll accept
only true file descriptors) and forces protocol implementers to use various
workarounds resulting in complex, non-standard and convoluted APIs.

More generally, ability to create full-blown file descriptors for
userspace-to-userspace signalling is missing. While eventfd(2) goes half the way
towards this goal it has follwoing shorcomings:

I.  There's no way to signal POLLPRI, POLLHUP etc.
II. There's no way to signal arbitrary combination of POLL* flags. Most notably,
    simultaneous !POLLIN and !POLLOUT, which is a perfectly valid combination
    for a network protocol (rx buffer is empty and tx buffer is full), cannot be
    signaled using eventfd.

This patch implements new EFD_MASK flag which solves the above problems.

Additionally, to provide a way to associate user-space state with eventfd
object, it allows to attach user-space data to the file descriptor.

The semantics of EFD_MASK are as follows:

eventfd(2):

If eventfd is created with EFD_MASK flag set, it is initialised in such a way
as to signal no events on the file descriptor when it is polled on. 'initval'
argument is ignored.

write(2):

User is allowed to write only buffers containing the following structure:

struct efd_mask {
  short events;
  union {
    void    *ptr;
    uint32_t u32;
    uint64_t u64;
  };
};

The value of 'events' should be any combination of event flags as defined by
poll(2) function (POLLIN, POLLOUT, POLLERR, POLLHUP etc.) Specified events will
be signaled when polling (select, poll, epoll) on the eventfd is done later on.
'ptr', 'u32' or 'u64' are opaque data that are not interpreted by eventfd
object.

read(2):

User is allowed to read an efd_mask structure from the eventfd marked by
EFD_MASK. Returned value shall be the last one written to the eventfd.

select(2), poll(2) and similar:

When polling on the eventfd marked by EFD_MASK flag, all the events specified
in last written 'events' field shall be signaled.

Signed-off-by: Martin Sustrik <sustrik@250bpm.com>
---
Following changes were made to the patch since v2:

- eventfd_mask structure renamed to efd_mask to keep user-space prefixes
  consistent
- efd_mask is __packed for all architectures
- eventfd.h header file added to uapi; given there wasn't one so far, in
  addition to moving efd_mask there, I've moved also all the eventfd flags that
  are meant to be visible from the user space
- comment for 'mask' member eventfd_ctx added 
- synchronisation bugs in eventfd_read fixed
- several variable declarations moved from the beginning of the function to
  the blocks where they are used
- changelog text made simpler and more up to the point

There was also a request to explain why this functionality is needed. I've
written an article explaining the problems user-space network protocol
implementers face here: http://www.250bpm.com/blog:16
---
 fs/eventfd.c                 |  194 ++++++++++++++++++++++++++++--------------
 include/linux/eventfd.h      |   17 +---
 include/uapi/linux/eventfd.h |   40 +++++++++
 3 files changed, 172 insertions(+), 79 deletions(-)
 create mode 100644 include/uapi/linux/eventfd.h

diff --git a/fs/eventfd.c b/fs/eventfd.c
index 35470d9..8f821b1 100644
--- a/fs/eventfd.c
+++ b/fs/eventfd.c
@@ -2,6 +2,7 @@
  *  fs/eventfd.c
  *
  *  Copyright (C) 2007  Davide Libenzi <davidel@xmailserver.org>
+ *  Copyright (C) 2013  Martin Sustrik <sustrik@250bpm.com>
  *
  */
 
@@ -22,18 +23,31 @@
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
 
+#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
+#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE | EFD_MASK)
+#define EFD_MASK_VALID_EVENTS (POLLIN | POLLPRI | POLLOUT | POLLERR | POLLHUP)
+
 struct eventfd_ctx {
 	struct kref kref;
 	wait_queue_head_t wqh;
-	/*
-	 * Every time that a write(2) is performed on an eventfd, the
-	 * value of the __u64 being written is added to "count" and a
-	 * wakeup is performed on "wqh". A read(2) will return the "count"
-	 * value to userspace, and will reset "count" to zero. The kernel
-	 * side eventfd_signal() also, adds to the "count" counter and
-	 * issue a wakeup.
-	 */
-	__u64 count;
+	union {
+		/*
+		 * Every time that a write(2) is performed on an eventfd, the
+		 * value of the __u64 being written is added to "count" and a
+		 * wakeup is performed on "wqh". A read(2) will return the
+		 * "count" value to userspace, and will reset "count" to zero.
+		 * The kernel side eventfd_signal() also, adds to the "count"
+		 * counter and issue a wakeup.
+		 */
+		__u64 count;
+
+		/*
+		 * When using eventfd in EFD_MASK mode this stracture stores the
+		 * current events to be signaled on the eventfd (events member)
+		 * along with opaque user-defined data (data member).
+		 */
+		struct efd_mask mask;
+	};
 	unsigned int flags;
 };
 
@@ -55,6 +69,9 @@ __u64 eventfd_signal(struct eventfd_ctx *ctx, __u64 n)
 {
 	unsigned long flags;
 
+	/* This function should never be used with eventfd in the mask mode. */
+	BUG_ON(ctx->flags & EFD_MASK);
+
 	spin_lock_irqsave(&ctx->wqh.lock, flags);
 	if (ULLONG_MAX - ctx->count < n)
 		n = ULLONG_MAX - ctx->count;
@@ -123,12 +140,16 @@ static unsigned int eventfd_poll(struct file *file, poll_table *wait)
 	poll_wait(file, &ctx->wqh, wait);
 
 	spin_lock_irqsave(&ctx->wqh.lock, flags);
-	if (ctx->count > 0)
-		events |= POLLIN;
-	if (ctx->count == ULLONG_MAX)
-		events |= POLLERR;
-	if (ULLONG_MAX - 1 > ctx->count)
-		events |= POLLOUT;
+	if (ctx->flags & EFD_MASK) {
+		events = ctx->mask.events;
+	} else {
+		if (ctx->count > 0)
+			events |= POLLIN;
+		if (ctx->count == ULLONG_MAX)
+			events |= POLLERR;
+		if (ULLONG_MAX - 1 > ctx->count)
+			events |= POLLOUT;
+	}
 	spin_unlock_irqrestore(&ctx->wqh.lock, flags);
 
 	return events;
@@ -158,6 +179,9 @@ int eventfd_ctx_remove_wait_queue(struct eventfd_ctx *ctx, wait_queue_t *wait,
 {
 	unsigned long flags;
 
+	/* This function should never be used with eventfd in the mask mode. */
+	BUG_ON(ctx->flags & EFD_MASK);
+
 	spin_lock_irqsave(&ctx->wqh.lock, flags);
 	eventfd_ctx_do_read(ctx, cnt);
 	__remove_wait_queue(&ctx->wqh, wait);
@@ -188,6 +212,9 @@ ssize_t eventfd_ctx_read(struct eventfd_ctx *ctx, int no_wait, __u64 *cnt)
 	ssize_t res;
 	DECLARE_WAITQUEUE(wait, current);
 
+	/* This function should never be used with eventfd in the mask mode. */
+	BUG_ON(ctx->flags & EFD_MASK);
+
 	spin_lock_irq(&ctx->wqh.lock);
 	*cnt = 0;
 	res = -EAGAIN;
@@ -227,63 +254,92 @@ static ssize_t eventfd_read(struct file *file, char __user *buf, size_t count,
 			    loff_t *ppos)
 {
 	struct eventfd_ctx *ctx = file->private_data;
-	ssize_t res;
-	__u64 cnt;
 
-	if (count < sizeof(cnt))
-		return -EINVAL;
-	res = eventfd_ctx_read(ctx, file->f_flags & O_NONBLOCK, &cnt);
-	if (res < 0)
-		return res;
-
-	return put_user(cnt, (__u64 __user *) buf) ? -EFAULT : sizeof(cnt);
+	if (ctx->flags & EFD_MASK) {
+		struct efd_mask mask;
+		if (count < sizeof(mask))
+			return -EINVAL;
+		spin_lock_irq(&ctx->wqh.lock);
+		mask = ctx->mask;
+		spin_unlock_irq(&ctx->wqh.lock);
+		if (copy_to_user(buf, &mask, sizeof(mask)))
+			return -EFAULT;
+		return sizeof(mask);
+	} else {
+		ssize_t res;
+		__u64 cnt;
+		if (count < sizeof(cnt))
+			return -EINVAL;
+		res = eventfd_ctx_read(ctx, file->f_flags & O_NONBLOCK, &cnt);
+		if (res < 0)
+			return res;
+		return put_user(cnt, (__u64 __user *) buf) ?
+			-EFAULT : sizeof(cnt);
+	}
 }
 
 static ssize_t eventfd_write(struct file *file, const char __user *buf, size_t count,
 			     loff_t *ppos)
 {
 	struct eventfd_ctx *ctx = file->private_data;
-	ssize_t res;
-	__u64 ucnt;
-	DECLARE_WAITQUEUE(wait, current);
 
-	if (count < sizeof(ucnt))
-		return -EINVAL;
-	if (copy_from_user(&ucnt, buf, sizeof(ucnt)))
-		return -EFAULT;
-	if (ucnt == ULLONG_MAX)
-		return -EINVAL;
-	spin_lock_irq(&ctx->wqh.lock);
-	res = -EAGAIN;
-	if (ULLONG_MAX - ctx->count > ucnt)
-		res = sizeof(ucnt);
-	else if (!(file->f_flags & O_NONBLOCK)) {
-		__add_wait_queue(&ctx->wqh, &wait);
-		for (res = 0;;) {
-			set_current_state(TASK_INTERRUPTIBLE);
-			if (ULLONG_MAX - ctx->count > ucnt) {
-				res = sizeof(ucnt);
-				break;
-			}
-			if (signal_pending(current)) {
-				res = -ERESTARTSYS;
-				break;
+	if (ctx->flags & EFD_MASK) {
+		struct efd_mask mask;
+		if (count < sizeof(mask))
+			return -EINVAL;
+		if (copy_from_user(&mask, buf, sizeof(mask)))
+			return -EFAULT;
+		if (mask.events & ~EFD_MASK_VALID_EVENTS)
+			return -EINVAL;
+		spin_lock_irq(&ctx->wqh.lock);
+		memcpy(&ctx->mask, &mask, sizeof(ctx->mask));
+		if (waitqueue_active(&ctx->wqh))
+			wake_up_locked_poll(&ctx->wqh,
+				(unsigned long)ctx->mask.events);
+		spin_unlock_irq(&ctx->wqh.lock);
+		return sizeof(ctx->mask);
+	} else {
+		ssize_t res;
+		__u64 ucnt;
+		DECLARE_WAITQUEUE(wait, current);
+		if (count < sizeof(ucnt))
+			return -EINVAL;
+		if (copy_from_user(&ucnt, buf, sizeof(ucnt)))
+			return -EFAULT;
+		if (ucnt == ULLONG_MAX)
+			return -EINVAL;
+		spin_lock_irq(&ctx->wqh.lock);
+		res = -EAGAIN;
+		if (ULLONG_MAX - ctx->count > ucnt)
+			res = sizeof(ucnt);
+		else if (!(file->f_flags & O_NONBLOCK)) {
+			__add_wait_queue(&ctx->wqh, &wait);
+			for (res = 0;;) {
+				set_current_state(TASK_INTERRUPTIBLE);
+				if (ULLONG_MAX - ctx->count > ucnt) {
+					res = sizeof(ucnt);
+					break;
+				}
+				if (signal_pending(current)) {
+					res = -ERESTARTSYS;
+					break;
+				}
+				spin_unlock_irq(&ctx->wqh.lock);
+				schedule();
+				spin_lock_irq(&ctx->wqh.lock);
 			}
-			spin_unlock_irq(&ctx->wqh.lock);
-			schedule();
-			spin_lock_irq(&ctx->wqh.lock);
+			__remove_wait_queue(&ctx->wqh, &wait);
+			__set_current_state(TASK_RUNNING);
 		}
-		__remove_wait_queue(&ctx->wqh, &wait);
-		__set_current_state(TASK_RUNNING);
-	}
-	if (likely(res > 0)) {
-		ctx->count += ucnt;
-		if (waitqueue_active(&ctx->wqh))
-			wake_up_locked_poll(&ctx->wqh, POLLIN);
-	}
-	spin_unlock_irq(&ctx->wqh.lock);
+		if (likely(res > 0)) {
+			ctx->count += ucnt;
+			if (waitqueue_active(&ctx->wqh))
+				wake_up_locked_poll(&ctx->wqh, POLLIN);
+		}
+		spin_unlock_irq(&ctx->wqh.lock);
 
-	return res;
+		return res;
+	}
 }
 
 #ifdef CONFIG_PROC_FS
@@ -293,8 +349,13 @@ static int eventfd_show_fdinfo(struct seq_file *m, struct file *f)
 	int ret;
 
 	spin_lock_irq(&ctx->wqh.lock);
-	ret = seq_printf(m, "eventfd-count: %16llx\n",
-			 (unsigned long long)ctx->count);
+	if (ctx->flags & EFD_MASK) {
+		ret = seq_printf(m, "eventfd-mask: %x\n",
+				 (unsigned)ctx->mask.events);
+	} else {
+		ret = seq_printf(m, "eventfd-count: %16llx\n",
+				 (unsigned long long)ctx->count);
+	}
 	spin_unlock_irq(&ctx->wqh.lock);
 
 	return ret;
@@ -412,7 +473,12 @@ struct file *eventfd_file_create(unsigned int count, int flags)
 
 	kref_init(&ctx->kref);
 	init_waitqueue_head(&ctx->wqh);
-	ctx->count = count;
+	if (flags & EFD_MASK) {
+		ctx->mask.events = 0;
+		ctx->mask.data = 0;
+	} else {
+		ctx->count = count;
+	}
 	ctx->flags = flags;
 
 	file = anon_inode_getfile("[eventfd]", &eventfd_fops, ctx,
diff --git a/include/linux/eventfd.h b/include/linux/eventfd.h
index 3c3ef19..218aba6 100644
--- a/include/linux/eventfd.h
+++ b/include/linux/eventfd.h
@@ -8,24 +8,11 @@
 #ifndef _LINUX_EVENTFD_H
 #define _LINUX_EVENTFD_H
 
-#include <linux/fcntl.h>
+#include <uapi/linux/eventfd.h>
+
 #include <linux/file.h>
 #include <linux/wait.h>
 
-/*
- * CAREFUL: Check include/asm-generic/fcntl.h when defining
- * new flags, since they might collide with O_* ones. We want
- * to re-use O_* flags that couldn't possibly have a meaning
- * from eventfd, in order to leave a free define-space for
- * shared O_* flags.
- */
-#define EFD_SEMAPHORE (1 << 0)
-#define EFD_CLOEXEC O_CLOEXEC
-#define EFD_NONBLOCK O_NONBLOCK
-
-#define EFD_SHARED_FCNTL_FLAGS (O_CLOEXEC | O_NONBLOCK)
-#define EFD_FLAGS_SET (EFD_SHARED_FCNTL_FLAGS | EFD_SEMAPHORE)
-
 #ifdef CONFIG_EVENTFD
 
 struct file *eventfd_file_create(unsigned int count, int flags);
diff --git a/include/uapi/linux/eventfd.h b/include/uapi/linux/eventfd.h
new file mode 100644
index 0000000..03057a5
--- /dev/null
+++ b/include/uapi/linux/eventfd.h
@@ -0,0 +1,40 @@
+/*
+ *  Copyright (C) 2013 Martin Sustrik <sustrik@250bpm.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ */
+
+#ifndef _UAPI_LINUX_EVENTFD_H
+#define _UAPI_LINUX_EVENTFD_H
+
+/* For O_CLOEXEC */
+#include <linux/fcntl.h>
+#include <linux/types.h>
+
+/*
+ * CAREFUL: Check include/asm-generic/fcntl.h when defining
+ * new flags, since they might collide with O_* ones. We want
+ * to re-use O_* flags that couldn't possibly have a meaning
+ * from eventfd, in order to leave a free define-space for
+ * shared O_* flags.
+ */
+
+/* Provide semaphore-like semantics for reads from the eventfd. */
+#define EFD_SEMAPHORE (1 << 0)
+/* Provide event mask semantics for the eventfd. */
+#define EFD_MASK (1 << 1)
+/*  Set the close-on-exec (FD_CLOEXEC) flag on the eventfd. */
+#define EFD_CLOEXEC O_CLOEXEC
+/*  Create the eventfd in non-blocking mode. */
+#define EFD_NONBLOCK O_NONBLOCK
+
+/* Structure to read/write to eventfd in EFD_MASK mode. */
+struct efd_mask {
+	__u32 events;
+	__u64 data;
+} __packed;
+
+#endif /* _UAPI_LINUX_EVENTFD_H */
-- 
1.7.4.1