Add io_uring support for epoll_ctl
Jens Axboe
maintainercover 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
Jens Axboe
maintainerPATCH 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
@@ -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
Jens Axboe
maintainerPATCH 2/3 eventpoll: support non-blocking do_epoll_ctl() calls
In reply to Jens AxboeAlso 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
@@ -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
@@ -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
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
[...]
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
-1if (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); }
Jens Axboe
maintainerRe: [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[...]
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
Jens Axboe
maintainerPATCH 3/3 io_uring: add support for epoll_ctl(2)
In reply to Jens AxboeThis 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
@@ -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
@@ -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