Skip to contentSkip to conversation

Add io_uring support for epoll_ctl

PATCH Superseded Jens Axboe 2 participants 6 messages Jan 22, 2020

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Jens Axboe

author maintainer
#0 lore raw

cover Add io_uring support for epoll_ctl

This adds support for epoll manipulation through io_uring, in particular epoll_ctl(2). Patch 1 is just a prep patch, patch 2 adds non-block support for epoll, and patch 3 wires it up for io_uring.

Patch 2 isn't the prettiest thing in the world, but we need to do nonblock grabbing of the mutexes and be able to back out safely.

Please review, thanks.

Signature · 1 line
Jens Axboe
author maintainer
#1 lore raw

PATCH 1/3 eventpoll: abstract out epoll_ctl() handler

No functional changes in this patch.

---
 fs/eventpoll.c | 45 +++++++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 20 deletions(-)
fs/eventpoll.c +25 -20
index 67a395039268..cd848e8d08e2 100644
@@ -2074,27 +2074,15 @@ SYSCALL_DEFINE1(epoll_create, int, size)
 	return do_epoll_create(0);
 }
 
-/*
- * The following function implements the controller interface for
- * the eventpoll file that enables the insertion/removal/change of
- * file descriptors inside the interest set.
- */
-SYSCALL_DEFINE4(epoll_ctl, int, epfd, int, op, int, fd,
-		struct epoll_event __user *, event)
+static int do_epoll_ctl(int epfd, int op, int fd, struct epoll_event *epds)
 {
 	int error;
 	int full_check = 0;
 	struct fd f, tf;
 	struct eventpoll *ep;
 	struct epitem *epi;
-	struct epoll_event epds;
 	struct eventpoll *tep = NULL;
 
-	error = -EFAULT;
-	if (ep_op_has_event(op) &&
-	    copy_from_user(&epds, event, sizeof(struct epoll_event)))
-		goto error_return;
-
 	error = -EBADF;
 	f = fdget(epfd);
 	if (!f.file)
@@ -2112,7 +2100,7 @@ SYSCALL_DEFINE4(epoll_ctl, int, epfd, int, op, int, fd,
 
 	/* Check if EPOLLWAKEUP is allowed */
 	if (ep_op_has_event(op))
-		ep_take_care_of_epollwakeup(&epds);
+		ep_take_care_of_epollwakeup(epds);
 
 	/*
 	 * We have to check that the file structure underneath the file descriptor
@@ -2128,11 +2116,11 @@ SYSCALL_DEFINE4(epoll_ctl, int, epfd, int, op, int, fd,
 	 * so EPOLLEXCLUSIVE is not allowed for a EPOLL_CTL_MOD operation.
 	 * Also, we do not currently supported nested exclusive wakeups.
 	 */
-	if (ep_op_has_event(op) && (epds.events & EPOLLEXCLUSIVE)) {
+	if (ep_op_has_event(op) && (epds->events & EPOLLEXCLUSIVE)) {
 		if (op == EPOLL_CTL_MOD)
 			goto error_tgt_fput;
 		if (op == EPOLL_CTL_ADD && (is_file_epoll(tf.file) ||
-				(epds.events & ~EPOLLEXCLUSIVE_OK_BITS)))
+				(epds->events & ~EPOLLEXCLUSIVE_OK_BITS)))
 			goto error_tgt_fput;
 	}
 
@@ -2192,8 +2180,8 @@ SYSCALL_DEFINE4(epoll_ctl, int, epfd, int, op, int, fd,
 	switch (op) {
 	case EPOLL_CTL_ADD:
 		if (!epi) {
-			epds.events |= EPOLLERR | EPOLLHUP;
-			error = ep_insert(ep, &epds, tf.file, fd, full_check);
+			epds->events |= EPOLLERR | EPOLLHUP;
+			error = ep_insert(ep, epds, tf.file, fd, full_check);
 		} else
 			error = -EEXIST;
 		if (full_check)
@@ -2208,8 +2196,8 @@ SYSCALL_DEFINE4(epoll_ctl, int, epfd, int, op, int, fd,
 	case EPOLL_CTL_MOD:
 		if (epi) {
 			if (!(epi->event.events & EPOLLEXCLUSIVE)) {
-				epds.events |= EPOLLERR | EPOLLHUP;
-				error = ep_modify(ep, epi, &epds);
+				epds->events |= EPOLLERR | EPOLLHUP;
+				error = ep_modify(ep, epi, epds);
 			}
 		} else
 			error = -ENOENT;
@@ -2231,6 +2219,23 @@ SYSCALL_DEFINE4(epoll_ctl, int, epfd, int, op, int, fd,
 	return error;
 }
 
+/*
+ * The following function implements the controller interface for
+ * the eventpoll file that enables the insertion/removal/change of
+ * file descriptors inside the interest set.
+ */
+SYSCALL_DEFINE4(epoll_ctl, int, epfd, int, op, int, fd,
+		struct epoll_event __user *, event)
+{
+	struct epoll_event epds;
+
+	if (ep_op_has_event(op) &&
+	    copy_from_user(&epds, event, sizeof(struct epoll_event)))
+		return -EFAULT;
+
+	return do_epoll_ctl(epfd, op, fd, &epds);
+}
+
 /*
  * Implement the event wait interface for the eventpoll file. It is the kernel
  * part of the user space epoll_wait(2).
Signature · 1 line
2.25.0
author maintainer
#2 lore raw

PATCH 2/3 eventpoll: support non-blocking do_epoll_ctl() calls

In reply to Jens Axboe

Also make it available outside of epoll, along with the helper that decides if we need to copy the passed in epoll_event.

---
 fs/eventpoll.c            | 42 ++++++++++++++++++++++++++++-----------
 include/linux/eventpoll.h |  9 +++++++++
 2 files changed, 39 insertions(+), 12 deletions(-)
fs/eventpoll.c +30 -12
index cd848e8d08e2..162af749ea50 100644
@@ -354,12 +354,6 @@ static inline struct epitem *ep_item_from_epqueue(poll_table *p)
 	return container_of(p, struct ep_pqueue, pt)->epi;
 }
 
-/* Tells if the epoll_ctl(2) operation needs an event copy from userspace */
-static inline int ep_op_has_event(int op)
-{
-	return op != EPOLL_CTL_DEL;
-}
-
 /* Initialize the poll safe wake up structure */
 static void ep_nested_calls_init(struct nested_calls *ncalls)
 {
@@ -2074,7 +2068,20 @@ SYSCALL_DEFINE1(epoll_create, int, size)
 	return do_epoll_create(0);
 }
 
-static int do_epoll_ctl(int epfd, int op, int fd, struct epoll_event *epds)
+static inline int epoll_mutex_lock(struct mutex *mutex, int depth,
+				   bool nonblock)
+{
+	if (!nonblock) {
+		mutex_lock_nested(mutex, depth);
+		return 0;
+	}
+	if (!mutex_trylock(mutex))
+		return 0;
+	return -EAGAIN;
+}
+
+int do_epoll_ctl(int epfd, int op, int fd, struct epoll_event *epds,
+		 bool nonblock)
 {
 	int error;
 	int full_check = 0;
@@ -2145,13 +2152,17 @@ static int do_epoll_ctl(int epfd, int op, int fd, struct epoll_event *epds)
 	 * deep wakeup paths from forming in parallel through multiple
 	 * EPOLL_CTL_ADD operations.
 	 */
-	mutex_lock_nested(&ep->mtx, 0);
+	error = epoll_mutex_lock(&ep->mtx, 0, nonblock);
+	if (error)
+		goto error_tgt_fput;
 	if (op == EPOLL_CTL_ADD) {
 		if (!list_empty(&f.file->f_ep_links) ||
 						is_file_epoll(tf.file)) {
 			full_check = 1;
 			mutex_unlock(&ep->mtx);
-			mutex_lock(&epmutex);
+			error = epoll_mutex_lock(&epmutex, 0, nonblock);
+			if (error)
+				goto error_tgt_fput;
 			if (is_file_epoll(tf.file)) {
 				error = -ELOOP;
 				if (ep_loop_check(ep, tf.file) != 0) {
@@ -2161,10 +2172,17 @@ static int do_epoll_ctl(int epfd, int op, int fd, struct epoll_event *epds)
 			} else
 				list_add(&tf.file->f_tfile_llink,
 							&tfile_check_list);
-			mutex_lock_nested(&ep->mtx, 0);
+			error = epoll_mutex_lock(&ep->mtx, 0, nonblock);
+			if (error) {
+out_del:
+				list_del(&tf.file->f_tfile_llink);
+				goto error_tgt_fput;
+			}
 			if (is_file_epoll(tf.file)) {
 				tep = tf.file->private_data;
-				mutex_lock_nested(&tep->mtx, 1);
+				error = epoll_mutex_lock(&tep->mtx, 1, nonblock);
+				if (error)
+					goto out_del;
 			}
 		}
 	}
@@ -2233,7 +2251,7 @@ SYSCALL_DEFINE4(epoll_ctl, int, epfd, int, op, int, fd,
 	    copy_from_user(&epds, event, sizeof(struct epoll_event)))
 		return -EFAULT;
 
-	return do_epoll_ctl(epfd, op, fd, &epds);
+	return do_epoll_ctl(epfd, op, fd, &epds, false);
 }
 
 /*
include/linux/eventpoll.h +9 -0
index bc6d79b00c4e..8f000fada5a4 100644
@@ -61,6 +61,15 @@ static inline void eventpoll_release(struct file *file)
 	eventpoll_release_file(file);
 }
 
+int do_epoll_ctl(int epfd, int op, int fd, struct epoll_event *epds,
+		 bool nonblock);
+
+/* Tells if the epoll_ctl(2) operation needs an event copy from userspace */
+static inline int ep_op_has_event(int op)
+{
+	return op != EPOLL_CTL_DEL;
+}
+
 #else
 
 static inline void eventpoll_init_file(struct file *file) {}
Signature · 1 line
2.25.0
#3 lore raw

Re: [PATCH 2/3] eventpoll: support non-blocking do_epoll_ctl() calls

Quoted 2 lines · Jens Axboe

Also make it available outside of epoll, along with the helper that decides if we need to copy the passed in epoll_event.

[...]

Quoted 4 lines
fs/eventpoll.c +0 -0
index cd848e8d08e2..162af749ea50 100644

[...]

fs/eventpoll.c SYSCALL_DEFINE1(epoll_create, int, size) · line 2080

Quoted 11 lines
fs/eventpoll.c +10 -1
-static int do_epoll_ctl(int epfd, int op, int fd, struct epoll_event *epds)
+static inline int epoll_mutex_lock(struct mutex *mutex, int depth,
+                                  bool nonblock)
+{
+       if (!nonblock) {
+               mutex_lock_nested(mutex, depth);
+               return 0;
+       }
+       if (!mutex_trylock(mutex))
+               return 0;
+       return -EAGAIN;

The documentation for mutex_trylock() says:

* Try to acquire the mutex atomically. Returns 1 if the mutex
* has been acquired successfully, and 0 on contention.

So in the success case, this evaluates to:

    if (!1)
      return 0;
    return -EAGAIN;

which is

    if (0)
      return 0;
    return -EAGAIN;

which is

    return -EAGAIN;

I think you'll have to get rid of the negation.

Quoted 14 lines
fs/eventpoll.c +6 -1
+}
+
+int do_epoll_ctl(int epfd, int op, int fd, struct epoll_event *epds,
+                bool nonblock)
 {
        int error;
        int full_check = 0;
@@ -2145,13 +2152,17 @@ static int do_epoll_ctl(int epfd, int op, int fd, struct epoll_event *epds)
         * deep wakeup paths from forming in parallel through multiple
         * EPOLL_CTL_ADD operations.
         */
-       mutex_lock_nested(&ep->mtx, 0);
+       error = epoll_mutex_lock(&ep->mtx, 0, nonblock);
+       if (error)

fs/eventpoll.c static int do_epoll_ctl(int epfd, int op, int fd, struct epoll_event *epds) · line 2165

Quoted 10 lines
fs/eventpoll.c +4 -1
+               goto error_tgt_fput;
        if (op == EPOLL_CTL_ADD) {
                if (!list_empty(&f.file->f_ep_links) ||
                                                is_file_epoll(tf.file)) {
                        full_check = 1;
                        mutex_unlock(&ep->mtx);
-                       mutex_lock(&epmutex);
+                       error = epoll_mutex_lock(&epmutex, 0, nonblock);
+                       if (error)
+                               goto error_tgt_fput;

When we reach the "goto", full_check==1 and epmutex is not held. But at the jump target, this code runs:

error_tgt_fput:
if (full_check) // true

    mutex_unlock(&epmutex);

So I think we're releasing a lock that we don't hold.

Quoted 10 lines
fs/eventpoll.c +2 -1
                        if (is_file_epoll(tf.file)) {
                                error = -ELOOP;
                                if (ep_loop_check(ep, tf.file) != 0) {
@@ -2161,10 +2172,17 @@ static int do_epoll_ctl(int epfd, int op, int fd, struct epoll_event *epds)
                        } else
                                list_add(&tf.file->f_tfile_llink,
                                                        &tfile_check_list);
-                       mutex_lock_nested(&ep->mtx, 0);
+                       error = epoll_mutex_lock(&ep->mtx, 0, nonblock);
+                       if (error) {

fs/eventpoll.c static int do_epoll_ctl(int epfd, int op, int fd, struct epoll_event *epds) · line 2185

Quoted 10 lines
fs/eventpoll.c +7 -1
+out_del:
+                               list_del(&tf.file->f_tfile_llink);
+                               goto error_tgt_fput;
+                       }
                        if (is_file_epoll(tf.file)) {
                                tep = tf.file->private_data;
-                               mutex_lock_nested(&tep->mtx, 1);
+                               error = epoll_mutex_lock(&tep->mtx, 1, nonblock);
+                               if (error)
+                                       goto out_del;

When we reach this "goto", ep->mtx is held and never dropped.

Quoted 10 lines
fs/eventpoll.c +1 -1
                        }
                }
        }
@@ -2233,7 +2251,7 @@ SYSCALL_DEFINE4(epoll_ctl, int, epfd, int, op, int, fd,
            copy_from_user(&epds, event, sizeof(struct epoll_event)))
                return -EFAULT;
 
-       return do_epoll_ctl(epfd, op, fd, &epds);
+       return do_epoll_ctl(epfd, op, fd, &epds, false);
 }
author maintainer
#4 lore raw

Re: [PATCH 2/3] eventpoll: support non-blocking do_epoll_ctl() calls

Quoted 38 lines · Jann Horn
Quoted 2 lines · Jens Axboe

Also make it available outside of epoll, along with the helper that decides if we need to copy the passed in epoll_event.

[...]

Quoted 4 lines
fs/eventpoll.c +0 -0
index cd848e8d08e2..162af749ea50 100644

[...]

fs/eventpoll.c SYSCALL_DEFINE1(epoll_create, int, size) · line 2080

Quoted 11 lines
fs/eventpoll.c +10 -1
-static int do_epoll_ctl(int epfd, int op, int fd, struct epoll_event *epds)
+static inline int epoll_mutex_lock(struct mutex *mutex, int depth,
+                                  bool nonblock)
+{
+       if (!nonblock) {
+               mutex_lock_nested(mutex, depth);
+               return 0;
+       }
+       if (!mutex_trylock(mutex))
+               return 0;
+       return -EAGAIN;

The documentation for mutex_trylock() says:

* Try to acquire the mutex atomically. Returns 1 if the mutex
* has been acquired successfully, and 0 on contention.

So in the success case, this evaluates to:

    if (!1)
      return 0;
    return -EAGAIN;

which is

    if (0)
      return 0;
    return -EAGAIN;
Quoted 5 lines

which is

    return -EAGAIN;

I think you'll have to get rid of the negation.

Doh indeed. I'll rework and run the test case, just rebased this and I think I inadvertently used an older version. Ditto for the below.

Signature · 1 line
Jens Axboe
author maintainer
#5 lore raw

PATCH 3/3 io_uring: add support for epoll_ctl(2)

In reply to Jens Axboe

This adds IORING_OP_EPOLL_CTL, which can perform the same work as the epoll_ctl(2) system call.

---
 fs/io_uring.c                 | 72 +++++++++++++++++++++++++++++++++++
 include/uapi/linux/io_uring.h |  1 +
 2 files changed, 73 insertions(+)
fs/io_uring.c +72 -0
index 09503d1e9e45..b3bff464d2e7 100644
@@ -74,6 +74,7 @@
 #include <linux/namei.h>
 #include <linux/fsnotify.h>
 #include <linux/fadvise.h>
+#include <linux/eventpoll.h>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/io_uring.h>
@@ -421,6 +422,14 @@ struct io_madvise {
 	u32				advice;
 };
 
+struct io_epoll {
+	struct file			*file;
+	int				epfd;
+	int				op;
+	int				fd;
+	struct epoll_event		event;
+};
+
 struct io_async_connect {
 	struct sockaddr_storage		address;
 };
@@ -534,6 +543,7 @@ struct io_kiocb {
 		struct io_files_update	files_update;
 		struct io_fadvise	fadvise;
 		struct io_madvise	madvise;
+		struct io_epoll		epoll;
 	};
 
 	struct io_async_ctx		*io;
@@ -719,6 +729,9 @@ static const struct io_op_def io_op_defs[] = {
 		.needs_file		= 1,
 		.fd_non_neg		= 1,
 	},
+	[IORING_OP_EPOLL_CTL] = {
+		.unbound_nonreg_file	= 1,
+	},
 };
 
 static void io_wq_submit_work(struct io_wq_work **workptr);
@@ -2578,6 +2591,54 @@ static int io_openat(struct io_kiocb *req, struct io_kiocb **nxt,
 	return io_openat2(req, nxt, force_nonblock);
 }
 
+static int io_epoll_ctl_prep(struct io_kiocb *req,
+			     const struct io_uring_sqe *sqe)
+{
+#if defined(CONFIG_EPOLL)
+	if (sqe->ioprio || sqe->buf_index || sqe->off)
+		return -EINVAL;
+
+	req->epoll.epfd = READ_ONCE(sqe->fd);
+	req->epoll.op = READ_ONCE(sqe->len);
+	req->epoll.fd = READ_ONCE(sqe->off);
+
+	if (ep_op_has_event(req->epoll.op)) {
+		struct epoll_event __user *ev;
+
+		ev = u64_to_user_ptr(READ_ONCE(sqe->addr));
+		if (copy_from_user(&req->epoll.event, ev, sizeof(*ev)))
+			return -EFAULT;
+	}
+
+	return 0;
+#else
+	return -EOPNOTSUPP;
+#endif
+}
+
+static int io_epoll_ctl(struct io_kiocb *req, struct io_kiocb **nxt,
+			bool force_nonblock)
+{
+#if defined(CONFIG_EPOLL)
+	struct io_epoll *ie = &req->epoll;
+	int ret;
+
+	ret = do_epoll_ctl(ie->epfd, ie->op, ie->fd, &ie->event, force_nonblock);
+	if (force_nonblock && ret == -EAGAIN) {
+		req->work.flags |= IO_WQ_WORK_NEEDS_FILES;
+		return -EAGAIN;
+	}
+
+	if (ret < 0)
+		req_set_fail_links(req);
+	io_cqring_add_event(req, ret);
+	io_put_req_find_next(req, nxt);
+	return 0;
+#else
+	return -EOPNOTSUPP;
+#endif
+}
+
 static int io_madvise_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 #if defined(CONFIG_ADVISE_SYSCALLS) && defined(CONFIG_MMU)
@@ -4039,6 +4100,9 @@ static int io_req_defer_prep(struct io_kiocb *req,
 	case IORING_OP_OPENAT2:
 		ret = io_openat2_prep(req, sqe);
 		break;
+	case IORING_OP_EPOLL_CTL:
+		ret = io_epoll_ctl_prep(req, sqe);
+		break;
 	default:
 		printk_once(KERN_WARNING "io_uring: unhandled opcode %d\n",
 				req->opcode);
@@ -4267,6 +4331,14 @@ static int io_issue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe,
 		}
 		ret = io_openat2(req, nxt, force_nonblock);
 		break;
+	case IORING_OP_EPOLL_CTL:
+		if (sqe) {
+			ret = io_epoll_ctl_prep(req, sqe);
+			if (ret)
+				break;
+		}
+		ret = io_epoll_ctl(req, nxt, force_nonblock);
+		break;
 	default:
 		ret = -EINVAL;
 		break;
include/uapi/linux/io_uring.h +1 -0
index 57d05cc5e271..cffa6fd33827 100644
@@ -106,6 +106,7 @@ enum {
 	IORING_OP_SEND,
 	IORING_OP_RECV,
 	IORING_OP_OPENAT2,
+	IORING_OP_EPOLL_CTL,
 
 	/* this goes last, obviously */
 	IORING_OP_LAST,
Signature · 1 line
2.25.0