From f3606e3a92ddd36299642c78592fc87609abb1f6 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Tue, 22 Sep 2020 08:18:24 -0600 Subject: io_uring: allow timeout/poll/files killing to take task into account We currently cancel these when the ring exits, and we cancel all of them. This is in preparation for killing only the ones associated with a given task. Reviewed-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 33 ++++++++++++++++++++++++--------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index aae0ef2ec34d..867145fb149c 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1226,13 +1226,26 @@ static void io_kill_timeout(struct io_kiocb *req) } } -static void io_kill_timeouts(struct io_ring_ctx *ctx) +static bool io_task_match(struct io_kiocb *req, struct task_struct *tsk) +{ + struct io_ring_ctx *ctx = req->ctx; + + if (!tsk || req->task == tsk) + return true; + if ((ctx->flags & IORING_SETUP_SQPOLL) && req->task == ctx->sqo_thread) + return true; + return false; +} + +static void io_kill_timeouts(struct io_ring_ctx *ctx, struct task_struct *tsk) { struct io_kiocb *req, *tmp; spin_lock_irq(&ctx->completion_lock); - list_for_each_entry_safe(req, tmp, &ctx->timeout_list, timeout.list) - io_kill_timeout(req); + list_for_each_entry_safe(req, tmp, &ctx->timeout_list, timeout.list) { + if (io_task_match(req, tsk)) + io_kill_timeout(req); + } spin_unlock_irq(&ctx->completion_lock); } @@ -5017,7 +5030,7 @@ static bool io_poll_remove_one(struct io_kiocb *req) return do_complete; } -static void io_poll_remove_all(struct io_ring_ctx *ctx) +static void io_poll_remove_all(struct io_ring_ctx *ctx, struct task_struct *tsk) { struct hlist_node *tmp; struct io_kiocb *req; @@ -5028,8 +5041,10 @@ static void io_poll_remove_all(struct io_ring_ctx *ctx) struct hlist_head *list; list = &ctx->cancel_hash[i]; - hlist_for_each_entry_safe(req, tmp, list, hash_node) - posted += io_poll_remove_one(req); + hlist_for_each_entry_safe(req, tmp, list, hash_node) { + if (io_task_match(req, tsk)) + posted += io_poll_remove_one(req); + } } spin_unlock_irq(&ctx->completion_lock); @@ -7989,8 +8004,8 @@ static void io_ring_ctx_wait_and_kill(struct io_ring_ctx *ctx) percpu_ref_kill(&ctx->refs); mutex_unlock(&ctx->uring_lock); - io_kill_timeouts(ctx); - io_poll_remove_all(ctx); + io_kill_timeouts(ctx, NULL); + io_poll_remove_all(ctx, NULL); if (ctx->io_wq) io_wq_cancel_all(ctx->io_wq); @@ -8221,7 +8236,7 @@ static bool io_cancel_task_cb(struct io_wq_work *work, void *data) struct io_kiocb *req = container_of(work, struct io_kiocb, work); struct task_struct *task = data; - return req->task == task; + return io_task_match(req, task); } static int io_uring_flush(struct file *file, void *data) -- cgit v1.2.3-58-ga151 From f573d384456b3025d3f8e58b3eafaeeb0f510784 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Tue, 22 Sep 2020 10:19:24 -0600 Subject: io_uring: move dropping of files into separate helper No functional changes in this patch, prep patch for grabbing references to the files_struct. Reviewed-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 867145fb149c..73c5dbb1591d 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -5648,6 +5648,20 @@ static int io_req_defer(struct io_kiocb *req, const struct io_uring_sqe *sqe) return -EIOCBQUEUED; } +static void io_req_drop_files(struct io_kiocb *req) +{ + struct io_ring_ctx *ctx = req->ctx; + unsigned long flags; + + spin_lock_irqsave(&ctx->inflight_lock, flags); + list_del(&req->inflight_entry); + if (waitqueue_active(&ctx->inflight_wait)) + wake_up(&ctx->inflight_wait); + spin_unlock_irqrestore(&ctx->inflight_lock, flags); + req->flags &= ~REQ_F_INFLIGHT; + req->work.files = NULL; +} + static void __io_clean_op(struct io_kiocb *req) { struct io_async_ctx *io = req->io; @@ -5697,17 +5711,8 @@ static void __io_clean_op(struct io_kiocb *req) req->flags &= ~REQ_F_NEED_CLEANUP; } - if (req->flags & REQ_F_INFLIGHT) { - struct io_ring_ctx *ctx = req->ctx; - unsigned long flags; - - spin_lock_irqsave(&ctx->inflight_lock, flags); - list_del(&req->inflight_entry); - if (waitqueue_active(&ctx->inflight_wait)) - wake_up(&ctx->inflight_wait); - spin_unlock_irqrestore(&ctx->inflight_lock, flags); - req->flags &= ~REQ_F_INFLIGHT; - } + if (req->flags & REQ_F_INFLIGHT) + io_req_drop_files(req); } static int io_issue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe, -- cgit v1.2.3-58-ga151 From 2aede0e417db846793c276c7a1bbf7262c8349b0 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Mon, 14 Sep 2020 10:45:53 -0600 Subject: io_uring: stash ctx task reference for SQPOLL We can grab a reference to the task instead of stashing away the task files_struct. This is doable without creating a circular reference between the ring fd and the task itself. Reviewed-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 47 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 73c5dbb1591d..d24e0322bd1d 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -265,7 +265,16 @@ struct io_ring_ctx { /* IO offload */ struct io_wq *io_wq; struct task_struct *sqo_thread; /* if using sq thread polling */ - struct mm_struct *sqo_mm; + + /* + * For SQPOLL usage - we hold a reference to the parent task, so we + * have access to the ->files + */ + struct task_struct *sqo_task; + + /* Only used for accounting purposes */ + struct mm_struct *mm_account; + wait_queue_head_t sqo_wait; /* @@ -969,9 +978,10 @@ static int __io_sq_thread_acquire_mm(struct io_ring_ctx *ctx) { if (!current->mm) { if (unlikely(!(ctx->flags & IORING_SETUP_SQPOLL) || - !mmget_not_zero(ctx->sqo_mm))) + !ctx->sqo_task->mm || + !mmget_not_zero(ctx->sqo_task->mm))) return -EFAULT; - kthread_use_mm(ctx->sqo_mm); + kthread_use_mm(ctx->sqo_task->mm); } return 0; @@ -7591,11 +7601,11 @@ static void io_unaccount_mem(struct io_ring_ctx *ctx, unsigned long nr_pages, if (ctx->limit_mem) __io_unaccount_mem(ctx->user, nr_pages); - if (ctx->sqo_mm) { + if (ctx->mm_account) { if (acct == ACCT_LOCKED) - ctx->sqo_mm->locked_vm -= nr_pages; + ctx->mm_account->locked_vm -= nr_pages; else if (acct == ACCT_PINNED) - atomic64_sub(nr_pages, &ctx->sqo_mm->pinned_vm); + atomic64_sub(nr_pages, &ctx->mm_account->pinned_vm); } } @@ -7610,11 +7620,11 @@ static int io_account_mem(struct io_ring_ctx *ctx, unsigned long nr_pages, return ret; } - if (ctx->sqo_mm) { + if (ctx->mm_account) { if (acct == ACCT_LOCKED) - ctx->sqo_mm->locked_vm += nr_pages; + ctx->mm_account->locked_vm += nr_pages; else if (acct == ACCT_PINNED) - atomic64_add(nr_pages, &ctx->sqo_mm->pinned_vm); + atomic64_add(nr_pages, &ctx->mm_account->pinned_vm); } return 0; @@ -7918,9 +7928,12 @@ static void io_ring_ctx_free(struct io_ring_ctx *ctx) { io_finish_async(ctx); io_sqe_buffer_unregister(ctx); - if (ctx->sqo_mm) { - mmdrop(ctx->sqo_mm); - ctx->sqo_mm = NULL; + + if (ctx->sqo_task) { + put_task_struct(ctx->sqo_task); + ctx->sqo_task = NULL; + mmdrop(ctx->mm_account); + ctx->mm_account = NULL; } io_sqe_files_unregister(ctx); @@ -8665,8 +8678,16 @@ static int io_uring_create(unsigned entries, struct io_uring_params *p, ctx->user = user; ctx->creds = get_current_cred(); + ctx->sqo_task = get_task_struct(current); + + /* + * This is just grabbed for accounting purposes. When a process exits, + * the mm is exited and dropped before the files, hence we need to hang + * on to this mm purely for the purposes of being able to unaccount + * memory (locked/pinned vm). It's not used for anything else. + */ mmgrab(current->mm); - ctx->sqo_mm = current->mm; + ctx->mm_account = current->mm; /* * Account memory _before_ installing the file descriptor. Once -- cgit v1.2.3-58-ga151 From e3bc8e9dad7f2f83cc807111d4472164c9210153 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Thu, 24 Sep 2020 08:45:57 -0600 Subject: io_uring: unconditionally grab req->task Sometimes we assign a weak reference to it, sometimes we grab a reference to it. Clean this up and make it unconditional, and drop the flag related to tracking this state. Reviewed-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 47 +++++++++-------------------------------------- 1 file changed, 9 insertions(+), 38 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index d24e0322bd1d..53392f22b212 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -553,7 +553,6 @@ enum { REQ_F_BUFFER_SELECTED_BIT, REQ_F_NO_FILE_TABLE_BIT, REQ_F_WORK_INITIALIZED_BIT, - REQ_F_TASK_PINNED_BIT, /* not a real bit, just to check we're not overflowing the space */ __REQ_F_LAST_BIT, @@ -599,8 +598,6 @@ enum { REQ_F_NO_FILE_TABLE = BIT(REQ_F_NO_FILE_TABLE_BIT), /* io_wq_work is initialized */ REQ_F_WORK_INITIALIZED = BIT(REQ_F_WORK_INITIALIZED_BIT), - /* req->task is refcounted */ - REQ_F_TASK_PINNED = BIT(REQ_F_TASK_PINNED_BIT), }; struct async_poll { @@ -942,14 +939,6 @@ struct sock *io_uring_get_socket(struct file *file) } EXPORT_SYMBOL(io_uring_get_socket); -static void io_get_req_task(struct io_kiocb *req) -{ - if (req->flags & REQ_F_TASK_PINNED) - return; - get_task_struct(req->task); - req->flags |= REQ_F_TASK_PINNED; -} - static inline void io_clean_op(struct io_kiocb *req) { if (req->flags & (REQ_F_NEED_CLEANUP | REQ_F_BUFFER_SELECTED | @@ -957,13 +946,6 @@ static inline void io_clean_op(struct io_kiocb *req) __io_clean_op(req); } -/* not idempotent -- it doesn't clear REQ_F_TASK_PINNED */ -static void __io_put_req_task(struct io_kiocb *req) -{ - if (req->flags & REQ_F_TASK_PINNED) - put_task_struct(req->task); -} - static void io_sq_thread_drop_mm(void) { struct mm_struct *mm = current->mm; @@ -1589,7 +1571,8 @@ static void __io_free_req_finish(struct io_kiocb *req) { struct io_ring_ctx *ctx = req->ctx; - __io_put_req_task(req); + put_task_struct(req->task); + if (likely(!io_is_fallback_req(req))) kmem_cache_free(req_cachep, req); else @@ -1916,16 +1899,13 @@ static void io_req_free_batch(struct req_batch *rb, struct io_kiocb *req) if (req->flags & REQ_F_LINK_HEAD) io_queue_next(req); - if (req->flags & REQ_F_TASK_PINNED) { - if (req->task != rb->task) { - if (rb->task) - put_task_struct_many(rb->task, rb->task_refs); - rb->task = req->task; - rb->task_refs = 0; - } - rb->task_refs++; - req->flags &= ~REQ_F_TASK_PINNED; + if (req->task != rb->task) { + if (rb->task) + put_task_struct_many(rb->task, rb->task_refs); + rb->task = req->task; + rb->task_refs = 0; } + rb->task_refs++; WARN_ON_ONCE(io_dismantle_req(req)); rb->reqs[rb->to_free++] = req; @@ -2550,9 +2530,6 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe, if (kiocb->ki_flags & IOCB_NOWAIT) req->flags |= REQ_F_NOWAIT; - if (kiocb->ki_flags & IOCB_DIRECT) - io_get_req_task(req); - if (force_nonblock) kiocb->ki_flags |= IOCB_NOWAIT; @@ -2564,7 +2541,6 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe, kiocb->ki_flags |= IOCB_HIPRI; kiocb->ki_complete = io_complete_rw_iopoll; req->iopoll_completed = 0; - io_get_req_task(req); } else { if (kiocb->ki_flags & IOCB_HIPRI) return -EINVAL; @@ -3132,8 +3108,6 @@ static bool io_rw_should_retry(struct io_kiocb *req) kiocb->ki_flags |= IOCB_WAITQ; kiocb->ki_flags &= ~IOCB_NOWAIT; kiocb->ki_waitq = wait; - - io_get_req_task(req); return true; } @@ -4965,7 +4939,6 @@ static bool io_arm_poll_handler(struct io_kiocb *req) apoll->double_poll = NULL; req->flags |= REQ_F_POLLED; - io_get_req_task(req); req->apoll = apoll; INIT_HLIST_NODE(&req->hash_node); @@ -5148,8 +5121,6 @@ static int io_poll_add_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe #endif poll->events = demangle_poll(events) | EPOLLERR | EPOLLHUP | (events & EPOLLEXCLUSIVE); - - io_get_req_task(req); return 0; } @@ -6336,7 +6307,6 @@ static int io_submit_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe, return ret; } trace_io_uring_link(ctx, req, head); - io_get_req_task(req); list_add_tail(&req->link_list, &head->link_list); /* last request of a link, enqueue the link */ @@ -6461,6 +6431,7 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req, /* one is dropped after submission, the other at completion */ refcount_set(&req->refs, 2); req->task = current; + get_task_struct(req->task); req->result = 0; if (unlikely(req->opcode >= IORING_OP_LAST)) -- cgit v1.2.3-58-ga151 From 76e1b6427fd8246376a97e3227049d49188dfb9c Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Sat, 26 Sep 2020 15:05:03 -0600 Subject: io_uring: return cancelation status from poll/timeout/files handlers Return whether we found and canceled requests or not. This is in preparation for using this information, no functional changes in this patch. Reviewed-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 53392f22b212..5ddb8b2fe3e5 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1229,16 +1229,23 @@ static bool io_task_match(struct io_kiocb *req, struct task_struct *tsk) return false; } -static void io_kill_timeouts(struct io_ring_ctx *ctx, struct task_struct *tsk) +/* + * Returns true if we found and killed one or more timeouts + */ +static bool io_kill_timeouts(struct io_ring_ctx *ctx, struct task_struct *tsk) { struct io_kiocb *req, *tmp; + int canceled = 0; spin_lock_irq(&ctx->completion_lock); list_for_each_entry_safe(req, tmp, &ctx->timeout_list, timeout.list) { - if (io_task_match(req, tsk)) + if (io_task_match(req, tsk)) { io_kill_timeout(req); + canceled++; + } } spin_unlock_irq(&ctx->completion_lock); + return canceled != 0; } static void __io_queue_deferred(struct io_ring_ctx *ctx) @@ -5013,7 +5020,10 @@ static bool io_poll_remove_one(struct io_kiocb *req) return do_complete; } -static void io_poll_remove_all(struct io_ring_ctx *ctx, struct task_struct *tsk) +/* + * Returns true if we found and killed one or more poll requests + */ +static bool io_poll_remove_all(struct io_ring_ctx *ctx, struct task_struct *tsk) { struct hlist_node *tmp; struct io_kiocb *req; @@ -5033,6 +5043,8 @@ static void io_poll_remove_all(struct io_ring_ctx *ctx, struct task_struct *tsk) if (posted) io_cqring_ev_posted(ctx); + + return posted != 0; } static int io_poll_cancel(struct io_ring_ctx *ctx, __u64 sqe_addr) @@ -8178,11 +8190,14 @@ static void io_cancel_defer_files(struct io_ring_ctx *ctx, } } -static void io_uring_cancel_files(struct io_ring_ctx *ctx, +/* + * Returns true if we found and killed one or more files pinning requests + */ +static bool io_uring_cancel_files(struct io_ring_ctx *ctx, struct files_struct *files) { if (list_empty_careful(&ctx->inflight_list)) - return; + return false; io_cancel_defer_files(ctx, files); /* cancel all at once, should be faster than doing it one by one*/ @@ -8218,6 +8233,8 @@ static void io_uring_cancel_files(struct io_ring_ctx *ctx, schedule(); finish_wait(&ctx->inflight_wait, &wait); } + + return true; } static bool io_cancel_task_cb(struct io_wq_work *work, void *data) -- cgit v1.2.3-58-ga151 From e6c8aa9ac33bd7c968af7816240fc081401fddcd Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Mon, 28 Sep 2020 13:10:13 -0600 Subject: io_uring: enable task/files specific overflow flushing This allows us to selectively flush out pending overflows, depending on the task and/or files_struct being passed in. No intended functional changes in this patch. Reviewed-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 5ddb8b2fe3e5..046d06266a11 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1344,12 +1344,24 @@ static void io_cqring_mark_overflow(struct io_ring_ctx *ctx) } } +static inline bool io_match_files(struct io_kiocb *req, + struct files_struct *files) +{ + if (!files) + return true; + if (req->flags & REQ_F_WORK_INITIALIZED) + return req->work.files == files; + return false; +} + /* Returns true if there are no backlogged entries after the flush */ -static bool io_cqring_overflow_flush(struct io_ring_ctx *ctx, bool force) +static bool io_cqring_overflow_flush(struct io_ring_ctx *ctx, bool force, + struct task_struct *tsk, + struct files_struct *files) { struct io_rings *rings = ctx->rings; + struct io_kiocb *req, *tmp; struct io_uring_cqe *cqe; - struct io_kiocb *req; unsigned long flags; LIST_HEAD(list); @@ -1368,13 +1380,16 @@ static bool io_cqring_overflow_flush(struct io_ring_ctx *ctx, bool force) ctx->cq_overflow_flushed = 1; cqe = NULL; - while (!list_empty(&ctx->cq_overflow_list)) { + list_for_each_entry_safe(req, tmp, &ctx->cq_overflow_list, compl.list) { + if (tsk && req->task != tsk) + continue; + if (!io_match_files(req, files)) + continue; + cqe = io_get_cqring(ctx); if (!cqe && !force) break; - req = list_first_entry(&ctx->cq_overflow_list, struct io_kiocb, - compl.list); list_move(&req->compl.list, &list); if (cqe) { WRITE_ONCE(cqe->user_data, req->user_data); @@ -1988,7 +2003,7 @@ static unsigned io_cqring_events(struct io_ring_ctx *ctx, bool noflush) if (noflush && !list_empty(&ctx->cq_overflow_list)) return -1U; - io_cqring_overflow_flush(ctx, false); + io_cqring_overflow_flush(ctx, false, NULL, NULL); } /* See comment at the top of this file */ @@ -6489,7 +6504,7 @@ static int io_submit_sqes(struct io_ring_ctx *ctx, unsigned int nr, /* if we have a backlog and couldn't flush it all, return BUSY */ if (test_bit(0, &ctx->sq_check_overflow)) { if (!list_empty(&ctx->cq_overflow_list) && - !io_cqring_overflow_flush(ctx, false)) + !io_cqring_overflow_flush(ctx, false, NULL, NULL)) return -EBUSY; } @@ -7993,7 +8008,7 @@ static void io_ring_exit_work(struct work_struct *work) */ do { if (ctx->rings) - io_cqring_overflow_flush(ctx, true); + io_cqring_overflow_flush(ctx, true, NULL, NULL); io_iopoll_try_reap_events(ctx); } while (!wait_for_completion_timeout(&ctx->ref_comp, HZ/20)); io_ring_ctx_free(ctx); @@ -8013,7 +8028,7 @@ static void io_ring_ctx_wait_and_kill(struct io_ring_ctx *ctx) /* if we failed setting up the ctx, we might not have any rings */ if (ctx->rings) - io_cqring_overflow_flush(ctx, true); + io_cqring_overflow_flush(ctx, true, NULL, NULL); io_iopoll_try_reap_events(ctx); idr_for_each(&ctx->personality_idr, io_remove_personalities, ctx); @@ -8069,12 +8084,6 @@ static bool io_match_link(struct io_kiocb *preq, struct io_kiocb *req) return false; } -static inline bool io_match_files(struct io_kiocb *req, - struct files_struct *files) -{ - return (req->flags & REQ_F_WORK_INITIALIZED) && req->work.files == files; -} - static bool io_match_link_files(struct io_kiocb *req, struct files_struct *files) { @@ -8365,7 +8374,7 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit, ret = 0; if (ctx->flags & IORING_SETUP_SQPOLL) { if (!list_empty_careful(&ctx->cq_overflow_list)) - io_cqring_overflow_flush(ctx, false); + io_cqring_overflow_flush(ctx, false, NULL, NULL); if (flags & IORING_ENTER_SQ_WAKEUP) wake_up(&ctx->sqo_wait); submitted = to_submit; -- cgit v1.2.3-58-ga151 From 0f2122045b946241a9e549c2a76cea54fa58a7ff Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Sun, 13 Sep 2020 13:09:39 -0600 Subject: io_uring: don't rely on weak ->files references Grab actual references to the files_struct. To avoid circular references issues due to this, we add a per-task note that keeps track of what io_uring contexts a task has used. When the tasks execs or exits its assigned files, we cancel requests based on this tracking. With that, we can grab proper references to the files table, and no longer need to rely on stashing away ring_fd and ring_file to check if the ring_fd may have been closed. Cc: stable@vger.kernel.org # v5.5+ Reviewed-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/exec.c | 6 + fs/file.c | 2 + fs/io_uring.c | 306 +++++++++++++++++++++++++++++++++++++++++------ include/linux/io_uring.h | 53 ++++++++ include/linux/sched.h | 5 + init/init_task.c | 3 + kernel/fork.c | 6 + 7 files changed, 344 insertions(+), 37 deletions(-) create mode 100644 include/linux/io_uring.h diff --git a/fs/exec.c b/fs/exec.c index a91003e28eaa..07910f5032e7 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -62,6 +62,7 @@ #include #include #include +#include #include #include @@ -1895,6 +1896,11 @@ static int bprm_execve(struct linux_binprm *bprm, struct files_struct *displaced; int retval; + /* + * Cancel any io_uring activity across execve + */ + io_uring_task_cancel(); + retval = unshare_files(&displaced); if (retval) return retval; diff --git a/fs/file.c b/fs/file.c index 21c0893f2f1d..4559b5fec3bd 100644 --- a/fs/file.c +++ b/fs/file.c @@ -21,6 +21,7 @@ #include #include #include +#include unsigned int sysctl_nr_open __read_mostly = 1024*1024; unsigned int sysctl_nr_open_min = BITS_PER_LONG; @@ -452,6 +453,7 @@ void exit_files(struct task_struct *tsk) struct files_struct * files = tsk->files; if (files) { + io_uring_files_cancel(files); task_lock(tsk); tsk->files = NULL; task_unlock(tsk); diff --git a/fs/io_uring.c b/fs/io_uring.c index 046d06266a11..ee75ba7113cf 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -79,6 +79,7 @@ #include #include #include +#include #define CREATE_TRACE_POINTS #include @@ -284,8 +285,6 @@ struct io_ring_ctx { */ struct fixed_file_data *file_data; unsigned nr_user_files; - int ring_fd; - struct file *ring_file; /* if used, fixed mapped user buffers */ unsigned nr_user_bufs; @@ -1433,7 +1432,12 @@ static void __io_cqring_fill_event(struct io_kiocb *req, long res, long cflags) WRITE_ONCE(cqe->user_data, req->user_data); WRITE_ONCE(cqe->res, res); WRITE_ONCE(cqe->flags, cflags); - } else if (ctx->cq_overflow_flushed) { + } else if (ctx->cq_overflow_flushed || req->task->io_uring->in_idle) { + /* + * If we're in ring overflow flush mode, or in task cancel mode, + * then we cannot store the request for later flushing, we need + * to drop it on the floor. + */ WRITE_ONCE(ctx->rings->cq_overflow, atomic_inc_return(&ctx->cached_cq_overflow)); } else { @@ -1591,8 +1595,12 @@ static bool io_dismantle_req(struct io_kiocb *req) static void __io_free_req_finish(struct io_kiocb *req) { + struct io_uring_task *tctx = req->task->io_uring; struct io_ring_ctx *ctx = req->ctx; + atomic_long_inc(&tctx->req_complete); + if (tctx->in_idle) + wake_up(&tctx->wait); put_task_struct(req->task); if (likely(!io_is_fallback_req(req))) @@ -1907,6 +1915,7 @@ static void io_req_free_batch_finish(struct io_ring_ctx *ctx, if (rb->to_free) __io_req_free_batch_flush(ctx, rb); if (rb->task) { + atomic_long_add(rb->task_refs, &rb->task->io_uring->req_complete); put_task_struct_many(rb->task, rb->task_refs); rb->task = NULL; } @@ -1922,8 +1931,10 @@ static void io_req_free_batch(struct req_batch *rb, struct io_kiocb *req) io_queue_next(req); if (req->task != rb->task) { - if (rb->task) + if (rb->task) { + atomic_long_add(rb->task_refs, &rb->task->io_uring->req_complete); put_task_struct_many(rb->task, rb->task_refs); + } rb->task = req->task; rb->task_refs = 0; } @@ -3978,8 +3989,7 @@ static int io_close_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) return -EBADF; req->close.fd = READ_ONCE(sqe->fd); - if ((req->file && req->file->f_op == &io_uring_fops) || - req->close.fd == req->ctx->ring_fd) + if ((req->file && req->file->f_op == &io_uring_fops)) return -EBADF; req->close.put_file = NULL; @@ -5667,6 +5677,7 @@ static void io_req_drop_files(struct io_kiocb *req) wake_up(&ctx->inflight_wait); spin_unlock_irqrestore(&ctx->inflight_lock, flags); req->flags &= ~REQ_F_INFLIGHT; + put_files_struct(req->work.files); req->work.files = NULL; } @@ -6067,34 +6078,20 @@ static int io_req_set_file(struct io_submit_state *state, struct io_kiocb *req, static int io_grab_files(struct io_kiocb *req) { - int ret = -EBADF; struct io_ring_ctx *ctx = req->ctx; io_req_init_async(req); if (req->work.files || (req->flags & REQ_F_NO_FILE_TABLE)) return 0; - if (!ctx->ring_file) - return -EBADF; - rcu_read_lock(); + req->work.files = get_files_struct(current); + req->flags |= REQ_F_INFLIGHT; + spin_lock_irq(&ctx->inflight_lock); - /* - * We use the f_ops->flush() handler to ensure that we can flush - * out work accessing these files if the fd is closed. Check if - * the fd has changed since we started down this path, and disallow - * this operation if it has. - */ - if (fcheck(ctx->ring_fd) == ctx->ring_file) { - list_add(&req->inflight_entry, &ctx->inflight_list); - req->flags |= REQ_F_INFLIGHT; - req->work.files = current->files; - ret = 0; - } + list_add(&req->inflight_entry, &ctx->inflight_list); spin_unlock_irq(&ctx->inflight_lock); - rcu_read_unlock(); - - return ret; + return 0; } static inline int io_prep_work_files(struct io_kiocb *req) @@ -6459,6 +6456,7 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req, refcount_set(&req->refs, 2); req->task = current; get_task_struct(req->task); + atomic_long_inc(&req->task->io_uring->req_issue); req->result = 0; if (unlikely(req->opcode >= IORING_OP_LAST)) @@ -6494,8 +6492,7 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req, return io_req_set_file(state, req, READ_ONCE(sqe->fd)); } -static int io_submit_sqes(struct io_ring_ctx *ctx, unsigned int nr, - struct file *ring_file, int ring_fd) +static int io_submit_sqes(struct io_ring_ctx *ctx, unsigned int nr) { struct io_submit_state state; struct io_kiocb *link = NULL; @@ -6516,9 +6513,6 @@ static int io_submit_sqes(struct io_ring_ctx *ctx, unsigned int nr, io_submit_state_start(&state, ctx, nr); - ctx->ring_fd = ring_fd; - ctx->ring_file = ring_file; - for (i = 0; i < nr; i++) { const struct io_uring_sqe *sqe; struct io_kiocb *req; @@ -6687,7 +6681,7 @@ static int io_sq_thread(void *data) mutex_lock(&ctx->uring_lock); if (likely(!percpu_ref_is_dying(&ctx->refs))) - ret = io_submit_sqes(ctx, to_submit, NULL, -1); + ret = io_submit_sqes(ctx, to_submit); mutex_unlock(&ctx->uring_lock); timeout = jiffies + ctx->sq_thread_idle; } @@ -7516,6 +7510,34 @@ out_fput: return ret; } +static int io_uring_alloc_task_context(struct task_struct *task) +{ + struct io_uring_task *tctx; + + tctx = kmalloc(sizeof(*tctx), GFP_KERNEL); + if (unlikely(!tctx)) + return -ENOMEM; + + xa_init(&tctx->xa); + init_waitqueue_head(&tctx->wait); + tctx->last = NULL; + tctx->in_idle = 0; + atomic_long_set(&tctx->req_issue, 0); + atomic_long_set(&tctx->req_complete, 0); + task->io_uring = tctx; + return 0; +} + +void __io_uring_free(struct task_struct *tsk) +{ + struct io_uring_task *tctx = tsk->io_uring; + + WARN_ON_ONCE(!xa_empty(&tctx->xa)); + xa_destroy(&tctx->xa); + kfree(tctx); + tsk->io_uring = NULL; +} + static int io_sq_offload_start(struct io_ring_ctx *ctx, struct io_uring_params *p) { @@ -7551,6 +7573,9 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx, ctx->sqo_thread = NULL; goto err; } + ret = io_uring_alloc_task_context(ctx->sqo_thread); + if (ret) + goto err; wake_up_process(ctx->sqo_thread); } else if (p->flags & IORING_SETUP_SQ_AFF) { /* Can't have SQ_AFF without SQPOLL */ @@ -8063,7 +8088,7 @@ static bool io_wq_files_match(struct io_wq_work *work, void *data) { struct files_struct *files = data; - return work->files == files; + return !files || work->files == files; } /* @@ -8218,7 +8243,7 @@ static bool io_uring_cancel_files(struct io_ring_ctx *ctx, spin_lock_irq(&ctx->inflight_lock); list_for_each_entry(req, &ctx->inflight_list, inflight_entry) { - if (req->work.files != files) + if (files && req->work.files != files) continue; /* req is being completed, ignore */ if (!refcount_inc_not_zero(&req->refs)) @@ -8254,18 +8279,217 @@ static bool io_cancel_task_cb(struct io_wq_work *work, void *data) return io_task_match(req, task); } +static bool __io_uring_cancel_task_requests(struct io_ring_ctx *ctx, + struct task_struct *task, + struct files_struct *files) +{ + bool ret; + + ret = io_uring_cancel_files(ctx, files); + if (!files) { + enum io_wq_cancel cret; + + cret = io_wq_cancel_cb(ctx->io_wq, io_cancel_task_cb, task, true); + if (cret != IO_WQ_CANCEL_NOTFOUND) + ret = true; + + /* SQPOLL thread does its own polling */ + if (!(ctx->flags & IORING_SETUP_SQPOLL)) { + while (!list_empty_careful(&ctx->iopoll_list)) { + io_iopoll_try_reap_events(ctx); + ret = true; + } + } + + ret |= io_poll_remove_all(ctx, task); + ret |= io_kill_timeouts(ctx, task); + } + + return ret; +} + +/* + * We need to iteratively cancel requests, in case a request has dependent + * hard links. These persist even for failure of cancelations, hence keep + * looping until none are found. + */ +static void io_uring_cancel_task_requests(struct io_ring_ctx *ctx, + struct files_struct *files) +{ + struct task_struct *task = current; + + if (ctx->flags & IORING_SETUP_SQPOLL) + task = ctx->sqo_thread; + + io_cqring_overflow_flush(ctx, true, task, files); + + while (__io_uring_cancel_task_requests(ctx, task, files)) { + io_run_task_work(); + cond_resched(); + } +} + +/* + * Note that this task has used io_uring. We use it for cancelation purposes. + */ +static int io_uring_add_task_file(struct file *file) +{ + if (unlikely(!current->io_uring)) { + int ret; + + ret = io_uring_alloc_task_context(current); + if (unlikely(ret)) + return ret; + } + if (current->io_uring->last != file) { + XA_STATE(xas, ¤t->io_uring->xa, (unsigned long) file); + void *old; + + rcu_read_lock(); + old = xas_load(&xas); + if (old != file) { + get_file(file); + xas_lock(&xas); + xas_store(&xas, file); + xas_unlock(&xas); + } + rcu_read_unlock(); + current->io_uring->last = file; + } + + return 0; +} + +/* + * Remove this io_uring_file -> task mapping. + */ +static void io_uring_del_task_file(struct file *file) +{ + struct io_uring_task *tctx = current->io_uring; + XA_STATE(xas, &tctx->xa, (unsigned long) file); + + if (tctx->last == file) + tctx->last = NULL; + + xas_lock(&xas); + file = xas_store(&xas, NULL); + xas_unlock(&xas); + + if (file) + fput(file); +} + +static void __io_uring_attempt_task_drop(struct file *file) +{ + XA_STATE(xas, ¤t->io_uring->xa, (unsigned long) file); + struct file *old; + + rcu_read_lock(); + old = xas_load(&xas); + rcu_read_unlock(); + + if (old == file) + io_uring_del_task_file(file); +} + +/* + * Drop task note for this file if we're the only ones that hold it after + * pending fput() + */ +static void io_uring_attempt_task_drop(struct file *file, bool exiting) +{ + if (!current->io_uring) + return; + /* + * fput() is pending, will be 2 if the only other ref is our potential + * task file note. If the task is exiting, drop regardless of count. + */ + if (!exiting && atomic_long_read(&file->f_count) != 2) + return; + + __io_uring_attempt_task_drop(file); +} + +void __io_uring_files_cancel(struct files_struct *files) +{ + struct io_uring_task *tctx = current->io_uring; + XA_STATE(xas, &tctx->xa, 0); + + /* make sure overflow events are dropped */ + tctx->in_idle = true; + + do { + struct io_ring_ctx *ctx; + struct file *file; + + xas_lock(&xas); + file = xas_next_entry(&xas, ULONG_MAX); + xas_unlock(&xas); + + if (!file) + break; + + ctx = file->private_data; + + io_uring_cancel_task_requests(ctx, files); + if (files) + io_uring_del_task_file(file); + } while (1); +} + +static inline bool io_uring_task_idle(struct io_uring_task *tctx) +{ + return atomic_long_read(&tctx->req_issue) == + atomic_long_read(&tctx->req_complete); +} + +/* + * Find any io_uring fd that this task has registered or done IO on, and cancel + * requests. + */ +void __io_uring_task_cancel(void) +{ + struct io_uring_task *tctx = current->io_uring; + DEFINE_WAIT(wait); + long completions; + + /* make sure overflow events are dropped */ + tctx->in_idle = true; + + while (!io_uring_task_idle(tctx)) { + /* read completions before cancelations */ + completions = atomic_long_read(&tctx->req_complete); + __io_uring_files_cancel(NULL); + + prepare_to_wait(&tctx->wait, &wait, TASK_UNINTERRUPTIBLE); + + /* + * If we've seen completions, retry. This avoids a race where + * a completion comes in before we did prepare_to_wait(). + */ + if (completions != atomic_long_read(&tctx->req_complete)) + continue; + if (io_uring_task_idle(tctx)) + break; + schedule(); + } + + finish_wait(&tctx->wait, &wait); + tctx->in_idle = false; +} + static int io_uring_flush(struct file *file, void *data) { struct io_ring_ctx *ctx = file->private_data; - io_uring_cancel_files(ctx, data); - /* * If the task is going away, cancel work it may have pending */ if (fatal_signal_pending(current) || (current->flags & PF_EXITING)) - io_wq_cancel_cb(ctx->io_wq, io_cancel_task_cb, current, true); + data = NULL; + io_uring_cancel_task_requests(ctx, data); + io_uring_attempt_task_drop(file, !data); return 0; } @@ -8379,8 +8603,11 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit, wake_up(&ctx->sqo_wait); submitted = to_submit; } else if (to_submit) { + ret = io_uring_add_task_file(f.file); + if (unlikely(ret)) + goto out; mutex_lock(&ctx->uring_lock); - submitted = io_submit_sqes(ctx, to_submit, f.file, fd); + submitted = io_submit_sqes(ctx, to_submit); mutex_unlock(&ctx->uring_lock); if (submitted != to_submit) @@ -8590,6 +8817,7 @@ static int io_uring_get_fd(struct io_ring_ctx *ctx) file = anon_inode_getfile("[io_uring]", &io_uring_fops, ctx, O_RDWR | O_CLOEXEC); if (IS_ERR(file)) { +err_fd: put_unused_fd(ret); ret = PTR_ERR(file); goto err; @@ -8598,6 +8826,10 @@ static int io_uring_get_fd(struct io_ring_ctx *ctx) #if defined(CONFIG_UNIX) ctx->ring_sock->file = file; #endif + if (unlikely(io_uring_add_task_file(file))) { + file = ERR_PTR(-ENOMEM); + goto err_fd; + } fd_install(ret, file); return ret; err: diff --git a/include/linux/io_uring.h b/include/linux/io_uring.h new file mode 100644 index 000000000000..c09135a1ef13 --- /dev/null +++ b/include/linux/io_uring.h @@ -0,0 +1,53 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +#ifndef _LINUX_IO_URING_H +#define _LINUX_IO_URING_H + +#include +#include +#include + +struct io_uring_task { + /* submission side */ + struct xarray xa; + struct wait_queue_head wait; + struct file *last; + atomic_long_t req_issue; + + /* completion side */ + bool in_idle ____cacheline_aligned_in_smp; + atomic_long_t req_complete; +}; + +#if defined(CONFIG_IO_URING) +void __io_uring_task_cancel(void); +void __io_uring_files_cancel(struct files_struct *files); +void __io_uring_free(struct task_struct *tsk); + +static inline void io_uring_task_cancel(void) +{ + if (current->io_uring && !xa_empty(¤t->io_uring->xa)) + __io_uring_task_cancel(); +} +static inline void io_uring_files_cancel(struct files_struct *files) +{ + if (current->io_uring && !xa_empty(¤t->io_uring->xa)) + __io_uring_files_cancel(files); +} +static inline void io_uring_free(struct task_struct *tsk) +{ + if (tsk->io_uring) + __io_uring_free(tsk); +} +#else +static inline void io_uring_task_cancel(void) +{ +} +static inline void io_uring_files_cancel(struct files_struct *files) +{ +} +static inline void io_uring_free(struct task_struct *tsk) +{ +} +#endif + +#endif diff --git a/include/linux/sched.h b/include/linux/sched.h index afe01e232935..8bf2295ebee4 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -63,6 +63,7 @@ struct sighand_struct; struct signal_struct; struct task_delay_info; struct task_group; +struct io_uring_task; /* * Task state bitmask. NOTE! These bits are also @@ -935,6 +936,10 @@ struct task_struct { /* Open file information: */ struct files_struct *files; +#ifdef CONFIG_IO_URING + struct io_uring_task *io_uring; +#endif + /* Namespaces: */ struct nsproxy *nsproxy; diff --git a/init/init_task.c b/init/init_task.c index f6889fce64af..a56f0abb63e9 100644 --- a/init/init_task.c +++ b/init/init_task.c @@ -114,6 +114,9 @@ struct task_struct init_task .thread = INIT_THREAD, .fs = &init_fs, .files = &init_files, +#ifdef CONFIG_IO_URING + .io_uring = NULL, +#endif .signal = &init_signals, .sighand = &init_sighand, .nsproxy = &init_nsproxy, diff --git a/kernel/fork.c b/kernel/fork.c index da8d360fb032..a3795aaaab5c 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -95,6 +95,7 @@ #include #include #include +#include #include #include @@ -728,6 +729,7 @@ void __put_task_struct(struct task_struct *tsk) WARN_ON(refcount_read(&tsk->usage)); WARN_ON(tsk == current); + io_uring_free(tsk); cgroup_free(tsk); task_numa_free(tsk, true); security_task_free(tsk); @@ -1983,6 +1985,10 @@ static __latent_entropy struct task_struct *copy_process( p->vtime.state = VTIME_INACTIVE; #endif +#ifdef CONFIG_IO_URING + p->io_uring = NULL; +#endif + #if defined(SPLIT_RSS_COUNTING) memset(&p->rss_stat, 0, sizeof(p->rss_stat)); #endif -- cgit v1.2.3-58-ga151 From 9b8284921513fc1ea57d87777283a59b05862f03 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Fri, 18 Sep 2020 20:13:06 -0600 Subject: io_uring: reference ->nsproxy for file table commands If we don't get and assign the namespace for the async work, then certain paths just don't work properly (like /dev/stdin, /proc/mounts, etc). Anything that references the current namespace of the given task should be assigned for async work on behalf of that task. Cc: stable@vger.kernel.org # v5.5+ Reported-by: Al Viro Signed-off-by: Jens Axboe --- fs/io-wq.c | 4 ++++ fs/io-wq.h | 1 + fs/io_uring.c | 3 +++ 3 files changed, 8 insertions(+) diff --git a/fs/io-wq.c b/fs/io-wq.c index 414beb543883..09e20bbf0d37 100644 --- a/fs/io-wq.c +++ b/fs/io-wq.c @@ -60,6 +60,7 @@ struct io_worker { const struct cred *cur_creds; const struct cred *saved_creds; struct files_struct *restore_files; + struct nsproxy *restore_nsproxy; struct fs_struct *restore_fs; }; @@ -153,6 +154,7 @@ static bool __io_worker_unuse(struct io_wqe *wqe, struct io_worker *worker) task_lock(current); current->files = worker->restore_files; + current->nsproxy = worker->restore_nsproxy; task_unlock(current); } @@ -318,6 +320,7 @@ static void io_worker_start(struct io_wqe *wqe, struct io_worker *worker) worker->flags |= (IO_WORKER_F_UP | IO_WORKER_F_RUNNING); worker->restore_files = current->files; + worker->restore_nsproxy = current->nsproxy; worker->restore_fs = current->fs; io_wqe_inc_running(wqe, worker); } @@ -454,6 +457,7 @@ static void io_impersonate_work(struct io_worker *worker, if (work->files && current->files != work->files) { task_lock(current); current->files = work->files; + current->nsproxy = work->nsproxy; task_unlock(current); } if (work->fs && current->fs != work->fs) diff --git a/fs/io-wq.h b/fs/io-wq.h index ddaf9614cf9b..2519830c8c55 100644 --- a/fs/io-wq.h +++ b/fs/io-wq.h @@ -88,6 +88,7 @@ struct io_wq_work { struct files_struct *files; struct mm_struct *mm; const struct cred *creds; + struct nsproxy *nsproxy; struct fs_struct *fs; unsigned long fsize; unsigned flags; diff --git a/fs/io_uring.c b/fs/io_uring.c index ee75ba7113cf..05ec385a6094 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -5678,6 +5678,7 @@ static void io_req_drop_files(struct io_kiocb *req) spin_unlock_irqrestore(&ctx->inflight_lock, flags); req->flags &= ~REQ_F_INFLIGHT; put_files_struct(req->work.files); + put_nsproxy(req->work.nsproxy); req->work.files = NULL; } @@ -6086,6 +6087,8 @@ static int io_grab_files(struct io_kiocb *req) return 0; req->work.files = get_files_struct(current); + get_nsproxy(current->nsproxy); + req->work.nsproxy = current->nsproxy; req->flags |= REQ_F_INFLIGHT; spin_lock_irq(&ctx->inflight_lock); -- cgit v1.2.3-58-ga151 From a3ec60054082ca2c2145ba487f9fc4de904e2b03 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Fri, 18 Sep 2020 20:41:00 -0600 Subject: io_uring: move io_uring_get_socket() into io_uring.h Now we have a io_uring kernel header, move this definition out of fs.h and into io_uring.h where it belongs. Signed-off-by: Jens Axboe --- include/linux/fs.h | 9 --------- include/linux/io_uring.h | 5 +++++ net/unix/scm.c | 1 + 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/include/linux/fs.h b/include/linux/fs.h index 7519ae003a08..1c4068428461 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -3514,15 +3514,6 @@ extern int vfs_fadvise(struct file *file, loff_t offset, loff_t len, extern int generic_fadvise(struct file *file, loff_t offset, loff_t len, int advice); -#if defined(CONFIG_IO_URING) -extern struct sock *io_uring_get_socket(struct file *file); -#else -static inline struct sock *io_uring_get_socket(struct file *file) -{ - return NULL; -} -#endif - int vfs_ioc_setflags_prepare(struct inode *inode, unsigned int oldflags, unsigned int flags); diff --git a/include/linux/io_uring.h b/include/linux/io_uring.h index c09135a1ef13..96315cfaf6d1 100644 --- a/include/linux/io_uring.h +++ b/include/linux/io_uring.h @@ -19,6 +19,7 @@ struct io_uring_task { }; #if defined(CONFIG_IO_URING) +struct sock *io_uring_get_socket(struct file *file); void __io_uring_task_cancel(void); void __io_uring_files_cancel(struct files_struct *files); void __io_uring_free(struct task_struct *tsk); @@ -39,6 +40,10 @@ static inline void io_uring_free(struct task_struct *tsk) __io_uring_free(tsk); } #else +static inline struct sock *io_uring_get_socket(struct file *file) +{ + return NULL; +} static inline void io_uring_task_cancel(void) { } diff --git a/net/unix/scm.c b/net/unix/scm.c index 8c40f2b32392..052ae709ce28 100644 --- a/net/unix/scm.c +++ b/net/unix/scm.c @@ -8,6 +8,7 @@ #include #include #include +#include #include "scm.h" -- cgit v1.2.3-58-ga151 From 9d4a75efa200a31deabe9ba1c941aef697e6bb30 Mon Sep 17 00:00:00 2001 From: Stefano Garzarella Date: Thu, 27 Aug 2020 16:58:29 +0200 Subject: io_uring: use an enumeration for io_uring_register(2) opcodes The enumeration allows us to keep track of the last io_uring_register(2) opcode available. Behaviour and opcodes names don't change. Signed-off-by: Stefano Garzarella Reviewed-by: Kees Cook Signed-off-by: Jens Axboe --- include/uapi/linux/io_uring.h | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h index d65fde732518..5f12ae6a415c 100644 --- a/include/uapi/linux/io_uring.h +++ b/include/uapi/linux/io_uring.h @@ -255,17 +255,22 @@ struct io_uring_params { /* * io_uring_register(2) opcodes and arguments */ -#define IORING_REGISTER_BUFFERS 0 -#define IORING_UNREGISTER_BUFFERS 1 -#define IORING_REGISTER_FILES 2 -#define IORING_UNREGISTER_FILES 3 -#define IORING_REGISTER_EVENTFD 4 -#define IORING_UNREGISTER_EVENTFD 5 -#define IORING_REGISTER_FILES_UPDATE 6 -#define IORING_REGISTER_EVENTFD_ASYNC 7 -#define IORING_REGISTER_PROBE 8 -#define IORING_REGISTER_PERSONALITY 9 -#define IORING_UNREGISTER_PERSONALITY 10 +enum { + IORING_REGISTER_BUFFERS = 0, + IORING_UNREGISTER_BUFFERS = 1, + IORING_REGISTER_FILES = 2, + IORING_UNREGISTER_FILES = 3, + IORING_REGISTER_EVENTFD = 4, + IORING_UNREGISTER_EVENTFD = 5, + IORING_REGISTER_FILES_UPDATE = 6, + IORING_REGISTER_EVENTFD_ASYNC = 7, + IORING_REGISTER_PROBE = 8, + IORING_REGISTER_PERSONALITY = 9, + IORING_UNREGISTER_PERSONALITY = 10, + + /* this goes last */ + IORING_REGISTER_LAST +}; struct io_uring_files_update { __u32 offset; -- cgit v1.2.3-58-ga151 From 21b55dbc0653018b8cd4513c37cbca303b0f0d50 Mon Sep 17 00:00:00 2001 From: Stefano Garzarella Date: Thu, 27 Aug 2020 16:58:30 +0200 Subject: io_uring: add IOURING_REGISTER_RESTRICTIONS opcode The new io_uring_register(2) IOURING_REGISTER_RESTRICTIONS opcode permanently installs a feature allowlist on an io_ring_ctx. The io_ring_ctx can then be passed to untrusted code with the knowledge that only operations present in the allowlist can be executed. The allowlist approach ensures that new features added to io_uring do not accidentally become available when an existing application is launched on a newer kernel version. Currently is it possible to restrict sqe opcodes, sqe flags, and register opcodes. IOURING_REGISTER_RESTRICTIONS can only be made once. Afterwards it is not possible to change restrictions anymore. This prevents untrusted code from removing restrictions. Suggested-by: Stefan Hajnoczi Signed-off-by: Stefano Garzarella Reviewed-by: Kees Cook Signed-off-by: Jens Axboe --- fs/io_uring.c | 124 +++++++++++++++++++++++++++++++++++++++++- include/uapi/linux/io_uring.h | 31 +++++++++++ 2 files changed, 154 insertions(+), 1 deletion(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 05ec385a6094..c4855cecc8f3 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -99,6 +99,8 @@ #define IORING_MAX_FILES_TABLE (1U << IORING_FILE_TABLE_SHIFT) #define IORING_FILE_TABLE_MASK (IORING_MAX_FILES_TABLE - 1) #define IORING_MAX_FIXED_FILES (64 * IORING_MAX_FILES_TABLE) +#define IORING_MAX_RESTRICTIONS (IORING_RESTRICTION_LAST + \ + IORING_REGISTER_LAST + IORING_OP_LAST) struct io_uring { u32 head ____cacheline_aligned_in_smp; @@ -220,6 +222,13 @@ struct io_buffer { __u16 bid; }; +struct io_restriction { + DECLARE_BITMAP(register_op, IORING_REGISTER_LAST); + DECLARE_BITMAP(sqe_op, IORING_OP_LAST); + u8 sqe_flags_allowed; + u8 sqe_flags_required; +}; + struct io_ring_ctx { struct { struct percpu_ref refs; @@ -232,6 +241,7 @@ struct io_ring_ctx { unsigned int cq_overflow_flushed: 1; unsigned int drain_next: 1; unsigned int eventfd_async: 1; + unsigned int restricted: 1; /* * Ring buffer of indices into array of io_uring_sqe, which is @@ -346,6 +356,7 @@ struct io_ring_ctx { struct llist_head file_put_llist; struct work_struct exit_work; + struct io_restriction restrictions; }; /* @@ -6438,6 +6449,32 @@ static inline void io_consume_sqe(struct io_ring_ctx *ctx) ctx->cached_sq_head++; } +/* + * Check SQE restrictions (opcode and flags). + * + * Returns 'true' if SQE is allowed, 'false' otherwise. + */ +static inline bool io_check_restriction(struct io_ring_ctx *ctx, + struct io_kiocb *req, + unsigned int sqe_flags) +{ + if (!ctx->restricted) + return true; + + if (!test_bit(req->opcode, ctx->restrictions.sqe_op)) + return false; + + if ((sqe_flags & ctx->restrictions.sqe_flags_required) != + ctx->restrictions.sqe_flags_required) + return false; + + if (sqe_flags & ~(ctx->restrictions.sqe_flags_allowed | + ctx->restrictions.sqe_flags_required)) + return false; + + return true; +} + #define SQE_VALID_FLAGS (IOSQE_FIXED_FILE|IOSQE_IO_DRAIN|IOSQE_IO_LINK| \ IOSQE_IO_HARDLINK | IOSQE_ASYNC | \ IOSQE_BUFFER_SELECT) @@ -6473,6 +6510,9 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req, if (unlikely(sqe_flags & ~SQE_VALID_FLAGS)) return -EINVAL; + if (unlikely(!io_check_restriction(ctx, req, sqe_flags))) + return -EACCES; + if ((sqe_flags & IOSQE_BUFFER_SELECT) && !io_op_defs[req->opcode].buffer_select) return -EOPNOTSUPP; @@ -9077,6 +9117,72 @@ static int io_unregister_personality(struct io_ring_ctx *ctx, unsigned id) return -EINVAL; } +static int io_register_restrictions(struct io_ring_ctx *ctx, void __user *arg, + unsigned int nr_args) +{ + struct io_uring_restriction *res; + size_t size; + int i, ret; + + /* We allow only a single restrictions registration */ + if (ctx->restricted) + return -EBUSY; + + if (!arg || nr_args > IORING_MAX_RESTRICTIONS) + return -EINVAL; + + size = array_size(nr_args, sizeof(*res)); + if (size == SIZE_MAX) + return -EOVERFLOW; + + res = memdup_user(arg, size); + if (IS_ERR(res)) + return PTR_ERR(res); + + ret = 0; + + for (i = 0; i < nr_args; i++) { + switch (res[i].opcode) { + case IORING_RESTRICTION_REGISTER_OP: + if (res[i].register_op >= IORING_REGISTER_LAST) { + ret = -EINVAL; + goto out; + } + + __set_bit(res[i].register_op, + ctx->restrictions.register_op); + break; + case IORING_RESTRICTION_SQE_OP: + if (res[i].sqe_op >= IORING_OP_LAST) { + ret = -EINVAL; + goto out; + } + + __set_bit(res[i].sqe_op, ctx->restrictions.sqe_op); + break; + case IORING_RESTRICTION_SQE_FLAGS_ALLOWED: + ctx->restrictions.sqe_flags_allowed = res[i].sqe_flags; + break; + case IORING_RESTRICTION_SQE_FLAGS_REQUIRED: + ctx->restrictions.sqe_flags_required = res[i].sqe_flags; + break; + default: + ret = -EINVAL; + goto out; + } + } + +out: + /* Reset all restrictions if an error happened */ + if (ret != 0) + memset(&ctx->restrictions, 0, sizeof(ctx->restrictions)); + else + ctx->restricted = 1; + + kfree(res); + return ret; +} + static bool io_register_op_must_quiesce(int op) { switch (op) { @@ -9123,6 +9229,18 @@ static int __io_uring_register(struct io_ring_ctx *ctx, unsigned opcode, if (ret) { percpu_ref_resurrect(&ctx->refs); ret = -EINTR; + goto out_quiesce; + } + } + + if (ctx->restricted) { + if (opcode >= IORING_REGISTER_LAST) { + ret = -EINVAL; + goto out; + } + + if (!test_bit(opcode, ctx->restrictions.register_op)) { + ret = -EACCES; goto out; } } @@ -9186,15 +9304,19 @@ static int __io_uring_register(struct io_ring_ctx *ctx, unsigned opcode, break; ret = io_unregister_personality(ctx, nr_args); break; + case IORING_REGISTER_RESTRICTIONS: + ret = io_register_restrictions(ctx, arg, nr_args); + break; default: ret = -EINVAL; break; } +out: if (io_register_op_must_quiesce(opcode)) { /* bring the ctx back to life */ percpu_ref_reinit(&ctx->refs); -out: +out_quiesce: reinit_completion(&ctx->ref_comp); } return ret; diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h index 5f12ae6a415c..6e7f2e5e917b 100644 --- a/include/uapi/linux/io_uring.h +++ b/include/uapi/linux/io_uring.h @@ -267,6 +267,7 @@ enum { IORING_REGISTER_PROBE = 8, IORING_REGISTER_PERSONALITY = 9, IORING_UNREGISTER_PERSONALITY = 10, + IORING_REGISTER_RESTRICTIONS = 11, /* this goes last */ IORING_REGISTER_LAST @@ -295,4 +296,34 @@ struct io_uring_probe { struct io_uring_probe_op ops[0]; }; +struct io_uring_restriction { + __u16 opcode; + union { + __u8 register_op; /* IORING_RESTRICTION_REGISTER_OP */ + __u8 sqe_op; /* IORING_RESTRICTION_SQE_OP */ + __u8 sqe_flags; /* IORING_RESTRICTION_SQE_FLAGS_* */ + }; + __u8 resv; + __u32 resv2[3]; +}; + +/* + * io_uring_restriction->opcode values + */ +enum { + /* Allow an io_uring_register(2) opcode */ + IORING_RESTRICTION_REGISTER_OP = 0, + + /* Allow an sqe opcode */ + IORING_RESTRICTION_SQE_OP = 1, + + /* Allow sqe flags */ + IORING_RESTRICTION_SQE_FLAGS_ALLOWED = 2, + + /* Require sqe flags (these flags must be set on each submission) */ + IORING_RESTRICTION_SQE_FLAGS_REQUIRED = 3, + + IORING_RESTRICTION_LAST +}; + #endif -- cgit v1.2.3-58-ga151 From 7e84e1c7566a1df470a9e1f49d3db2ce311261a4 Mon Sep 17 00:00:00 2001 From: Stefano Garzarella Date: Thu, 27 Aug 2020 16:58:31 +0200 Subject: io_uring: allow disabling rings during the creation This patch adds a new IORING_SETUP_R_DISABLED flag to start the rings disabled, allowing the user to register restrictions, buffers, files, before to start processing SQEs. When IORING_SETUP_R_DISABLED is set, SQE are not processed and SQPOLL kthread is not started. The restrictions registration are allowed only when the rings are disable to prevent concurrency issue while processing SQEs. The rings can be enabled using IORING_REGISTER_ENABLE_RINGS opcode with io_uring_register(2). Suggested-by: Jens Axboe Signed-off-by: Stefano Garzarella Reviewed-by: Kees Cook Signed-off-by: Jens Axboe --- fs/io_uring.c | 61 ++++++++++++++++++++++++++++++++++++++----- include/uapi/linux/io_uring.h | 2 ++ 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index c4855cecc8f3..b8fdb10c23e3 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -227,6 +227,7 @@ struct io_restriction { DECLARE_BITMAP(sqe_op, IORING_OP_LAST); u8 sqe_flags_allowed; u8 sqe_flags_required; + bool registered; }; struct io_ring_ctx { @@ -6910,6 +6911,14 @@ static int io_sqe_files_unregister(struct io_ring_ctx *ctx) static void io_sq_thread_stop(struct io_ring_ctx *ctx) { if (ctx->sqo_thread) { + /* + * We may arrive here from the error branch in + * io_sq_offload_create() where the kthread is created + * without being waked up, thus wake it up now to make + * sure the wait will complete. + */ + wake_up_process(ctx->sqo_thread); + wait_for_completion(&ctx->sq_thread_comp); /* * The park is a bit of a work-around, without it we get @@ -7581,8 +7590,8 @@ void __io_uring_free(struct task_struct *tsk) tsk->io_uring = NULL; } -static int io_sq_offload_start(struct io_ring_ctx *ctx, - struct io_uring_params *p) +static int io_sq_offload_create(struct io_ring_ctx *ctx, + struct io_uring_params *p) { int ret; @@ -7619,7 +7628,6 @@ static int io_sq_offload_start(struct io_ring_ctx *ctx, ret = io_uring_alloc_task_context(ctx->sqo_thread); if (ret) goto err; - wake_up_process(ctx->sqo_thread); } else if (p->flags & IORING_SETUP_SQ_AFF) { /* Can't have SQ_AFF without SQPOLL */ ret = -EINVAL; @@ -7636,6 +7644,12 @@ err: return ret; } +static void io_sq_offload_start(struct io_ring_ctx *ctx) +{ + if ((ctx->flags & IORING_SETUP_SQPOLL) && ctx->sqo_thread) + wake_up_process(ctx->sqo_thread); +} + static inline void __io_unaccount_mem(struct user_struct *user, unsigned long nr_pages) { @@ -8633,6 +8647,10 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit, if (!percpu_ref_tryget(&ctx->refs)) goto out_fput; + ret = -EBADFD; + if (ctx->flags & IORING_SETUP_R_DISABLED) + goto out; + /* * For SQ polling, the thread will do all submissions and completions. * Just return the requested submit count, and wake the thread if @@ -8975,10 +8993,13 @@ static int io_uring_create(unsigned entries, struct io_uring_params *p, if (ret) goto err; - ret = io_sq_offload_start(ctx, p); + ret = io_sq_offload_create(ctx, p); if (ret) goto err; + if (!(p->flags & IORING_SETUP_R_DISABLED)) + io_sq_offload_start(ctx); + memset(&p->sq_off, 0, sizeof(p->sq_off)); p->sq_off.head = offsetof(struct io_rings, sq.head); p->sq_off.tail = offsetof(struct io_rings, sq.tail); @@ -9041,7 +9062,8 @@ static long io_uring_setup(u32 entries, struct io_uring_params __user *params) if (p.flags & ~(IORING_SETUP_IOPOLL | IORING_SETUP_SQPOLL | IORING_SETUP_SQ_AFF | IORING_SETUP_CQSIZE | - IORING_SETUP_CLAMP | IORING_SETUP_ATTACH_WQ)) + IORING_SETUP_CLAMP | IORING_SETUP_ATTACH_WQ | + IORING_SETUP_R_DISABLED)) return -EINVAL; return io_uring_create(entries, &p, params); @@ -9124,8 +9146,12 @@ static int io_register_restrictions(struct io_ring_ctx *ctx, void __user *arg, size_t size; int i, ret; + /* Restrictions allowed only if rings started disabled */ + if (!(ctx->flags & IORING_SETUP_R_DISABLED)) + return -EBADFD; + /* We allow only a single restrictions registration */ - if (ctx->restricted) + if (ctx->restrictions.registered) return -EBUSY; if (!arg || nr_args > IORING_MAX_RESTRICTIONS) @@ -9177,12 +9203,27 @@ out: if (ret != 0) memset(&ctx->restrictions, 0, sizeof(ctx->restrictions)); else - ctx->restricted = 1; + ctx->restrictions.registered = true; kfree(res); return ret; } +static int io_register_enable_rings(struct io_ring_ctx *ctx) +{ + if (!(ctx->flags & IORING_SETUP_R_DISABLED)) + return -EBADFD; + + if (ctx->restrictions.registered) + ctx->restricted = 1; + + ctx->flags &= ~IORING_SETUP_R_DISABLED; + + io_sq_offload_start(ctx); + + return 0; +} + static bool io_register_op_must_quiesce(int op) { switch (op) { @@ -9304,6 +9345,12 @@ static int __io_uring_register(struct io_ring_ctx *ctx, unsigned opcode, break; ret = io_unregister_personality(ctx, nr_args); break; + case IORING_REGISTER_ENABLE_RINGS: + ret = -EINVAL; + if (arg || nr_args) + break; + ret = io_register_enable_rings(ctx); + break; case IORING_REGISTER_RESTRICTIONS: ret = io_register_restrictions(ctx, arg, nr_args); break; diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h index 6e7f2e5e917b..a0c85e0e9016 100644 --- a/include/uapi/linux/io_uring.h +++ b/include/uapi/linux/io_uring.h @@ -95,6 +95,7 @@ enum { #define IORING_SETUP_CQSIZE (1U << 3) /* app defines CQ size */ #define IORING_SETUP_CLAMP (1U << 4) /* clamp SQ/CQ ring sizes */ #define IORING_SETUP_ATTACH_WQ (1U << 5) /* attach to existing wq */ +#define IORING_SETUP_R_DISABLED (1U << 6) /* start with ring disabled */ enum { IORING_OP_NOP, @@ -268,6 +269,7 @@ enum { IORING_REGISTER_PERSONALITY = 9, IORING_UNREGISTER_PERSONALITY = 10, IORING_REGISTER_RESTRICTIONS = 11, + IORING_REGISTER_ENABLE_RINGS = 12, /* this goes last */ IORING_REGISTER_LAST -- cgit v1.2.3-58-ga151 From 95da84659226d75698a1ab958be0af21d9cc2a9c Mon Sep 17 00:00:00 2001 From: Sebastian Andrzej Siewior Date: Tue, 1 Sep 2020 10:41:46 +0200 Subject: io_wq: Make io_wqe::lock a raw_spinlock_t During a context switch the scheduler invokes wq_worker_sleeping() with disabled preemption. Disabling preemption is needed because it protects access to `worker->sleeping'. As an optimisation it avoids invoking schedule() within the schedule path as part of possible wake up (thus preempt_enable_no_resched() afterwards). The io-wq has been added to the mix in the same section with disabled preemption. This breaks on PREEMPT_RT because io_wq_worker_sleeping() acquires a spinlock_t. Also within the schedule() the spinlock_t must be acquired after tsk_is_pi_blocked() otherwise it will block on the sleeping lock again while scheduling out. While playing with `io_uring-bench' I didn't notice a significant latency spike after converting io_wqe::lock to a raw_spinlock_t. The latency was more or less the same. In order to keep the spinlock_t it would have to be moved after the tsk_is_pi_blocked() check which would introduce a branch instruction into the hot path. The lock is used to maintain the `work_list' and wakes one task up at most. Should io_wqe_cancel_pending_work() cause latency spikes, while searching for a specific item, then it would need to drop the lock during iterations. revert_creds() is also invoked under the lock. According to debug cred::non_rcu is 0. Otherwise it should be moved outside of the locked section because put_cred_rcu()->free_uid() acquires a sleeping lock. Convert io_wqe::lock to a raw_spinlock_t.c Signed-off-by: Sebastian Andrzej Siewior Signed-off-by: Jens Axboe --- fs/io-wq.c | 52 ++++++++++++++++++++++++++-------------------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/fs/io-wq.c b/fs/io-wq.c index 09e20bbf0d37..c1a7ef85844b 100644 --- a/fs/io-wq.c +++ b/fs/io-wq.c @@ -88,7 +88,7 @@ enum { */ struct io_wqe { struct { - spinlock_t lock; + raw_spinlock_t lock; struct io_wq_work_list work_list; unsigned long hash_map; unsigned flags; @@ -149,7 +149,7 @@ static bool __io_worker_unuse(struct io_wqe *wqe, struct io_worker *worker) if (current->files != worker->restore_files) { __acquire(&wqe->lock); - spin_unlock_irq(&wqe->lock); + raw_spin_unlock_irq(&wqe->lock); dropped_lock = true; task_lock(current); @@ -168,7 +168,7 @@ static bool __io_worker_unuse(struct io_wqe *wqe, struct io_worker *worker) if (worker->mm) { if (!dropped_lock) { __acquire(&wqe->lock); - spin_unlock_irq(&wqe->lock); + raw_spin_unlock_irq(&wqe->lock); dropped_lock = true; } __set_current_state(TASK_RUNNING); @@ -222,17 +222,17 @@ static void io_worker_exit(struct io_worker *worker) worker->flags = 0; preempt_enable(); - spin_lock_irq(&wqe->lock); + raw_spin_lock_irq(&wqe->lock); hlist_nulls_del_rcu(&worker->nulls_node); list_del_rcu(&worker->all_list); if (__io_worker_unuse(wqe, worker)) { __release(&wqe->lock); - spin_lock_irq(&wqe->lock); + raw_spin_lock_irq(&wqe->lock); } acct->nr_workers--; nr_workers = wqe->acct[IO_WQ_ACCT_BOUND].nr_workers + wqe->acct[IO_WQ_ACCT_UNBOUND].nr_workers; - spin_unlock_irq(&wqe->lock); + raw_spin_unlock_irq(&wqe->lock); /* all workers gone, wq exit can proceed */ if (!nr_workers && refcount_dec_and_test(&wqe->wq->refs)) @@ -508,7 +508,7 @@ get_next: else if (!wq_list_empty(&wqe->work_list)) wqe->flags |= IO_WQE_FLAG_STALLED; - spin_unlock_irq(&wqe->lock); + raw_spin_unlock_irq(&wqe->lock); if (!work) break; io_assign_current_work(worker, work); @@ -542,17 +542,17 @@ get_next: io_wqe_enqueue(wqe, linked); if (hash != -1U && !next_hashed) { - spin_lock_irq(&wqe->lock); + raw_spin_lock_irq(&wqe->lock); wqe->hash_map &= ~BIT_ULL(hash); wqe->flags &= ~IO_WQE_FLAG_STALLED; /* skip unnecessary unlock-lock wqe->lock */ if (!work) goto get_next; - spin_unlock_irq(&wqe->lock); + raw_spin_unlock_irq(&wqe->lock); } } while (work); - spin_lock_irq(&wqe->lock); + raw_spin_lock_irq(&wqe->lock); } while (1); } @@ -567,7 +567,7 @@ static int io_wqe_worker(void *data) while (!test_bit(IO_WQ_BIT_EXIT, &wq->state)) { set_current_state(TASK_INTERRUPTIBLE); loop: - spin_lock_irq(&wqe->lock); + raw_spin_lock_irq(&wqe->lock); if (io_wqe_run_queue(wqe)) { __set_current_state(TASK_RUNNING); io_worker_handle_work(worker); @@ -578,7 +578,7 @@ loop: __release(&wqe->lock); goto loop; } - spin_unlock_irq(&wqe->lock); + raw_spin_unlock_irq(&wqe->lock); if (signal_pending(current)) flush_signals(current); if (schedule_timeout(WORKER_IDLE_TIMEOUT)) @@ -590,11 +590,11 @@ loop: } if (test_bit(IO_WQ_BIT_EXIT, &wq->state)) { - spin_lock_irq(&wqe->lock); + raw_spin_lock_irq(&wqe->lock); if (!wq_list_empty(&wqe->work_list)) io_worker_handle_work(worker); else - spin_unlock_irq(&wqe->lock); + raw_spin_unlock_irq(&wqe->lock); } io_worker_exit(worker); @@ -634,9 +634,9 @@ void io_wq_worker_sleeping(struct task_struct *tsk) worker->flags &= ~IO_WORKER_F_RUNNING; - spin_lock_irq(&wqe->lock); + raw_spin_lock_irq(&wqe->lock); io_wqe_dec_running(wqe, worker); - spin_unlock_irq(&wqe->lock); + raw_spin_unlock_irq(&wqe->lock); } static bool create_io_worker(struct io_wq *wq, struct io_wqe *wqe, int index) @@ -660,7 +660,7 @@ static bool create_io_worker(struct io_wq *wq, struct io_wqe *wqe, int index) return false; } - spin_lock_irq(&wqe->lock); + raw_spin_lock_irq(&wqe->lock); hlist_nulls_add_head_rcu(&worker->nulls_node, &wqe->free_list); list_add_tail_rcu(&worker->all_list, &wqe->all_list); worker->flags |= IO_WORKER_F_FREE; @@ -669,7 +669,7 @@ static bool create_io_worker(struct io_wq *wq, struct io_wqe *wqe, int index) if (!acct->nr_workers && (worker->flags & IO_WORKER_F_BOUND)) worker->flags |= IO_WORKER_F_FIXED; acct->nr_workers++; - spin_unlock_irq(&wqe->lock); + raw_spin_unlock_irq(&wqe->lock); if (index == IO_WQ_ACCT_UNBOUND) atomic_inc(&wq->user->processes); @@ -724,12 +724,12 @@ static int io_wq_manager(void *data) if (!node_online(node)) continue; - spin_lock_irq(&wqe->lock); + raw_spin_lock_irq(&wqe->lock); if (io_wqe_need_worker(wqe, IO_WQ_ACCT_BOUND)) fork_worker[IO_WQ_ACCT_BOUND] = true; if (io_wqe_need_worker(wqe, IO_WQ_ACCT_UNBOUND)) fork_worker[IO_WQ_ACCT_UNBOUND] = true; - spin_unlock_irq(&wqe->lock); + raw_spin_unlock_irq(&wqe->lock); if (fork_worker[IO_WQ_ACCT_BOUND]) create_io_worker(wq, wqe, IO_WQ_ACCT_BOUND); if (fork_worker[IO_WQ_ACCT_UNBOUND]) @@ -825,10 +825,10 @@ static void io_wqe_enqueue(struct io_wqe *wqe, struct io_wq_work *work) } work_flags = work->flags; - spin_lock_irqsave(&wqe->lock, flags); + raw_spin_lock_irqsave(&wqe->lock, flags); io_wqe_insert_work(wqe, work); wqe->flags &= ~IO_WQE_FLAG_STALLED; - spin_unlock_irqrestore(&wqe->lock, flags); + raw_spin_unlock_irqrestore(&wqe->lock, flags); if ((work_flags & IO_WQ_WORK_CONCURRENT) || !atomic_read(&acct->nr_running)) @@ -955,13 +955,13 @@ static void io_wqe_cancel_pending_work(struct io_wqe *wqe, unsigned long flags; retry: - spin_lock_irqsave(&wqe->lock, flags); + raw_spin_lock_irqsave(&wqe->lock, flags); wq_list_for_each(node, prev, &wqe->work_list) { work = container_of(node, struct io_wq_work, list); if (!match->fn(work, match->data)) continue; io_wqe_remove_pending(wqe, work, prev); - spin_unlock_irqrestore(&wqe->lock, flags); + raw_spin_unlock_irqrestore(&wqe->lock, flags); io_run_cancel(work, wqe); match->nr_pending++; if (!match->cancel_all) @@ -970,7 +970,7 @@ retry: /* not safe to continue after unlock */ goto retry; } - spin_unlock_irqrestore(&wqe->lock, flags); + raw_spin_unlock_irqrestore(&wqe->lock, flags); } static void io_wqe_cancel_running_work(struct io_wqe *wqe, @@ -1078,7 +1078,7 @@ struct io_wq *io_wq_create(unsigned bounded, struct io_wq_data *data) } atomic_set(&wqe->acct[IO_WQ_ACCT_UNBOUND].nr_running, 0); wqe->wq = wq; - spin_lock_init(&wqe->lock); + raw_spin_lock_init(&wqe->lock); INIT_WQ_LIST(&wqe->work_list); INIT_HLIST_NULLS_HEAD(&wqe->free_list, 0); INIT_LIST_HEAD(&wqe->all_list); -- cgit v1.2.3-58-ga151 From e35afcf91230238dc27f98d1cd7cb787474b28cb Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Wed, 2 Sep 2020 12:44:20 -0600 Subject: io_uring: io_sq_thread() doesn't need to flush signals We're not handling signals by default in kernel threads, and we never use TWA_SIGNAL for the SQPOLL thread internally. Hence we can never have a signal pending, and we don't need to check for it (nor flush it). Signed-off-by: Jens Axboe --- fs/io_uring.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index b8fdb10c23e3..26f819ae177d 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -6709,8 +6709,6 @@ static int io_sq_thread(void *data) io_ring_clear_wakeup_flag(ctx); continue; } - if (signal_pending(current)) - flush_signals(current); schedule(); finish_wait(&ctx->sqo_wait, &wait); -- cgit v1.2.3-58-ga151 From ce71bfea207b4d7c21d36f24ec37618ffcea1da8 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Mon, 31 Aug 2020 12:08:10 -0600 Subject: fs: align IOCB_* flags with RWF_* flags We have a set of flags that are shared between the two and inherired in kiocb_set_rw_flags(), but we check and set these individually. Reorder the IOCB flags so that the bottom part of the space is synced with the RWF flag space, and then we can do them all in one mask and set operation. The only exception is RWF_SYNC, which needs to mark IOCB_SYNC and IOCB_DSYNC. Do that one separately. This shaves 15 bytes of text from kiocb_set_rw_flags() for me. Suggested-by: Matthew Wilcox (Oracle) Signed-off-by: Jens Axboe --- include/linux/fs.h | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/include/linux/fs.h b/include/linux/fs.h index 1c4068428461..4ebc14dee421 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -310,17 +310,20 @@ enum rw_hint { WRITE_LIFE_EXTREME = RWH_WRITE_LIFE_EXTREME, }; -#define IOCB_EVENTFD (1 << 0) -#define IOCB_APPEND (1 << 1) -#define IOCB_DIRECT (1 << 2) -#define IOCB_HIPRI (1 << 3) -#define IOCB_DSYNC (1 << 4) -#define IOCB_SYNC (1 << 5) -#define IOCB_WRITE (1 << 6) -#define IOCB_NOWAIT (1 << 7) +/* Match RWF_* bits to IOCB bits */ +#define IOCB_HIPRI (__force int) RWF_HIPRI +#define IOCB_DSYNC (__force int) RWF_DSYNC +#define IOCB_SYNC (__force int) RWF_SYNC +#define IOCB_NOWAIT (__force int) RWF_NOWAIT +#define IOCB_APPEND (__force int) RWF_APPEND + +/* non-RWF related bits - start at 16 */ +#define IOCB_EVENTFD (1 << 16) +#define IOCB_DIRECT (1 << 17) +#define IOCB_WRITE (1 << 18) /* iocb->ki_waitq is valid */ -#define IOCB_WAITQ (1 << 8) -#define IOCB_NOIO (1 << 9) +#define IOCB_WAITQ (1 << 19) +#define IOCB_NOIO (1 << 20) struct kiocb { struct file *ki_filp; @@ -3317,6 +3320,9 @@ static inline int kiocb_set_rw_flags(struct kiocb *ki, rwf_t flags) { int kiocb_flags = 0; + /* make sure there's no overlap between RWF and private IOCB flags */ + BUILD_BUG_ON((__force int) RWF_SUPPORTED & IOCB_EVENTFD); + if (!flags) return 0; if (unlikely(flags & ~RWF_SUPPORTED)) @@ -3325,16 +3331,11 @@ static inline int kiocb_set_rw_flags(struct kiocb *ki, rwf_t flags) if (flags & RWF_NOWAIT) { if (!(ki->ki_filp->f_mode & FMODE_NOWAIT)) return -EOPNOTSUPP; - kiocb_flags |= IOCB_NOWAIT | IOCB_NOIO; + kiocb_flags |= IOCB_NOIO; } - if (flags & RWF_HIPRI) - kiocb_flags |= IOCB_HIPRI; - if (flags & RWF_DSYNC) - kiocb_flags |= IOCB_DSYNC; + kiocb_flags |= (__force int) (flags & RWF_SUPPORTED); if (flags & RWF_SYNC) - kiocb_flags |= (IOCB_DSYNC | IOCB_SYNC); - if (flags & RWF_APPEND) - kiocb_flags |= IOCB_APPEND; + kiocb_flags |= IOCB_DSYNC; ki->ki_flags |= kiocb_flags; return 0; -- cgit v1.2.3-58-ga151 From 6a7793828fb21c1e0c119d98c2f898d8f83c8a8b Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Wed, 2 Sep 2020 12:21:41 -0600 Subject: io_uring: use private ctx wait queue entries for SQPOLL This is in preparation to sharing the poller thread between rings. For that we need per-ring wait_queue_entry storage, and we can't easily put that on the stack if one thread is managing multiple rings. We'll also be sharing the wait_queue_head across rings for the purposes of wakeups, provide the usual private ring wait_queue_head for now but make it a pointer so we can easily override it when sharing. Signed-off-by: Jens Axboe --- fs/io_uring.c | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 26f819ae177d..128ffa79d9d3 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -287,7 +287,9 @@ struct io_ring_ctx { /* Only used for accounting purposes */ struct mm_struct *mm_account; - wait_queue_head_t sqo_wait; + struct wait_queue_head *sqo_wait; + struct wait_queue_head __sqo_wait; + struct wait_queue_entry sqo_wait_entry; /* * If used, fixed file set. Writers must ensure that ->refs is dead, @@ -1057,7 +1059,8 @@ static struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p) goto err; ctx->flags = p->flags; - init_waitqueue_head(&ctx->sqo_wait); + init_waitqueue_head(&ctx->__sqo_wait); + ctx->sqo_wait = &ctx->__sqo_wait; init_waitqueue_head(&ctx->cq_wait); INIT_LIST_HEAD(&ctx->cq_overflow_list); init_completion(&ctx->ref_comp); @@ -1340,8 +1343,8 @@ static void io_cqring_ev_posted(struct io_ring_ctx *ctx) { if (waitqueue_active(&ctx->wait)) wake_up(&ctx->wait); - if (waitqueue_active(&ctx->sqo_wait)) - wake_up(&ctx->sqo_wait); + if (waitqueue_active(ctx->sqo_wait)) + wake_up(ctx->sqo_wait); if (io_should_trigger_evfd(ctx)) eventfd_signal(ctx->cq_ev_fd, 1); } @@ -2448,9 +2451,8 @@ static void io_iopoll_req_issued(struct io_kiocb *req) else list_add_tail(&req->inflight_entry, &ctx->iopoll_list); - if ((ctx->flags & IORING_SETUP_SQPOLL) && - wq_has_sleeper(&ctx->sqo_wait)) - wake_up(&ctx->sqo_wait); + if ((ctx->flags & IORING_SETUP_SQPOLL) && wq_has_sleeper(ctx->sqo_wait)) + wake_up(ctx->sqo_wait); } static void __io_state_file_put(struct io_submit_state *state) @@ -6627,10 +6629,11 @@ static int io_sq_thread(void *data) { struct io_ring_ctx *ctx = data; const struct cred *old_cred; - DEFINE_WAIT(wait); unsigned long timeout; int ret = 0; + init_wait(&ctx->sqo_wait_entry); + complete(&ctx->sq_thread_comp); old_cred = override_creds(ctx->creds); @@ -6680,7 +6683,7 @@ static int io_sq_thread(void *data) continue; } - prepare_to_wait(&ctx->sqo_wait, &wait, + prepare_to_wait(ctx->sqo_wait, &ctx->sqo_wait_entry, TASK_INTERRUPTIBLE); /* @@ -6692,7 +6695,7 @@ static int io_sq_thread(void *data) */ if ((ctx->flags & IORING_SETUP_IOPOLL) && !list_empty_careful(&ctx->iopoll_list)) { - finish_wait(&ctx->sqo_wait, &wait); + finish_wait(ctx->sqo_wait, &ctx->sqo_wait_entry); continue; } @@ -6701,22 +6704,22 @@ static int io_sq_thread(void *data) to_submit = io_sqring_entries(ctx); if (!to_submit || ret == -EBUSY) { if (kthread_should_park()) { - finish_wait(&ctx->sqo_wait, &wait); + finish_wait(ctx->sqo_wait, &ctx->sqo_wait_entry); break; } if (io_run_task_work()) { - finish_wait(&ctx->sqo_wait, &wait); + finish_wait(ctx->sqo_wait, &ctx->sqo_wait_entry); io_ring_clear_wakeup_flag(ctx); continue; } schedule(); - finish_wait(&ctx->sqo_wait, &wait); + finish_wait(ctx->sqo_wait, &ctx->sqo_wait_entry); io_ring_clear_wakeup_flag(ctx); ret = 0; continue; } - finish_wait(&ctx->sqo_wait, &wait); + finish_wait(ctx->sqo_wait, &ctx->sqo_wait_entry); io_ring_clear_wakeup_flag(ctx); } @@ -8659,7 +8662,7 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit, if (!list_empty_careful(&ctx->cq_overflow_list)) io_cqring_overflow_flush(ctx, false, NULL, NULL); if (flags & IORING_ENTER_SQ_WAKEUP) - wake_up(&ctx->sqo_wait); + wake_up(ctx->sqo_wait); submitted = to_submit; } else if (to_submit) { ret = io_uring_add_task_file(f.file); -- cgit v1.2.3-58-ga151 From 3f0e64d054114b725569c2481bbc6a8eb538bf15 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Wed, 2 Sep 2020 12:42:47 -0600 Subject: io_uring: move SQPOLL post-wakeup ring need wakeup flag into wake handler We need to decouple the clearing on wakeup from the the inline schedule, as that is going to be required for handling multiple rings in one thread. Wrap our wakeup handler so we can clear it when we get the wakeup, by definition that is when we no longer need the flag set. Signed-off-by: Jens Axboe --- fs/io_uring.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 128ffa79d9d3..6d97767c82d5 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -6625,6 +6625,23 @@ static inline void io_ring_clear_wakeup_flag(struct io_ring_ctx *ctx) spin_unlock_irq(&ctx->completion_lock); } +static int io_sq_wake_function(struct wait_queue_entry *wqe, unsigned mode, + int sync, void *key) +{ + struct io_ring_ctx *ctx = container_of(wqe, struct io_ring_ctx, sqo_wait_entry); + int ret; + + ret = autoremove_wake_function(wqe, mode, sync, key); + if (ret) { + unsigned long flags; + + spin_lock_irqsave(&ctx->completion_lock, flags); + ctx->rings->sq_flags &= ~IORING_SQ_NEED_WAKEUP; + spin_unlock_irqrestore(&ctx->completion_lock, flags); + } + return ret; +} + static int io_sq_thread(void *data) { struct io_ring_ctx *ctx = data; @@ -6633,6 +6650,7 @@ static int io_sq_thread(void *data) int ret = 0; init_wait(&ctx->sqo_wait_entry); + ctx->sqo_wait_entry.func = io_sq_wake_function; complete(&ctx->sq_thread_comp); @@ -6715,7 +6733,6 @@ static int io_sq_thread(void *data) schedule(); finish_wait(ctx->sqo_wait, &ctx->sqo_wait_entry); - io_ring_clear_wakeup_flag(ctx); ret = 0; continue; } -- cgit v1.2.3-58-ga151 From c8d1ba583fe67c6b5e054d89f1433498a924286f Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Mon, 14 Sep 2020 11:07:26 -0600 Subject: io_uring: split work handling part of SQPOLL into helper This is done in preparation for handling more than one ctx, but it also cleans up the code a bit since io_sq_thread() was a bit too unwieldy to get a get overview on. __io_sq_thread() is now the main handler, and it returns an enum sq_ret that tells io_sq_thread() what it ended up doing. The parent then makes a decision on idle, spinning, or work handling based on that. Signed-off-by: Jens Axboe --- fs/io_uring.c | 177 ++++++++++++++++++++++++++++++---------------------------- 1 file changed, 93 insertions(+), 84 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 6d97767c82d5..4958e78cd390 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -6642,110 +6642,119 @@ static int io_sq_wake_function(struct wait_queue_entry *wqe, unsigned mode, return ret; } -static int io_sq_thread(void *data) +enum sq_ret { + SQT_IDLE = 1, + SQT_SPIN = 2, + SQT_DID_WORK = 4, +}; + +static enum sq_ret __io_sq_thread(struct io_ring_ctx *ctx, + unsigned long start_jiffies) { - struct io_ring_ctx *ctx = data; - const struct cred *old_cred; - unsigned long timeout; + unsigned long timeout = start_jiffies + ctx->sq_thread_idle; + unsigned int to_submit; int ret = 0; - init_wait(&ctx->sqo_wait_entry); - ctx->sqo_wait_entry.func = io_sq_wake_function; +again: + if (!list_empty(&ctx->iopoll_list)) { + unsigned nr_events = 0; - complete(&ctx->sq_thread_comp); + mutex_lock(&ctx->uring_lock); + if (!list_empty(&ctx->iopoll_list) && !need_resched()) + io_do_iopoll(ctx, &nr_events, 0); + mutex_unlock(&ctx->uring_lock); + } - old_cred = override_creds(ctx->creds); + to_submit = io_sqring_entries(ctx); - timeout = jiffies + ctx->sq_thread_idle; - while (!kthread_should_park()) { - unsigned int to_submit; + /* + * If submit got -EBUSY, flag us as needing the application + * to enter the kernel to reap and flush events. + */ + if (!to_submit || ret == -EBUSY || need_resched()) { + /* + * Drop cur_mm before scheduling, we can't hold it for + * long periods (or over schedule()). Do this before + * adding ourselves to the waitqueue, as the unuse/drop + * may sleep. + */ + io_sq_thread_drop_mm(); - if (!list_empty(&ctx->iopoll_list)) { - unsigned nr_events = 0; + /* + * We're polling. If we're within the defined idle + * period, then let us spin without work before going + * to sleep. The exception is if we got EBUSY doing + * more IO, we should wait for the application to + * reap events and wake us up. + */ + if (!list_empty(&ctx->iopoll_list) || need_resched() || + (!time_after(jiffies, timeout) && ret != -EBUSY && + !percpu_ref_is_dying(&ctx->refs))) + return SQT_SPIN; - mutex_lock(&ctx->uring_lock); - if (!list_empty(&ctx->iopoll_list) && !need_resched()) - io_do_iopoll(ctx, &nr_events, 0); - else - timeout = jiffies + ctx->sq_thread_idle; - mutex_unlock(&ctx->uring_lock); + prepare_to_wait(ctx->sqo_wait, &ctx->sqo_wait_entry, + TASK_INTERRUPTIBLE); + + /* + * While doing polled IO, before going to sleep, we need + * to check if there are new reqs added to iopoll_list, + * it is because reqs may have been punted to io worker + * and will be added to iopoll_list later, hence check + * the iopoll_list again. + */ + if ((ctx->flags & IORING_SETUP_IOPOLL) && + !list_empty_careful(&ctx->iopoll_list)) { + finish_wait(ctx->sqo_wait, &ctx->sqo_wait_entry); + goto again; } + io_ring_set_wakeup_flag(ctx); + to_submit = io_sqring_entries(ctx); + if (!to_submit || ret == -EBUSY) + return SQT_IDLE; + } - /* - * If submit got -EBUSY, flag us as needing the application - * to enter the kernel to reap and flush events. - */ - if (!to_submit || ret == -EBUSY || need_resched()) { - /* - * Drop cur_mm before scheduling, we can't hold it for - * long periods (or over schedule()). Do this before - * adding ourselves to the waitqueue, as the unuse/drop - * may sleep. - */ - io_sq_thread_drop_mm(); + finish_wait(ctx->sqo_wait, &ctx->sqo_wait_entry); + io_ring_clear_wakeup_flag(ctx); - /* - * We're polling. If we're within the defined idle - * period, then let us spin without work before going - * to sleep. The exception is if we got EBUSY doing - * more IO, we should wait for the application to - * reap events and wake us up. - */ - if (!list_empty(&ctx->iopoll_list) || need_resched() || - (!time_after(jiffies, timeout) && ret != -EBUSY && - !percpu_ref_is_dying(&ctx->refs))) { - io_run_task_work(); - cond_resched(); - continue; - } + mutex_lock(&ctx->uring_lock); + if (likely(!percpu_ref_is_dying(&ctx->refs))) + ret = io_submit_sqes(ctx, to_submit); + mutex_unlock(&ctx->uring_lock); + return SQT_DID_WORK; +} - prepare_to_wait(ctx->sqo_wait, &ctx->sqo_wait_entry, - TASK_INTERRUPTIBLE); +static int io_sq_thread(void *data) +{ + struct io_ring_ctx *ctx = data; + const struct cred *old_cred; + unsigned long start_jiffies; - /* - * While doing polled IO, before going to sleep, we need - * to check if there are new reqs added to iopoll_list, - * it is because reqs may have been punted to io worker - * and will be added to iopoll_list later, hence check - * the iopoll_list again. - */ - if ((ctx->flags & IORING_SETUP_IOPOLL) && - !list_empty_careful(&ctx->iopoll_list)) { - finish_wait(ctx->sqo_wait, &ctx->sqo_wait_entry); - continue; - } + init_wait(&ctx->sqo_wait_entry); + ctx->sqo_wait_entry.func = io_sq_wake_function; - io_ring_set_wakeup_flag(ctx); + complete(&ctx->sq_thread_comp); - to_submit = io_sqring_entries(ctx); - if (!to_submit || ret == -EBUSY) { - if (kthread_should_park()) { - finish_wait(ctx->sqo_wait, &ctx->sqo_wait_entry); - break; - } - if (io_run_task_work()) { - finish_wait(ctx->sqo_wait, &ctx->sqo_wait_entry); - io_ring_clear_wakeup_flag(ctx); - continue; - } - schedule(); - finish_wait(ctx->sqo_wait, &ctx->sqo_wait_entry); + old_cred = override_creds(ctx->creds); - ret = 0; - continue; - } - finish_wait(ctx->sqo_wait, &ctx->sqo_wait_entry); + start_jiffies = jiffies; + while (!kthread_should_park()) { + enum sq_ret ret; - io_ring_clear_wakeup_flag(ctx); + ret = __io_sq_thread(ctx, start_jiffies); + switch (ret) { + case SQT_IDLE: + schedule(); + start_jiffies = jiffies; + continue; + case SQT_SPIN: + io_run_task_work(); + cond_resched(); + fallthrough; + case SQT_DID_WORK: + continue; } - - mutex_lock(&ctx->uring_lock); - if (likely(!percpu_ref_is_dying(&ctx->refs))) - ret = io_submit_sqes(ctx, to_submit); - mutex_unlock(&ctx->uring_lock); - timeout = jiffies + ctx->sq_thread_idle; } io_run_task_work(); -- cgit v1.2.3-58-ga151 From 534ca6d684f1feaf2edd90e641129725cba7e86d Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Wed, 2 Sep 2020 13:52:19 -0600 Subject: io_uring: split SQPOLL data into separate structure Move all the necessary state out of io_ring_ctx, and into a new structure, io_sq_data. The latter now deals with any state or variables associated with the SQPOLL thread itself. In preparation for supporting more than one io_ring_ctx per SQPOLL thread. Signed-off-by: Jens Axboe --- fs/io_uring.c | 127 +++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 86 insertions(+), 41 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 4958e78cd390..9a7c645f971c 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -230,6 +230,12 @@ struct io_restriction { bool registered; }; +struct io_sq_data { + refcount_t refs; + struct task_struct *thread; + struct wait_queue_head wait; +}; + struct io_ring_ctx { struct { struct percpu_ref refs; @@ -276,7 +282,6 @@ struct io_ring_ctx { /* IO offload */ struct io_wq *io_wq; - struct task_struct *sqo_thread; /* if using sq thread polling */ /* * For SQPOLL usage - we hold a reference to the parent task, so we @@ -287,8 +292,8 @@ struct io_ring_ctx { /* Only used for accounting purposes */ struct mm_struct *mm_account; - struct wait_queue_head *sqo_wait; - struct wait_queue_head __sqo_wait; + struct io_sq_data *sq_data; /* if using sq thread polling */ + struct wait_queue_entry sqo_wait_entry; /* @@ -1059,8 +1064,6 @@ static struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p) goto err; ctx->flags = p->flags; - init_waitqueue_head(&ctx->__sqo_wait); - ctx->sqo_wait = &ctx->__sqo_wait; init_waitqueue_head(&ctx->cq_wait); INIT_LIST_HEAD(&ctx->cq_overflow_list); init_completion(&ctx->ref_comp); @@ -1238,8 +1241,10 @@ static bool io_task_match(struct io_kiocb *req, struct task_struct *tsk) if (!tsk || req->task == tsk) return true; - if ((ctx->flags & IORING_SETUP_SQPOLL) && req->task == ctx->sqo_thread) - return true; + if (ctx->flags & IORING_SETUP_SQPOLL) { + if (ctx->sq_data && req->task == ctx->sq_data->thread) + return true; + } return false; } @@ -1343,8 +1348,8 @@ static void io_cqring_ev_posted(struct io_ring_ctx *ctx) { if (waitqueue_active(&ctx->wait)) wake_up(&ctx->wait); - if (waitqueue_active(ctx->sqo_wait)) - wake_up(ctx->sqo_wait); + if (ctx->sq_data && waitqueue_active(&ctx->sq_data->wait)) + wake_up(&ctx->sq_data->wait); if (io_should_trigger_evfd(ctx)) eventfd_signal(ctx->cq_ev_fd, 1); } @@ -2451,8 +2456,9 @@ static void io_iopoll_req_issued(struct io_kiocb *req) else list_add_tail(&req->inflight_entry, &ctx->iopoll_list); - if ((ctx->flags & IORING_SETUP_SQPOLL) && wq_has_sleeper(ctx->sqo_wait)) - wake_up(ctx->sqo_wait); + if ((ctx->flags & IORING_SETUP_SQPOLL) && + wq_has_sleeper(&ctx->sq_data->wait)) + wake_up(&ctx->sq_data->wait); } static void __io_state_file_put(struct io_submit_state *state) @@ -6652,6 +6658,7 @@ static enum sq_ret __io_sq_thread(struct io_ring_ctx *ctx, unsigned long start_jiffies) { unsigned long timeout = start_jiffies + ctx->sq_thread_idle; + struct io_sq_data *sqd = ctx->sq_data; unsigned int to_submit; int ret = 0; @@ -6692,7 +6699,7 @@ again: !percpu_ref_is_dying(&ctx->refs))) return SQT_SPIN; - prepare_to_wait(ctx->sqo_wait, &ctx->sqo_wait_entry, + prepare_to_wait(&sqd->wait, &ctx->sqo_wait_entry, TASK_INTERRUPTIBLE); /* @@ -6704,7 +6711,7 @@ again: */ if ((ctx->flags & IORING_SETUP_IOPOLL) && !list_empty_careful(&ctx->iopoll_list)) { - finish_wait(ctx->sqo_wait, &ctx->sqo_wait_entry); + finish_wait(&sqd->wait, &ctx->sqo_wait_entry); goto again; } @@ -6715,7 +6722,7 @@ again: return SQT_IDLE; } - finish_wait(ctx->sqo_wait, &ctx->sqo_wait_entry); + finish_wait(&sqd->wait, &ctx->sqo_wait_entry); io_ring_clear_wakeup_flag(ctx); mutex_lock(&ctx->uring_lock); @@ -6935,26 +6942,54 @@ static int io_sqe_files_unregister(struct io_ring_ctx *ctx) return 0; } -static void io_sq_thread_stop(struct io_ring_ctx *ctx) +static void io_put_sq_data(struct io_sq_data *sqd) { - if (ctx->sqo_thread) { - /* - * We may arrive here from the error branch in - * io_sq_offload_create() where the kthread is created - * without being waked up, thus wake it up now to make - * sure the wait will complete. - */ - wake_up_process(ctx->sqo_thread); - - wait_for_completion(&ctx->sq_thread_comp); + if (refcount_dec_and_test(&sqd->refs)) { /* * The park is a bit of a work-around, without it we get * warning spews on shutdown with SQPOLL set and affinity * set to a single CPU. */ - kthread_park(ctx->sqo_thread); - kthread_stop(ctx->sqo_thread); - ctx->sqo_thread = NULL; + if (sqd->thread) { + kthread_park(sqd->thread); + kthread_stop(sqd->thread); + } + + kfree(sqd); + } +} + +static struct io_sq_data *io_get_sq_data(struct io_uring_params *p) +{ + struct io_sq_data *sqd; + + sqd = kzalloc(sizeof(*sqd), GFP_KERNEL); + if (!sqd) + return ERR_PTR(-ENOMEM); + + refcount_set(&sqd->refs, 1); + init_waitqueue_head(&sqd->wait); + return sqd; +} + +static void io_sq_thread_stop(struct io_ring_ctx *ctx) +{ + struct io_sq_data *sqd = ctx->sq_data; + + if (sqd) { + if (sqd->thread) { + /* + * We may arrive here from the error branch in + * io_sq_offload_create() where the kthread is created + * without being waked up, thus wake it up now to make + * sure the wait will complete. + */ + wake_up_process(sqd->thread); + wait_for_completion(&ctx->sq_thread_comp); + } + + io_put_sq_data(sqd); + ctx->sq_data = NULL; } } @@ -7623,10 +7658,19 @@ static int io_sq_offload_create(struct io_ring_ctx *ctx, int ret; if (ctx->flags & IORING_SETUP_SQPOLL) { + struct io_sq_data *sqd; + ret = -EPERM; if (!capable(CAP_SYS_ADMIN)) goto err; + sqd = io_get_sq_data(p); + if (IS_ERR(sqd)) { + ret = PTR_ERR(sqd); + goto err; + } + ctx->sq_data = sqd; + ctx->sq_thread_idle = msecs_to_jiffies(p->sq_thread_idle); if (!ctx->sq_thread_idle) ctx->sq_thread_idle = HZ; @@ -7640,19 +7684,18 @@ static int io_sq_offload_create(struct io_ring_ctx *ctx, if (!cpu_online(cpu)) goto err; - ctx->sqo_thread = kthread_create_on_cpu(io_sq_thread, - ctx, cpu, - "io_uring-sq"); + sqd->thread = kthread_create_on_cpu(io_sq_thread, ctx, + cpu, "io_uring-sq"); } else { - ctx->sqo_thread = kthread_create(io_sq_thread, ctx, + sqd->thread = kthread_create(io_sq_thread, ctx, "io_uring-sq"); } - if (IS_ERR(ctx->sqo_thread)) { - ret = PTR_ERR(ctx->sqo_thread); - ctx->sqo_thread = NULL; + if (IS_ERR(sqd->thread)) { + ret = PTR_ERR(sqd->thread); + sqd->thread = NULL; goto err; } - ret = io_uring_alloc_task_context(ctx->sqo_thread); + ret = io_uring_alloc_task_context(sqd->thread); if (ret) goto err; } else if (p->flags & IORING_SETUP_SQ_AFF) { @@ -7673,8 +7716,10 @@ err: static void io_sq_offload_start(struct io_ring_ctx *ctx) { - if ((ctx->flags & IORING_SETUP_SQPOLL) && ctx->sqo_thread) - wake_up_process(ctx->sqo_thread); + struct io_sq_data *sqd = ctx->sq_data; + + if ((ctx->flags & IORING_SETUP_SQPOLL) && sqd->thread) + wake_up_process(sqd->thread); } static inline void __io_unaccount_mem(struct user_struct *user, @@ -8402,8 +8447,8 @@ static void io_uring_cancel_task_requests(struct io_ring_ctx *ctx, { struct task_struct *task = current; - if (ctx->flags & IORING_SETUP_SQPOLL) - task = ctx->sqo_thread; + if ((ctx->flags & IORING_SETUP_SQPOLL) && ctx->sq_data) + task = ctx->sq_data->thread; io_cqring_overflow_flush(ctx, true, task, files); @@ -8688,7 +8733,7 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit, if (!list_empty_careful(&ctx->cq_overflow_list)) io_cqring_overflow_flush(ctx, false, NULL, NULL); if (flags & IORING_ENTER_SQ_WAKEUP) - wake_up(ctx->sqo_wait); + wake_up(&ctx->sq_data->wait); submitted = to_submit; } else if (to_submit) { ret = io_uring_add_task_file(f.file); -- cgit v1.2.3-58-ga151 From 69fb21310fd36aad96370e05953f2c2366f492e4 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Mon, 14 Sep 2020 11:16:23 -0600 Subject: io_uring: base SQPOLL handling off io_sq_data Remove the SQPOLL thread from the ctx, and use the io_sq_data as the data structure we pass in. io_sq_data has a list of ctx's that we can then iterate over and handle. As of now we're ready to handle multiple ctx's, though we're still just handling a single one after this patch. Signed-off-by: Jens Axboe --- fs/io_uring.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 101 insertions(+), 25 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 9a7c645f971c..0a9eced754cb 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -232,6 +232,13 @@ struct io_restriction { struct io_sq_data { refcount_t refs; + struct mutex lock; + + /* ctx's that are using this sqd */ + struct list_head ctx_list; + struct list_head ctx_new_list; + struct mutex ctx_lock; + struct task_struct *thread; struct wait_queue_head wait; }; @@ -295,6 +302,7 @@ struct io_ring_ctx { struct io_sq_data *sq_data; /* if using sq thread polling */ struct wait_queue_entry sqo_wait_entry; + struct list_head sqd_list; /* * If used, fixed file set. Writers must ensure that ->refs is dead, @@ -1064,6 +1072,7 @@ static struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p) goto err; ctx->flags = p->flags; + INIT_LIST_HEAD(&ctx->sqd_list); init_waitqueue_head(&ctx->cq_wait); INIT_LIST_HEAD(&ctx->cq_overflow_list); init_completion(&ctx->ref_comp); @@ -6715,8 +6724,6 @@ again: goto again; } - io_ring_set_wakeup_flag(ctx); - to_submit = io_sqring_entries(ctx); if (!to_submit || ret == -EBUSY) return SQT_IDLE; @@ -6732,42 +6739,72 @@ again: return SQT_DID_WORK; } +static void io_sqd_init_new(struct io_sq_data *sqd) +{ + struct io_ring_ctx *ctx; + + while (!list_empty(&sqd->ctx_new_list)) { + ctx = list_first_entry(&sqd->ctx_new_list, struct io_ring_ctx, sqd_list); + init_wait(&ctx->sqo_wait_entry); + ctx->sqo_wait_entry.func = io_sq_wake_function; + list_move_tail(&ctx->sqd_list, &sqd->ctx_list); + complete(&ctx->sq_thread_comp); + } +} + static int io_sq_thread(void *data) { - struct io_ring_ctx *ctx = data; - const struct cred *old_cred; + const struct cred *old_cred = NULL; + struct io_sq_data *sqd = data; + struct io_ring_ctx *ctx; unsigned long start_jiffies; - init_wait(&ctx->sqo_wait_entry); - ctx->sqo_wait_entry.func = io_sq_wake_function; + start_jiffies = jiffies; + while (!kthread_should_stop()) { + enum sq_ret ret = 0; - complete(&ctx->sq_thread_comp); + /* + * Any changes to the sqd lists are synchronized through the + * kthread parking. This synchronizes the thread vs users, + * the users are synchronized on the sqd->ctx_lock. + */ + if (kthread_should_park()) + kthread_parkme(); - old_cred = override_creds(ctx->creds); + if (unlikely(!list_empty(&sqd->ctx_new_list))) + io_sqd_init_new(sqd); - start_jiffies = jiffies; - while (!kthread_should_park()) { - enum sq_ret ret; + list_for_each_entry(ctx, &sqd->ctx_list, sqd_list) { + if (current->cred != ctx->creds) { + if (old_cred) + revert_creds(old_cred); + old_cred = override_creds(ctx->creds); + } - ret = __io_sq_thread(ctx, start_jiffies); - switch (ret) { - case SQT_IDLE: - schedule(); - start_jiffies = jiffies; - continue; - case SQT_SPIN: + ret |= __io_sq_thread(ctx, start_jiffies); + + io_sq_thread_drop_mm(); + } + + if (ret & SQT_SPIN) { io_run_task_work(); cond_resched(); - fallthrough; - case SQT_DID_WORK: - continue; + } else if (ret == SQT_IDLE) { + if (kthread_should_park()) + continue; + list_for_each_entry(ctx, &sqd->ctx_list, sqd_list) + io_ring_set_wakeup_flag(ctx); + schedule(); + start_jiffies = jiffies; + list_for_each_entry(ctx, &sqd->ctx_list, sqd_list) + io_ring_clear_wakeup_flag(ctx); } } io_run_task_work(); - io_sq_thread_drop_mm(); - revert_creds(old_cred); + if (old_cred) + revert_creds(old_cred); kthread_parkme(); @@ -6968,10 +7005,32 @@ static struct io_sq_data *io_get_sq_data(struct io_uring_params *p) return ERR_PTR(-ENOMEM); refcount_set(&sqd->refs, 1); + INIT_LIST_HEAD(&sqd->ctx_list); + INIT_LIST_HEAD(&sqd->ctx_new_list); + mutex_init(&sqd->ctx_lock); + mutex_init(&sqd->lock); init_waitqueue_head(&sqd->wait); return sqd; } +static void io_sq_thread_unpark(struct io_sq_data *sqd) + __releases(&sqd->lock) +{ + if (!sqd->thread) + return; + kthread_unpark(sqd->thread); + mutex_unlock(&sqd->lock); +} + +static void io_sq_thread_park(struct io_sq_data *sqd) + __acquires(&sqd->lock) +{ + if (!sqd->thread) + return; + mutex_lock(&sqd->lock); + kthread_park(sqd->thread); +} + static void io_sq_thread_stop(struct io_ring_ctx *ctx) { struct io_sq_data *sqd = ctx->sq_data; @@ -6986,6 +7045,17 @@ static void io_sq_thread_stop(struct io_ring_ctx *ctx) */ wake_up_process(sqd->thread); wait_for_completion(&ctx->sq_thread_comp); + + io_sq_thread_park(sqd); + } + + mutex_lock(&sqd->ctx_lock); + list_del(&ctx->sqd_list); + mutex_unlock(&sqd->ctx_lock); + + if (sqd->thread) { + finish_wait(&sqd->wait, &ctx->sqo_wait_entry); + io_sq_thread_unpark(sqd); } io_put_sq_data(sqd); @@ -7669,7 +7739,13 @@ static int io_sq_offload_create(struct io_ring_ctx *ctx, ret = PTR_ERR(sqd); goto err; } + ctx->sq_data = sqd; + io_sq_thread_park(sqd); + mutex_lock(&sqd->ctx_lock); + list_add(&ctx->sqd_list, &sqd->ctx_new_list); + mutex_unlock(&sqd->ctx_lock); + io_sq_thread_unpark(sqd); ctx->sq_thread_idle = msecs_to_jiffies(p->sq_thread_idle); if (!ctx->sq_thread_idle) @@ -7684,10 +7760,10 @@ static int io_sq_offload_create(struct io_ring_ctx *ctx, if (!cpu_online(cpu)) goto err; - sqd->thread = kthread_create_on_cpu(io_sq_thread, ctx, + sqd->thread = kthread_create_on_cpu(io_sq_thread, sqd, cpu, "io_uring-sq"); } else { - sqd->thread = kthread_create(io_sq_thread, ctx, + sqd->thread = kthread_create(io_sq_thread, sqd, "io_uring-sq"); } if (IS_ERR(sqd->thread)) { -- cgit v1.2.3-58-ga151 From aa06165de863a09bceebef65ecaf19294b26fd2e Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Wed, 2 Sep 2020 14:50:27 -0600 Subject: io_uring: enable IORING_SETUP_ATTACH_WQ to attach to SQPOLL thread too We support using IORING_SETUP_ATTACH_WQ to share async backends between rings created by the same process, this now also allows the same to happen with SQPOLL. The setup procedure remains the same, the caller sets io_uring_params->wq_fd to the 'parent' context, and then the newly created ring will attach to that async backend. This means that multiple rings can share the same SQPOLL thread, saving resources. Signed-off-by: Jens Axboe --- fs/io_uring.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/fs/io_uring.c b/fs/io_uring.c index 0a9eced754cb..546bc1fbf482 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -6996,10 +6996,39 @@ static void io_put_sq_data(struct io_sq_data *sqd) } } +static struct io_sq_data *io_attach_sq_data(struct io_uring_params *p) +{ + struct io_ring_ctx *ctx_attach; + struct io_sq_data *sqd; + struct fd f; + + f = fdget(p->wq_fd); + if (!f.file) + return ERR_PTR(-ENXIO); + if (f.file->f_op != &io_uring_fops) { + fdput(f); + return ERR_PTR(-EINVAL); + } + + ctx_attach = f.file->private_data; + sqd = ctx_attach->sq_data; + if (!sqd) { + fdput(f); + return ERR_PTR(-EINVAL); + } + + refcount_inc(&sqd->refs); + fdput(f); + return sqd; +} + static struct io_sq_data *io_get_sq_data(struct io_uring_params *p) { struct io_sq_data *sqd; + if (p->flags & IORING_SETUP_ATTACH_WQ) + return io_attach_sq_data(p); + sqd = kzalloc(sizeof(*sqd), GFP_KERNEL); if (!sqd) return ERR_PTR(-ENOMEM); @@ -7751,6 +7780,9 @@ static int io_sq_offload_create(struct io_ring_ctx *ctx, if (!ctx->sq_thread_idle) ctx->sq_thread_idle = HZ; + if (sqd->thread) + goto done; + if (p->flags & IORING_SETUP_SQ_AFF) { int cpu = p->sq_thread_cpu; @@ -7780,6 +7812,7 @@ static int io_sq_offload_create(struct io_ring_ctx *ctx, goto err; } +done: ret = io_init_wq_offload(ctx, p); if (ret) goto err; -- cgit v1.2.3-58-ga151 From 738277adc81929b3e7c9b63fec6693868cc5f931 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Thu, 3 Sep 2020 05:54:56 -0600 Subject: io_uring: mark io_uring_fops/io_op_defs as __read_mostly These structures are never written, move them appropriately. Signed-off-by: Jens Axboe --- fs/io_uring.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 546bc1fbf482..a924ab1cf15b 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -755,7 +755,7 @@ struct io_op_def { unsigned needs_fsize : 1; }; -static const struct io_op_def io_op_defs[] = { +static const struct io_op_def io_op_defs[] __read_mostly = { [IORING_OP_NOP] = {}, [IORING_OP_READV] = { .async_ctx = 1, @@ -950,7 +950,7 @@ static int io_setup_async_rw(struct io_kiocb *req, const struct iovec *iovec, static struct kmem_cache *req_cachep; -static const struct file_operations io_uring_fops; +static const struct file_operations io_uring_fops __read_mostly; struct sock *io_uring_get_socket(struct file *file) { -- cgit v1.2.3-58-ga151 From 90554200724d5b280439dc361fe7ee92fe459ea7 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Thu, 3 Sep 2020 12:12:41 -0600 Subject: io_uring: provide IORING_ENTER_SQ_WAIT for SQPOLL SQ ring waits When using SQPOLL, applications can run into the issue of running out of SQ ring entries because the thread hasn't consumed them yet. The only option for dealing with that is checking later, or busy checking for the condition. Provide IORING_ENTER_SQ_WAIT if applications want to wait on this condition. Signed-off-by: Jens Axboe --- fs/io_uring.c | 40 +++++++++++++++++++++++++++++++++++++--- include/uapi/linux/io_uring.h | 1 + 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index a924ab1cf15b..3ee6ee1785d2 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -301,6 +301,7 @@ struct io_ring_ctx { struct io_sq_data *sq_data; /* if using sq thread polling */ + struct wait_queue_head sqo_sq_wait; struct wait_queue_entry sqo_wait_entry; struct list_head sqd_list; @@ -1072,6 +1073,7 @@ static struct io_ring_ctx *io_ring_ctx_alloc(struct io_uring_params *p) goto err; ctx->flags = p->flags; + init_waitqueue_head(&ctx->sqo_sq_wait); INIT_LIST_HEAD(&ctx->sqd_list); init_waitqueue_head(&ctx->cq_wait); INIT_LIST_HEAD(&ctx->cq_overflow_list); @@ -1324,6 +1326,13 @@ static void io_commit_cqring(struct io_ring_ctx *ctx) __io_queue_deferred(ctx); } +static inline bool io_sqring_full(struct io_ring_ctx *ctx) +{ + struct io_rings *r = ctx->rings; + + return READ_ONCE(r->sq.tail) - ctx->cached_sq_head == r->sq_ring_entries; +} + static struct io_uring_cqe *io_get_cqring(struct io_ring_ctx *ctx) { struct io_rings *rings = ctx->rings; @@ -6736,6 +6745,10 @@ again: if (likely(!percpu_ref_is_dying(&ctx->refs))) ret = io_submit_sqes(ctx, to_submit); mutex_unlock(&ctx->uring_lock); + + if (!io_sqring_full(ctx) && wq_has_sleeper(&ctx->sqo_sq_wait)) + wake_up(&ctx->sqo_sq_wait); + return SQT_DID_WORK; } @@ -8231,8 +8244,7 @@ static __poll_t io_uring_poll(struct file *file, poll_table *wait) * io_commit_cqring */ smp_rmb(); - if (READ_ONCE(ctx->rings->sq.tail) - ctx->cached_sq_head != - ctx->rings->sq_ring_entries) + if (!io_sqring_full(ctx)) mask |= EPOLLOUT | EPOLLWRNORM; if (io_cqring_events(ctx, false)) mask |= EPOLLIN | EPOLLRDNORM; @@ -8801,6 +8813,25 @@ static unsigned long io_uring_nommu_get_unmapped_area(struct file *file, #endif /* !CONFIG_MMU */ +static void io_sqpoll_wait_sq(struct io_ring_ctx *ctx) +{ + DEFINE_WAIT(wait); + + do { + if (!io_sqring_full(ctx)) + break; + + prepare_to_wait(&ctx->sqo_sq_wait, &wait, TASK_INTERRUPTIBLE); + + if (!io_sqring_full(ctx)) + break; + + schedule(); + } while (!signal_pending(current)); + + finish_wait(&ctx->sqo_sq_wait, &wait); +} + SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit, u32, min_complete, u32, flags, const sigset_t __user *, sig, size_t, sigsz) @@ -8812,7 +8843,8 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit, io_run_task_work(); - if (flags & ~(IORING_ENTER_GETEVENTS | IORING_ENTER_SQ_WAKEUP)) + if (flags & ~(IORING_ENTER_GETEVENTS | IORING_ENTER_SQ_WAKEUP | + IORING_ENTER_SQ_WAIT)) return -EINVAL; f = fdget(fd); @@ -8843,6 +8875,8 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, fd, u32, to_submit, io_cqring_overflow_flush(ctx, false, NULL, NULL); if (flags & IORING_ENTER_SQ_WAKEUP) wake_up(&ctx->sq_data->wait); + if (flags & IORING_ENTER_SQ_WAIT) + io_sqpoll_wait_sq(ctx); submitted = to_submit; } else if (to_submit) { ret = io_uring_add_task_file(f.file); diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h index a0c85e0e9016..98d8e06dea22 100644 --- a/include/uapi/linux/io_uring.h +++ b/include/uapi/linux/io_uring.h @@ -225,6 +225,7 @@ struct io_cqring_offsets { */ #define IORING_ENTER_GETEVENTS (1U << 0) #define IORING_ENTER_SQ_WAKEUP (1U << 1) +#define IORING_ENTER_SQ_WAIT (1U << 2) /* * Passed in for io_uring_setup(2). Copied back with updated info on success -- cgit v1.2.3-58-ga151 From f4bff104fffba2f069bc1b19ef0decbca7ff5459 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sun, 6 Sep 2020 00:45:45 +0300 Subject: io_uring: simplify io_rw_prep_async() Don't touch iter->iov and iov in between __io_import_iovec() and io_req_map_rw(), the former function aleady sets it correctly, because it creates one more case with NULL'ed iov to consider in io_req_map_rw(). Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 3ee6ee1785d2..6d8c2dcbfa08 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -3065,16 +3065,14 @@ static inline int io_rw_prep_async(struct io_kiocb *req, int rw, bool force_nonblock) { struct io_async_rw *iorw = &req->io->rw; - struct iovec *iov; + struct iovec *iov = iorw->fast_iov; ssize_t ret; - iorw->iter.iov = iov = iorw->fast_iov; ret = __io_import_iovec(rw, req, &iov, &iorw->iter, !force_nonblock); if (unlikely(ret < 0)) return ret; - iorw->iter.iov = iov; - io_req_map_rw(req, iorw->iter.iov, iorw->fast_iov, &iorw->iter); + io_req_map_rw(req, iov, iorw->fast_iov, &iorw->iter); return 0; } -- cgit v1.2.3-58-ga151 From afb87658f89b65e067334e06d833ccd859e659e6 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sun, 6 Sep 2020 00:45:46 +0300 Subject: io_uring: refactor io_req_map_rw() Set rw->free_iovec to @iovec, that gives an identical result and stresses that @iovec param rw->free_iovec play the same role. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 6d8c2dcbfa08..bc7028d9e6f9 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -3010,7 +3010,7 @@ static void io_req_map_rw(struct io_kiocb *req, const struct iovec *iovec, struct io_async_rw *rw = &req->io->rw; memcpy(&rw->iter, iter, sizeof(*iter)); - rw->free_iovec = NULL; + rw->free_iovec = iovec; rw->bytes_done = 0; /* can only be fixed buffers, no need to do anything */ if (iter->type == ITER_BVEC) @@ -3027,7 +3027,6 @@ static void io_req_map_rw(struct io_kiocb *req, const struct iovec *iovec, memcpy(rw->fast_iov + iov_off, fast_iov + iov_off, sizeof(struct iovec) * iter->nr_segs); } else { - rw->free_iovec = iovec; req->flags |= REQ_F_NEED_CLEANUP; } } -- cgit v1.2.3-58-ga151 From ab0b196ce5551a093388bb2ceace30df7891051c Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sun, 6 Sep 2020 00:45:47 +0300 Subject: io_uring: fix overlapped memcpy in io_req_map_rw() When io_req_map_rw() is called from io_rw_prep_async(), it memcpy() iorw->iter into itself. Even though it doesn't lead to an error, such a memcpy()'s aliasing rules violation is considered to be a bad practise. Inline io_req_map_rw() into io_rw_prep_async(). We don't really need any remapping there, so it's much simpler than the generic implementation. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index bc7028d9e6f9..ed7032bb76bf 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -3071,7 +3071,10 @@ static inline int io_rw_prep_async(struct io_kiocb *req, int rw, if (unlikely(ret < 0)) return ret; - io_req_map_rw(req, iov, iorw->fast_iov, &iorw->iter); + iorw->bytes_done = 0; + iorw->free_iovec = iov; + if (iov) + req->flags |= REQ_F_NEED_CLEANUP; return 0; } -- cgit v1.2.3-58-ga151 From 4be1c615126963ade321492283c05fb653f55099 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sun, 6 Sep 2020 00:45:48 +0300 Subject: io_uring: kill extra user_bufs check Testing ctx->user_bufs for NULL in io_import_fixed() is not neccessary, because in that case ctx->nr_user_bufs would be zero, and the following check would fail. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index ed7032bb76bf..f3d90a6108f8 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -2672,18 +2672,12 @@ static ssize_t io_import_fixed(struct io_kiocb *req, int rw, struct io_ring_ctx *ctx = req->ctx; size_t len = req->rw.len; struct io_mapped_ubuf *imu; - u16 index, buf_index; + u16 index, buf_index = req->buf_index; size_t offset; u64 buf_addr; - /* attempt to use fixed buffers without having provided iovecs */ - if (unlikely(!ctx->user_bufs)) - return -EFAULT; - - buf_index = req->buf_index; if (unlikely(buf_index >= ctx->nr_user_bufs)) return -EFAULT; - index = array_index_nospec(buf_index, ctx->nr_user_bufs); imu = &ctx->user_bufs[index]; buf_addr = req->rw.addr; -- cgit v1.2.3-58-ga151 From e8c2bc1fb6c9495b71efe7af476a351ccfba73c4 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Sat, 15 Aug 2020 18:44:09 -0700 Subject: io_uring: get rid of req->io/io_async_ctx union There's really no point in having this union, it just means that we're always allocating enough room to cater to any command. But that's pointless, as the ->io field is request type private anyway. This gets rid of the io_async_ctx structure, and fills in the required size in the io_op_defs[] instead. Signed-off-by: Jens Axboe --- fs/io_uring.c | 211 ++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 117 insertions(+), 94 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index f3d90a6108f8..268d8c526116 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -550,15 +550,6 @@ struct io_async_rw { struct wait_page_queue wpq; }; -struct io_async_ctx { - union { - struct io_async_rw rw; - struct io_async_msghdr msg; - struct io_async_connect connect; - struct io_timeout_data timeout; - }; -}; - enum { REQ_F_FIXED_FILE_BIT = IOSQE_FIXED_FILE_BIT, REQ_F_IO_DRAIN_BIT = IOSQE_IO_DRAIN_BIT, @@ -662,7 +653,8 @@ struct io_kiocb { struct io_completion compl; }; - struct io_async_ctx *io; + /* opcode allocated if it needs to store data for async defer */ + void *async_data; u8 opcode; /* polled IO has completed */ u8 iopoll_completed; @@ -730,8 +722,6 @@ struct io_submit_state { }; struct io_op_def { - /* needs req->io allocated for deferral/async */ - unsigned async_ctx : 1; /* needs current->mm setup, does mm access */ unsigned needs_mm : 1; /* needs req->file assigned */ @@ -753,27 +743,34 @@ struct io_op_def { unsigned pollout : 1; /* op supports buffer selection */ unsigned buffer_select : 1; + /* needs rlimit(RLIMIT_FSIZE) assigned */ unsigned needs_fsize : 1; + /* must always have async data allocated */ + unsigned needs_async_data : 1; + /* size of async data needed, if any */ + unsigned short async_size; }; static const struct io_op_def io_op_defs[] __read_mostly = { [IORING_OP_NOP] = {}, [IORING_OP_READV] = { - .async_ctx = 1, .needs_mm = 1, .needs_file = 1, .unbound_nonreg_file = 1, .pollin = 1, .buffer_select = 1, + .needs_async_data = 1, + .async_size = sizeof(struct io_async_rw), }, [IORING_OP_WRITEV] = { - .async_ctx = 1, .needs_mm = 1, .needs_file = 1, .hash_reg_file = 1, .unbound_nonreg_file = 1, .pollout = 1, .needs_fsize = 1, + .needs_async_data = 1, + .async_size = sizeof(struct io_async_rw), }, [IORING_OP_FSYNC] = { .needs_file = 1, @@ -782,6 +779,7 @@ static const struct io_op_def io_op_defs[] __read_mostly = { .needs_file = 1, .unbound_nonreg_file = 1, .pollin = 1, + .async_size = sizeof(struct io_async_rw), }, [IORING_OP_WRITE_FIXED] = { .needs_file = 1, @@ -789,6 +787,7 @@ static const struct io_op_def io_op_defs[] __read_mostly = { .unbound_nonreg_file = 1, .pollout = 1, .needs_fsize = 1, + .async_size = sizeof(struct io_async_rw), }, [IORING_OP_POLL_ADD] = { .needs_file = 1, @@ -799,25 +798,28 @@ static const struct io_op_def io_op_defs[] __read_mostly = { .needs_file = 1, }, [IORING_OP_SENDMSG] = { - .async_ctx = 1, .needs_mm = 1, .needs_file = 1, .unbound_nonreg_file = 1, .needs_fs = 1, .pollout = 1, + .needs_async_data = 1, + .async_size = sizeof(struct io_async_msghdr), }, [IORING_OP_RECVMSG] = { - .async_ctx = 1, .needs_mm = 1, .needs_file = 1, .unbound_nonreg_file = 1, .needs_fs = 1, .pollin = 1, .buffer_select = 1, + .needs_async_data = 1, + .async_size = sizeof(struct io_async_msghdr), }, [IORING_OP_TIMEOUT] = { - .async_ctx = 1, .needs_mm = 1, + .needs_async_data = 1, + .async_size = sizeof(struct io_timeout_data), }, [IORING_OP_TIMEOUT_REMOVE] = {}, [IORING_OP_ACCEPT] = { @@ -829,15 +831,17 @@ static const struct io_op_def io_op_defs[] __read_mostly = { }, [IORING_OP_ASYNC_CANCEL] = {}, [IORING_OP_LINK_TIMEOUT] = { - .async_ctx = 1, .needs_mm = 1, + .needs_async_data = 1, + .async_size = sizeof(struct io_timeout_data), }, [IORING_OP_CONNECT] = { - .async_ctx = 1, .needs_mm = 1, .needs_file = 1, .unbound_nonreg_file = 1, .pollout = 1, + .needs_async_data = 1, + .async_size = sizeof(struct io_async_connect), }, [IORING_OP_FALLOCATE] = { .needs_file = 1, @@ -867,6 +871,7 @@ static const struct io_op_def io_op_defs[] __read_mostly = { .unbound_nonreg_file = 1, .pollin = 1, .buffer_select = 1, + .async_size = sizeof(struct io_async_rw), }, [IORING_OP_WRITE] = { .needs_mm = 1, @@ -874,6 +879,7 @@ static const struct io_op_def io_op_defs[] __read_mostly = { .unbound_nonreg_file = 1, .pollout = 1, .needs_fsize = 1, + .async_size = sizeof(struct io_async_rw), }, [IORING_OP_FADVISE] = { .needs_file = 1, @@ -1233,9 +1239,10 @@ static void io_queue_async_work(struct io_kiocb *req) static void io_kill_timeout(struct io_kiocb *req) { + struct io_timeout_data *io = req->async_data; int ret; - ret = hrtimer_try_to_cancel(&req->io->timeout.timer); + ret = hrtimer_try_to_cancel(&io->timer); if (ret != -1) { atomic_set(&req->ctx->cq_timeouts, atomic_read(&req->ctx->cq_timeouts) + 1); @@ -1623,8 +1630,8 @@ static bool io_dismantle_req(struct io_kiocb *req) { io_clean_op(req); - if (req->io) - kfree(req->io); + if (req->async_data) + kfree(req->async_data); if (req->file) io_put_file(req, req->file, (req->flags & REQ_F_FIXED_FILE)); @@ -1683,10 +1690,11 @@ static void __io_free_req(struct io_kiocb *req) static bool io_link_cancel_timeout(struct io_kiocb *req) { + struct io_timeout_data *io = req->async_data; struct io_ring_ctx *ctx = req->ctx; int ret; - ret = hrtimer_try_to_cancel(&req->io->timeout.timer); + ret = hrtimer_try_to_cancel(&io->timer); if (ret != -1) { io_cqring_fill_event(req, -ECANCELED); io_commit_cqring(ctx); @@ -2368,7 +2376,7 @@ static bool io_resubmit_prep(struct io_kiocb *req, int error) goto end_req; } - if (!req->io) { + if (!req->async_data) { ret = io_import_iovec(rw, req, &iovec, &iter, false); if (ret < 0) goto end_req; @@ -2649,13 +2657,14 @@ static void kiocb_done(struct kiocb *kiocb, ssize_t ret, struct io_comp_state *cs) { struct io_kiocb *req = container_of(kiocb, struct io_kiocb, rw.kiocb); + struct io_async_rw *io = req->async_data; /* add previously done IO, if any */ - if (req->io && req->io->rw.bytes_done > 0) { + if (io && io->bytes_done > 0) { if (ret < 0) - ret = req->io->rw.bytes_done; + ret = io->bytes_done; else - ret += req->io->rw.bytes_done; + ret += io->bytes_done; } if (req->flags & REQ_F_CUR_POS) @@ -2929,10 +2938,12 @@ static ssize_t io_import_iovec(int rw, struct io_kiocb *req, struct iovec **iovec, struct iov_iter *iter, bool needs_lock) { - if (!req->io) + struct io_async_rw *iorw = req->async_data; + + if (!iorw) return __io_import_iovec(rw, req, iovec, iter, needs_lock); *iovec = NULL; - return iov_iter_count(&req->io->rw.iter); + return iov_iter_count(&iorw->iter); } static inline loff_t *io_kiocb_ppos(struct kiocb *kiocb) @@ -3001,7 +3012,7 @@ static ssize_t loop_rw_iter(int rw, struct file *file, struct kiocb *kiocb, static void io_req_map_rw(struct io_kiocb *req, const struct iovec *iovec, const struct iovec *fast_iov, struct iov_iter *iter) { - struct io_async_rw *rw = &req->io->rw; + struct io_async_rw *rw = req->async_data; memcpy(&rw->iter, iter, sizeof(*iter)); rw->free_iovec = iovec; @@ -3025,28 +3036,29 @@ static void io_req_map_rw(struct io_kiocb *req, const struct iovec *iovec, } } -static inline int __io_alloc_async_ctx(struct io_kiocb *req) +static inline int __io_alloc_async_data(struct io_kiocb *req) { - req->io = kmalloc(sizeof(*req->io), GFP_KERNEL); - return req->io == NULL; + WARN_ON_ONCE(!io_op_defs[req->opcode].async_size); + req->async_data = kmalloc(io_op_defs[req->opcode].async_size, GFP_KERNEL); + return req->async_data == NULL; } -static int io_alloc_async_ctx(struct io_kiocb *req) +static int io_alloc_async_data(struct io_kiocb *req) { - if (!io_op_defs[req->opcode].async_ctx) + if (!io_op_defs[req->opcode].needs_async_data) return 0; - return __io_alloc_async_ctx(req); + return __io_alloc_async_data(req); } static int io_setup_async_rw(struct io_kiocb *req, const struct iovec *iovec, const struct iovec *fast_iov, struct iov_iter *iter, bool force) { - if (!force && !io_op_defs[req->opcode].async_ctx) + if (!force && !io_op_defs[req->opcode].needs_async_data) return 0; - if (!req->io) { - if (__io_alloc_async_ctx(req)) + if (!req->async_data) { + if (__io_alloc_async_data(req)) return -ENOMEM; io_req_map_rw(req, iovec, fast_iov, iter); @@ -3057,7 +3069,7 @@ static int io_setup_async_rw(struct io_kiocb *req, const struct iovec *iovec, static inline int io_rw_prep_async(struct io_kiocb *req, int rw, bool force_nonblock) { - struct io_async_rw *iorw = &req->io->rw; + struct io_async_rw *iorw = req->async_data; struct iovec *iov = iorw->fast_iov; ssize_t ret; @@ -3085,7 +3097,7 @@ static int io_read_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe, return -EBADF; /* either don't need iovec imported or already have it */ - if (!req->io || req->flags & REQ_F_NEED_CLEANUP) + if (!req->async_data || req->flags & REQ_F_NEED_CLEANUP) return 0; return io_rw_prep_async(req, READ, force_nonblock); } @@ -3148,7 +3160,8 @@ static int io_async_buf_func(struct wait_queue_entry *wait, unsigned mode, */ static bool io_rw_should_retry(struct io_kiocb *req) { - struct wait_page_queue *wait = &req->io->rw.wpq; + struct io_async_rw *rw = req->async_data; + struct wait_page_queue *wait = &rw->wpq; struct kiocb *kiocb = &req->rw.kiocb; /* never retry for NOWAIT, we just complete with -EAGAIN */ @@ -3192,12 +3205,13 @@ static int io_read(struct io_kiocb *req, bool force_nonblock, struct iovec inline_vecs[UIO_FASTIOV], *iovec = inline_vecs; struct kiocb *kiocb = &req->rw.kiocb; struct iov_iter __iter, *iter = &__iter; + struct io_async_rw *rw = req->async_data; ssize_t io_size, ret, ret2; size_t iov_count; bool no_async; - if (req->io) - iter = &req->io->rw.iter; + if (rw) + iter = &rw->iter; ret = io_import_iovec(READ, req, &iovec, iter, !force_nonblock); if (ret < 0) @@ -3257,12 +3271,13 @@ copy_iov: } if (no_async) return -EAGAIN; + rw = req->async_data; /* it's copied and will be cleaned with ->io */ iovec = NULL; /* now use our persistent iterator, if we aren't already */ - iter = &req->io->rw.iter; + iter = &rw->iter; retry: - req->io->rw.bytes_done += ret; + rw->bytes_done += ret; /* if we can retry, do so with the callbacks armed */ if (!io_rw_should_retry(req)) { kiocb->ki_flags &= ~IOCB_WAITQ; @@ -3306,7 +3321,7 @@ static int io_write_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe, return -EBADF; /* either don't need iovec imported or already have it */ - if (!req->io || req->flags & REQ_F_NEED_CLEANUP) + if (!req->async_data || req->flags & REQ_F_NEED_CLEANUP) return 0; return io_rw_prep_async(req, WRITE, force_nonblock); } @@ -3317,11 +3332,12 @@ static int io_write(struct io_kiocb *req, bool force_nonblock, struct iovec inline_vecs[UIO_FASTIOV], *iovec = inline_vecs; struct kiocb *kiocb = &req->rw.kiocb; struct iov_iter __iter, *iter = &__iter; + struct io_async_rw *rw = req->async_data; size_t iov_count; ssize_t ret, ret2, io_size; - if (req->io) - iter = &req->io->rw.iter; + if (rw) + iter = &rw->iter; ret = io_import_iovec(WRITE, req, &iovec, iter, !force_nonblock); if (ret < 0) @@ -4098,15 +4114,18 @@ static int io_sync_file_range(struct io_kiocb *req, bool force_nonblock) static int io_setup_async_msg(struct io_kiocb *req, struct io_async_msghdr *kmsg) { - if (req->io) + struct io_async_msghdr *async_msg = req->async_data; + + if (async_msg) return -EAGAIN; - if (io_alloc_async_ctx(req)) { + if (io_alloc_async_data(req)) { if (kmsg->iov != kmsg->fast_iov) kfree(kmsg->iov); return -ENOMEM; } + async_msg = req->async_data; req->flags |= REQ_F_NEED_CLEANUP; - memcpy(&req->io->msg, kmsg, sizeof(*kmsg)); + memcpy(async_msg, kmsg, sizeof(*kmsg)); return -EAGAIN; } @@ -4121,8 +4140,8 @@ static int io_sendmsg_copy_hdr(struct io_kiocb *req, static int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) { + struct io_async_msghdr *async_msg = req->async_data; struct io_sr_msg *sr = &req->sr_msg; - struct io_async_ctx *io = req->io; int ret; if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL)) @@ -4137,13 +4156,13 @@ static int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) sr->msg_flags |= MSG_CMSG_COMPAT; #endif - if (!io || req->opcode == IORING_OP_SEND) + if (!async_msg || !io_op_defs[req->opcode].needs_async_data) return 0; /* iovec is already imported */ if (req->flags & REQ_F_NEED_CLEANUP) return 0; - ret = io_sendmsg_copy_hdr(req, &io->msg); + ret = io_sendmsg_copy_hdr(req, async_msg); if (!ret) req->flags |= REQ_F_NEED_CLEANUP; return ret; @@ -4161,9 +4180,9 @@ static int io_sendmsg(struct io_kiocb *req, bool force_nonblock, if (unlikely(!sock)) return ret; - if (req->io) { - kmsg = &req->io->msg; - kmsg->msg.msg_name = &req->io->msg.addr; + if (req->async_data) { + kmsg = req->async_data; + kmsg->msg.msg_name = &kmsg->addr; /* if iov is set, it's allocated already */ if (!kmsg->iov) kmsg->iov = kmsg->fast_iov; @@ -4350,8 +4369,8 @@ static inline unsigned int io_put_recv_kbuf(struct io_kiocb *req) static int io_recvmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) { + struct io_async_msghdr *async_msg = req->async_data; struct io_sr_msg *sr = &req->sr_msg; - struct io_async_ctx *io = req->io; int ret; if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL)) @@ -4367,13 +4386,13 @@ static int io_recvmsg_prep(struct io_kiocb *req, sr->msg_flags |= MSG_CMSG_COMPAT; #endif - if (!io || req->opcode == IORING_OP_RECV) + if (!async_msg || !io_op_defs[req->opcode].needs_async_data) return 0; /* iovec is already imported */ if (req->flags & REQ_F_NEED_CLEANUP) return 0; - ret = io_recvmsg_copy_hdr(req, &io->msg); + ret = io_recvmsg_copy_hdr(req, async_msg); if (!ret) req->flags |= REQ_F_NEED_CLEANUP; return ret; @@ -4392,9 +4411,9 @@ static int io_recvmsg(struct io_kiocb *req, bool force_nonblock, if (unlikely(!sock)) return ret; - if (req->io) { - kmsg = &req->io->msg; - kmsg->msg.msg_name = &req->io->msg.addr; + if (req->async_data) { + kmsg = req->async_data; + kmsg->msg.msg_name = &kmsg->addr; /* if iov is set, it's allocated already */ if (!kmsg->iov) kmsg->iov = kmsg->fast_iov; @@ -4536,7 +4555,7 @@ static int io_accept(struct io_kiocb *req, bool force_nonblock, static int io_connect_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) { struct io_connect *conn = &req->connect; - struct io_async_ctx *io = req->io; + struct io_async_connect *io = req->async_data; if (unlikely(req->ctx->flags & (IORING_SETUP_IOPOLL|IORING_SETUP_SQPOLL))) return -EINVAL; @@ -4550,22 +4569,22 @@ static int io_connect_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) return 0; return move_addr_to_kernel(conn->addr, conn->addr_len, - &io->connect.address); + &io->address); } static int io_connect(struct io_kiocb *req, bool force_nonblock, struct io_comp_state *cs) { - struct io_async_ctx __io, *io; + struct io_async_connect __io, *io; unsigned file_flags; int ret; - if (req->io) { - io = req->io; + if (req->async_data) { + io = req->async_data; } else { ret = move_addr_to_kernel(req->connect.addr, req->connect.addr_len, - &__io.connect.address); + &__io.address); if (ret) goto out; io = &__io; @@ -4573,16 +4592,17 @@ static int io_connect(struct io_kiocb *req, bool force_nonblock, file_flags = force_nonblock ? O_NONBLOCK : 0; - ret = __sys_connect_file(req->file, &io->connect.address, + ret = __sys_connect_file(req->file, &io->address, req->connect.addr_len, file_flags); if ((ret == -EAGAIN || ret == -EINPROGRESS) && force_nonblock) { - if (req->io) + if (req->async_data) return -EAGAIN; - if (io_alloc_async_ctx(req)) { + if (io_alloc_async_data(req)) { ret = -ENOMEM; goto out; } - memcpy(&req->io->connect, &__io.connect, sizeof(__io.connect)); + io = req->async_data; + memcpy(req->async_data, &__io, sizeof(__io)); return -EAGAIN; } if (ret == -ERESTARTSYS) @@ -4724,9 +4744,9 @@ static bool io_poll_rewait(struct io_kiocb *req, struct io_poll_iocb *poll) static struct io_poll_iocb *io_poll_get_double(struct io_kiocb *req) { - /* pure poll stashes this in ->io, poll driven retry elsewhere */ + /* pure poll stashes this in ->async_data, poll driven retry elsewhere */ if (req->opcode == IORING_OP_POLL_ADD) - return (struct io_poll_iocb *) req->io; + return req->async_data; return req->apoll->double_poll; } @@ -5169,7 +5189,7 @@ static void io_poll_queue_proc(struct file *file, struct wait_queue_head *head, { struct io_poll_table *pt = container_of(p, struct io_poll_table, pt); - __io_queue_proc(&pt->req->poll, pt, head, (struct io_poll_iocb **) &pt->req->io); + __io_queue_proc(&pt->req->poll, pt, head, (struct io_poll_iocb **) &pt->req->async_data); } static int io_poll_add_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) @@ -5250,11 +5270,12 @@ static enum hrtimer_restart io_timeout_fn(struct hrtimer *timer) static int __io_timeout_cancel(struct io_kiocb *req) { + struct io_timeout_data *io = req->async_data; int ret; list_del_init(&req->timeout.list); - ret = hrtimer_try_to_cancel(&req->io->timeout.timer); + ret = hrtimer_try_to_cancel(&io->timer); if (ret == -1) return -EALREADY; @@ -5341,10 +5362,10 @@ static int io_timeout_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe, req->timeout.off = off; - if (!req->io && io_alloc_async_ctx(req)) + if (!req->async_data && io_alloc_async_data(req)) return -ENOMEM; - data = &req->io->timeout; + data = req->async_data; data->req = req; if (get_timespec64(&data->ts, u64_to_user_ptr(sqe->addr))) @@ -5362,7 +5383,7 @@ static int io_timeout_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe, static int io_timeout(struct io_kiocb *req) { struct io_ring_ctx *ctx = req->ctx; - struct io_timeout_data *data = &req->io->timeout; + struct io_timeout_data *data = req->async_data; struct list_head *entry; u32 tail, off = req->timeout.off; @@ -5533,7 +5554,7 @@ static int io_req_defer_prep(struct io_kiocb *req, if (!sqe) return 0; - if (io_alloc_async_ctx(req)) + if (io_alloc_async_data(req)) return -EAGAIN; ret = io_prep_work_files(req); if (unlikely(ret)) @@ -5672,7 +5693,7 @@ static int io_req_defer(struct io_kiocb *req, const struct io_uring_sqe *sqe) if (!req_need_defer(req, seq) && list_empty_careful(&ctx->defer_list)) return 0; - if (!req->io) { + if (!req->async_data) { ret = io_req_defer_prep(req, sqe); if (ret) return ret; @@ -5716,8 +5737,6 @@ static void io_req_drop_files(struct io_kiocb *req) static void __io_clean_op(struct io_kiocb *req) { - struct io_async_ctx *io = req->io; - if (req->flags & REQ_F_BUFFER_SELECTED) { switch (req->opcode) { case IORING_OP_READV: @@ -5740,15 +5759,19 @@ static void __io_clean_op(struct io_kiocb *req) case IORING_OP_READ: case IORING_OP_WRITEV: case IORING_OP_WRITE_FIXED: - case IORING_OP_WRITE: - if (io->rw.free_iovec) - kfree(io->rw.free_iovec); + case IORING_OP_WRITE: { + struct io_async_rw *io = req->async_data; + if (io->free_iovec) + kfree(io->free_iovec); break; + } case IORING_OP_RECVMSG: - case IORING_OP_SENDMSG: - if (io->msg.iov != io->msg.fast_iov) - kfree(io->msg.iov); + case IORING_OP_SENDMSG: { + struct io_async_msghdr *io = req->async_data; + if (io->iov != io->fast_iov) + kfree(io->iov); break; + } case IORING_OP_SPLICE: case IORING_OP_TEE: io_put_file(req, req->splice.file_in, @@ -6180,7 +6203,7 @@ static void __io_queue_linked_timeout(struct io_kiocb *req) * we got a chance to setup the timer */ if (!list_empty(&req->link_list)) { - struct io_timeout_data *data = &req->io->timeout; + struct io_timeout_data *data = req->async_data; data->timer.function = io_link_timeout_fn; hrtimer_start(&data->timer, timespec64_to_ktime(data->ts), @@ -6304,7 +6327,7 @@ fail_req: io_req_complete(req, ret); } } else if (req->flags & REQ_F_FORCE_ASYNC) { - if (!req->io) { + if (!req->async_data) { ret = io_req_defer_prep(req, sqe); if (unlikely(ret)) goto fail_req; @@ -6509,7 +6532,7 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req, req->opcode = READ_ONCE(sqe->opcode); req->user_data = READ_ONCE(sqe->user_data); - req->io = NULL; + req->async_data = NULL; req->file = NULL; req->ctx = ctx; req->flags = 0; -- cgit v1.2.3-58-ga151 From e95eee2dee7862f267a169b10d384c82f71010ce Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Tue, 8 Sep 2020 09:11:32 -0600 Subject: io_uring: cap SQ submit size for SQPOLL with multiple rings In the spirit of fairness, cap the max number of SQ entries we'll submit for SQPOLL if we have multiple rings. If we don't do that, we could be submitting tons of entries for one ring, while others are waiting to get service. The value of 8 is somewhat arbitrarily chosen as something that allows a fair bit of batching, without using an excessive time per ring. Signed-off-by: Jens Axboe --- fs/io_uring.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 268d8c526116..61a65a2b0a24 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -6690,7 +6690,7 @@ enum sq_ret { }; static enum sq_ret __io_sq_thread(struct io_ring_ctx *ctx, - unsigned long start_jiffies) + unsigned long start_jiffies, bool cap_entries) { unsigned long timeout = start_jiffies + ctx->sq_thread_idle; struct io_sq_data *sqd = ctx->sq_data; @@ -6758,6 +6758,10 @@ again: finish_wait(&sqd->wait, &ctx->sqo_wait_entry); io_ring_clear_wakeup_flag(ctx); + /* if we're handling multiple rings, cap submit size for fairness */ + if (cap_entries && to_submit > 8) + to_submit = 8; + mutex_lock(&ctx->uring_lock); if (likely(!percpu_ref_is_dying(&ctx->refs))) ret = io_submit_sqes(ctx, to_submit); @@ -6792,6 +6796,7 @@ static int io_sq_thread(void *data) start_jiffies = jiffies; while (!kthread_should_stop()) { enum sq_ret ret = 0; + bool cap_entries; /* * Any changes to the sqd lists are synchronized through the @@ -6804,6 +6809,8 @@ static int io_sq_thread(void *data) if (unlikely(!list_empty(&sqd->ctx_new_list))) io_sqd_init_new(sqd); + cap_entries = !list_is_singular(&sqd->ctx_list); + list_for_each_entry(ctx, &sqd->ctx_list, sqd_list) { if (current->cred != ctx->creds) { if (old_cred) @@ -6811,7 +6818,7 @@ static int io_sq_thread(void *data) old_cred = override_creds(ctx->creds); } - ret |= __io_sq_thread(ctx, start_jiffies); + ret |= __io_sq_thread(ctx, start_jiffies, cap_entries); io_sq_thread_drop_mm(); } -- cgit v1.2.3-58-ga151 From 14db84110d489276d9c06ea38d59d3ff0ade2ae1 Mon Sep 17 00:00:00 2001 From: Zheng Bin Date: Wed, 9 Sep 2020 20:12:37 +0800 Subject: io_uring: remove unneeded semicolon Fixes coccicheck warning: fs/io_uring.c:4242:13-14: Unneeded semicolon Signed-off-by: Zheng Bin Signed-off-by: Jens Axboe --- fs/io_uring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 61a65a2b0a24..23fecfb7e100 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -4231,7 +4231,7 @@ static int io_send(struct io_kiocb *req, bool force_nonblock, ret = import_single_range(WRITE, sr->buf, sr->len, &iov, &msg.msg_iter); if (unlikely(ret)) - return ret;; + return ret; msg.msg_name = NULL; msg.msg_control = NULL; -- cgit v1.2.3-58-ga151 From de2939388be564836b06f0f06b3787bdedaed822 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Thu, 17 Sep 2020 16:19:16 -0600 Subject: io_uring: improve registered buffer accounting for huge pages io_uring does account any registered buffer as pinned/locked memory, and checks limit and fails if the given user doesn't have a big enough limit to register the ranges specified. However, if huge pages are used, we are potentially under-accounting the memory in terms of what gets pinned on the vm side. This patch rectifies that, by ensuring that we account the full size of a compound page, regardless of how much of it is being registered. Huge pages are not accounted mulitple times - if multiple sections of a huge page is registered, then the page is only accounted once. Reported-by: Andrea Arcangeli Signed-off-by: Jens Axboe --- fs/io_uring.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 80 insertions(+), 10 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 23fecfb7e100..0deaf8b5068d 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -190,6 +190,7 @@ struct io_mapped_ubuf { size_t len; struct bio_vec *bvec; unsigned int nr_bvecs; + unsigned long acct_pages; }; struct fixed_file_table { @@ -8002,7 +8003,8 @@ static int io_sqe_buffer_unregister(struct io_ring_ctx *ctx) for (j = 0; j < imu->nr_bvecs; j++) unpin_user_page(imu->bvec[j].bv_page); - io_unaccount_mem(ctx, imu->nr_bvecs, ACCT_PINNED); + if (imu->acct_pages) + io_unaccount_mem(ctx, imu->acct_pages, ACCT_PINNED); kvfree(imu->bvec); imu->nr_bvecs = 0; } @@ -8038,11 +8040,80 @@ static int io_copy_iov(struct io_ring_ctx *ctx, struct iovec *dst, return 0; } +/* + * Not super efficient, but this is just a registration time. And we do cache + * the last compound head, so generally we'll only do a full search if we don't + * match that one. + * + * We check if the given compound head page has already been accounted, to + * avoid double accounting it. This allows us to account the full size of the + * page, not just the constituent pages of a huge page. + */ +static bool headpage_already_acct(struct io_ring_ctx *ctx, struct page **pages, + int nr_pages, struct page *hpage) +{ + int i, j; + + /* check current page array */ + for (i = 0; i < nr_pages; i++) { + if (!PageCompound(pages[i])) + continue; + if (compound_head(pages[i]) == hpage) + return true; + } + + /* check previously registered pages */ + for (i = 0; i < ctx->nr_user_bufs; i++) { + struct io_mapped_ubuf *imu = &ctx->user_bufs[i]; + + for (j = 0; j < imu->nr_bvecs; j++) { + if (!PageCompound(imu->bvec[j].bv_page)) + continue; + if (compound_head(imu->bvec[j].bv_page) == hpage) + return true; + } + } + + return false; +} + +static int io_buffer_account_pin(struct io_ring_ctx *ctx, struct page **pages, + int nr_pages, struct io_mapped_ubuf *imu, + struct page **last_hpage) +{ + int i, ret; + + for (i = 0; i < nr_pages; i++) { + if (!PageCompound(pages[i])) { + imu->acct_pages++; + } else { + struct page *hpage; + + hpage = compound_head(pages[i]); + if (hpage == *last_hpage) + continue; + *last_hpage = hpage; + if (headpage_already_acct(ctx, pages, i, hpage)) + continue; + imu->acct_pages += page_size(hpage) >> PAGE_SHIFT; + } + } + + if (!imu->acct_pages) + return 0; + + ret = io_account_mem(ctx, imu->acct_pages, ACCT_PINNED); + if (ret) + imu->acct_pages = 0; + return ret; +} + static int io_sqe_buffer_register(struct io_ring_ctx *ctx, void __user *arg, unsigned nr_args) { struct vm_area_struct **vmas = NULL; struct page **pages = NULL; + struct page *last_hpage = NULL; int i, j, got_pages = 0; int ret = -EINVAL; @@ -8085,10 +8156,6 @@ static int io_sqe_buffer_register(struct io_ring_ctx *ctx, void __user *arg, start = ubuf >> PAGE_SHIFT; nr_pages = end - start; - ret = io_account_mem(ctx, nr_pages, ACCT_PINNED); - if (ret) - goto err; - ret = 0; if (!pages || nr_pages > got_pages) { kvfree(vmas); @@ -8100,7 +8167,6 @@ static int io_sqe_buffer_register(struct io_ring_ctx *ctx, void __user *arg, GFP_KERNEL); if (!pages || !vmas) { ret = -ENOMEM; - io_unaccount_mem(ctx, nr_pages, ACCT_PINNED); goto err; } got_pages = nr_pages; @@ -8109,10 +8175,8 @@ static int io_sqe_buffer_register(struct io_ring_ctx *ctx, void __user *arg, imu->bvec = kvmalloc_array(nr_pages, sizeof(struct bio_vec), GFP_KERNEL); ret = -ENOMEM; - if (!imu->bvec) { - io_unaccount_mem(ctx, nr_pages, ACCT_PINNED); + if (!imu->bvec) goto err; - } ret = 0; mmap_read_lock(current->mm); @@ -8141,7 +8205,13 @@ static int io_sqe_buffer_register(struct io_ring_ctx *ctx, void __user *arg, */ if (pret > 0) unpin_user_pages(pages, pret); - io_unaccount_mem(ctx, nr_pages, ACCT_PINNED); + kvfree(imu->bvec); + goto err; + } + + ret = io_buffer_account_pin(ctx, pages, pret, imu, &last_hpage); + if (ret) { + unpin_user_pages(pages, pret); kvfree(imu->bvec); goto err; } -- cgit v1.2.3-58-ga151 From 91d8f5191e8fe6fc6a87aa5353b36f5a7409fbec Mon Sep 17 00:00:00 2001 From: Dennis Zhou Date: Wed, 16 Sep 2020 13:41:05 -0700 Subject: io_uring: add blkcg accounting to offloaded operations There are a few operations that are offloaded to the worker threads. In this case, we lose process context and end up in kthread context. This results in ios to be not accounted to the issuing cgroup and consequently end up as issued by root. Just like others, adopt the personality of the blkcg too when issuing via the workqueues. For the SQPOLL thread, it will live and attach in the inited cgroup's context. Signed-off-by: Dennis Zhou Signed-off-by: Jens Axboe --- fs/io-wq.c | 23 +++++++++++++++ fs/io-wq.h | 3 ++ fs/io_uring.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+) diff --git a/fs/io-wq.c b/fs/io-wq.c index c1a7ef85844b..1be0d70f8673 100644 --- a/fs/io-wq.c +++ b/fs/io-wq.c @@ -17,6 +17,7 @@ #include #include #include +#include #include "io-wq.h" @@ -57,6 +58,9 @@ struct io_worker { struct rcu_head rcu; struct mm_struct *mm; +#ifdef CONFIG_BLK_CGROUP + struct cgroup_subsys_state *blkcg_css; +#endif const struct cred *cur_creds; const struct cred *saved_creds; struct files_struct *restore_files; @@ -177,6 +181,13 @@ static bool __io_worker_unuse(struct io_wqe *wqe, struct io_worker *worker) worker->mm = NULL; } +#ifdef CONFIG_BLK_CGROUP + if (worker->blkcg_css) { + kthread_associate_blkcg(NULL); + worker->blkcg_css = NULL; + } +#endif + return dropped_lock; } @@ -439,6 +450,17 @@ static void io_wq_switch_mm(struct io_worker *worker, struct io_wq_work *work) work->flags |= IO_WQ_WORK_CANCEL; } +static inline void io_wq_switch_blkcg(struct io_worker *worker, + struct io_wq_work *work) +{ +#ifdef CONFIG_BLK_CGROUP + if (work->blkcg_css != worker->blkcg_css) { + kthread_associate_blkcg(work->blkcg_css); + worker->blkcg_css = work->blkcg_css; + } +#endif +} + static void io_wq_switch_creds(struct io_worker *worker, struct io_wq_work *work) { @@ -467,6 +489,7 @@ static void io_impersonate_work(struct io_worker *worker, if (worker->cur_creds != work->creds) io_wq_switch_creds(worker, work); current->signal->rlim[RLIMIT_FSIZE].rlim_cur = work->fsize; + io_wq_switch_blkcg(worker, work); } static void io_assign_current_work(struct io_worker *worker, diff --git a/fs/io-wq.h b/fs/io-wq.h index 2519830c8c55..84bcf6a85523 100644 --- a/fs/io-wq.h +++ b/fs/io-wq.h @@ -87,6 +87,9 @@ struct io_wq_work { struct io_wq_work_node list; struct files_struct *files; struct mm_struct *mm; +#ifdef CONFIG_BLK_CGROUP + struct cgroup_subsys_state *blkcg_css; +#endif const struct cred *creds; struct nsproxy *nsproxy; struct fs_struct *fs; diff --git a/fs/io_uring.c b/fs/io_uring.c index 0deaf8b5068d..d7f41e3021e6 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -80,6 +80,7 @@ #include #include #include +#include #define CREATE_TRACE_POINTS #include @@ -300,6 +301,10 @@ struct io_ring_ctx { /* Only used for accounting purposes */ struct mm_struct *mm_account; +#ifdef CONFIG_BLK_CGROUP + struct cgroup_subsys_state *sqo_blkcg_css; +#endif + struct io_sq_data *sq_data; /* if using sq thread polling */ struct wait_queue_head sqo_sq_wait; @@ -748,6 +753,8 @@ struct io_op_def { unsigned needs_fsize : 1; /* must always have async data allocated */ unsigned needs_async_data : 1; + /* needs blkcg context, issues async io potentially */ + unsigned needs_blkcg : 1; /* size of async data needed, if any */ unsigned short async_size; }; @@ -761,6 +768,7 @@ static const struct io_op_def io_op_defs[] __read_mostly = { .pollin = 1, .buffer_select = 1, .needs_async_data = 1, + .needs_blkcg = 1, .async_size = sizeof(struct io_async_rw), }, [IORING_OP_WRITEV] = { @@ -771,15 +779,18 @@ static const struct io_op_def io_op_defs[] __read_mostly = { .pollout = 1, .needs_fsize = 1, .needs_async_data = 1, + .needs_blkcg = 1, .async_size = sizeof(struct io_async_rw), }, [IORING_OP_FSYNC] = { .needs_file = 1, + .needs_blkcg = 1, }, [IORING_OP_READ_FIXED] = { .needs_file = 1, .unbound_nonreg_file = 1, .pollin = 1, + .needs_blkcg = 1, .async_size = sizeof(struct io_async_rw), }, [IORING_OP_WRITE_FIXED] = { @@ -788,6 +799,7 @@ static const struct io_op_def io_op_defs[] __read_mostly = { .unbound_nonreg_file = 1, .pollout = 1, .needs_fsize = 1, + .needs_blkcg = 1, .async_size = sizeof(struct io_async_rw), }, [IORING_OP_POLL_ADD] = { @@ -797,6 +809,7 @@ static const struct io_op_def io_op_defs[] __read_mostly = { [IORING_OP_POLL_REMOVE] = {}, [IORING_OP_SYNC_FILE_RANGE] = { .needs_file = 1, + .needs_blkcg = 1, }, [IORING_OP_SENDMSG] = { .needs_mm = 1, @@ -805,6 +818,7 @@ static const struct io_op_def io_op_defs[] __read_mostly = { .needs_fs = 1, .pollout = 1, .needs_async_data = 1, + .needs_blkcg = 1, .async_size = sizeof(struct io_async_msghdr), }, [IORING_OP_RECVMSG] = { @@ -815,6 +829,7 @@ static const struct io_op_def io_op_defs[] __read_mostly = { .pollin = 1, .buffer_select = 1, .needs_async_data = 1, + .needs_blkcg = 1, .async_size = sizeof(struct io_async_msghdr), }, [IORING_OP_TIMEOUT] = { @@ -847,15 +862,18 @@ static const struct io_op_def io_op_defs[] __read_mostly = { [IORING_OP_FALLOCATE] = { .needs_file = 1, .needs_fsize = 1, + .needs_blkcg = 1, }, [IORING_OP_OPENAT] = { .file_table = 1, .needs_fs = 1, + .needs_blkcg = 1, }, [IORING_OP_CLOSE] = { .needs_file = 1, .needs_file_no_error = 1, .file_table = 1, + .needs_blkcg = 1, }, [IORING_OP_FILES_UPDATE] = { .needs_mm = 1, @@ -865,6 +883,7 @@ static const struct io_op_def io_op_defs[] __read_mostly = { .needs_mm = 1, .needs_fs = 1, .file_table = 1, + .needs_blkcg = 1, }, [IORING_OP_READ] = { .needs_mm = 1, @@ -872,6 +891,7 @@ static const struct io_op_def io_op_defs[] __read_mostly = { .unbound_nonreg_file = 1, .pollin = 1, .buffer_select = 1, + .needs_blkcg = 1, .async_size = sizeof(struct io_async_rw), }, [IORING_OP_WRITE] = { @@ -880,19 +900,23 @@ static const struct io_op_def io_op_defs[] __read_mostly = { .unbound_nonreg_file = 1, .pollout = 1, .needs_fsize = 1, + .needs_blkcg = 1, .async_size = sizeof(struct io_async_rw), }, [IORING_OP_FADVISE] = { .needs_file = 1, + .needs_blkcg = 1, }, [IORING_OP_MADVISE] = { .needs_mm = 1, + .needs_blkcg = 1, }, [IORING_OP_SEND] = { .needs_mm = 1, .needs_file = 1, .unbound_nonreg_file = 1, .pollout = 1, + .needs_blkcg = 1, }, [IORING_OP_RECV] = { .needs_mm = 1, @@ -900,10 +924,12 @@ static const struct io_op_def io_op_defs[] __read_mostly = { .unbound_nonreg_file = 1, .pollin = 1, .buffer_select = 1, + .needs_blkcg = 1, }, [IORING_OP_OPENAT2] = { .file_table = 1, .needs_fs = 1, + .needs_blkcg = 1, }, [IORING_OP_EPOLL_CTL] = { .unbound_nonreg_file = 1, @@ -913,6 +939,7 @@ static const struct io_op_def io_op_defs[] __read_mostly = { .needs_file = 1, .hash_reg_file = 1, .unbound_nonreg_file = 1, + .needs_blkcg = 1, }, [IORING_OP_PROVIDE_BUFFERS] = {}, [IORING_OP_REMOVE_BUFFERS] = {}, @@ -1011,6 +1038,26 @@ static int io_sq_thread_acquire_mm(struct io_ring_ctx *ctx, return __io_sq_thread_acquire_mm(ctx); } +static void io_sq_thread_associate_blkcg(struct io_ring_ctx *ctx, + struct cgroup_subsys_state **cur_css) + +{ +#ifdef CONFIG_BLK_CGROUP + /* puts the old one when swapping */ + if (*cur_css != ctx->sqo_blkcg_css) { + kthread_associate_blkcg(ctx->sqo_blkcg_css); + *cur_css = ctx->sqo_blkcg_css; + } +#endif +} + +static void io_sq_thread_unassociate_blkcg(void) +{ +#ifdef CONFIG_BLK_CGROUP + kthread_associate_blkcg(NULL); +#endif +} + static inline void req_set_fail_links(struct io_kiocb *req) { if ((req->flags & (REQ_F_LINK | REQ_F_HARDLINK)) == REQ_F_LINK) @@ -1148,6 +1195,10 @@ static bool io_req_clean_work(struct io_kiocb *req) mmdrop(req->work.mm); req->work.mm = NULL; } +#ifdef CONFIG_BLK_CGROUP + if (req->work.blkcg_css) + css_put(req->work.blkcg_css); +#endif if (req->work.creds) { put_cred(req->work.creds); req->work.creds = NULL; @@ -1187,6 +1238,19 @@ static void io_prep_async_work(struct io_kiocb *req) mmgrab(current->mm); req->work.mm = current->mm; } +#ifdef CONFIG_BLK_CGROUP + if (!req->work.blkcg_css && def->needs_blkcg) { + rcu_read_lock(); + req->work.blkcg_css = blkcg_css(); + /* + * This should be rare, either the cgroup is dying or the task + * is moving cgroups. Just punt to root for the handful of ios. + */ + if (!css_tryget_online(req->work.blkcg_css)) + req->work.blkcg_css = NULL; + rcu_read_unlock(); + } +#endif if (!req->work.creds) req->work.creds = get_current_cred(); if (!req->work.fs && def->needs_fs) { @@ -6789,6 +6853,7 @@ static void io_sqd_init_new(struct io_sq_data *sqd) static int io_sq_thread(void *data) { + struct cgroup_subsys_state *cur_css = NULL; const struct cred *old_cred = NULL; struct io_sq_data *sqd = data; struct io_ring_ctx *ctx; @@ -6818,6 +6883,7 @@ static int io_sq_thread(void *data) revert_creds(old_cred); old_cred = override_creds(ctx->creds); } + io_sq_thread_associate_blkcg(ctx, &cur_css); ret |= __io_sq_thread(ctx, start_jiffies, cap_entries); @@ -6841,6 +6907,8 @@ static int io_sq_thread(void *data) io_run_task_work(); + if (cur_css) + io_sq_thread_unassociate_blkcg(); if (old_cred) revert_creds(old_cred); @@ -8304,6 +8372,11 @@ static void io_ring_ctx_free(struct io_ring_ctx *ctx) ctx->mm_account = NULL; } +#ifdef CONFIG_BLK_CGROUP + if (ctx->sqo_blkcg_css) + css_put(ctx->sqo_blkcg_css); +#endif + io_sqe_files_unregister(ctx); io_eventfd_unregister(ctx); io_destroy_buffers(ctx); @@ -9288,6 +9361,25 @@ static int io_uring_create(unsigned entries, struct io_uring_params *p, mmgrab(current->mm); ctx->mm_account = current->mm; +#ifdef CONFIG_BLK_CGROUP + /* + * The sq thread will belong to the original cgroup it was inited in. + * If the cgroup goes offline (e.g. disabling the io controller), then + * issued bios will be associated with the closest cgroup later in the + * block layer. + */ + rcu_read_lock(); + ctx->sqo_blkcg_css = blkcg_css(); + ret = css_tryget_online(ctx->sqo_blkcg_css); + rcu_read_unlock(); + if (!ret) { + /* don't init against a dying cgroup, have the user try again */ + ctx->sqo_blkcg_css = NULL; + ret = -ENODEV; + goto err; + } +#endif + /* * Account memory _before_ installing the file descriptor. Once * the descriptor is installed, it can get closed at any time. Also -- cgit v1.2.3-58-ga151 From af9c1a44f8dee7a958e07977f24ba40e3c770987 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Thu, 24 Sep 2020 13:32:18 -0600 Subject: io_uring: process task work in io_uring_register() We do this for CQ ring wait, in case task_work completions come in. We should do the same in io_uring_register(), to avoid spurious -EINTR if the ring quiescing ends up having to process task_work to complete the operation Reported-by: Dan Melnic Signed-off-by: Jens Axboe --- fs/io_uring.c | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index d7f41e3021e6..b7a3e7c7f7bf 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -6950,6 +6950,22 @@ static int io_wake_function(struct wait_queue_entry *curr, unsigned int mode, return autoremove_wake_function(curr, mode, wake_flags, key); } +static int io_run_task_work_sig(void) +{ + if (io_run_task_work()) + return 1; + if (!signal_pending(current)) + return 0; + if (current->jobctl & JOBCTL_TASK_WORK) { + spin_lock_irq(¤t->sighand->siglock); + current->jobctl &= ~JOBCTL_TASK_WORK; + recalc_sigpending(); + spin_unlock_irq(¤t->sighand->siglock); + return 1; + } + return -EINTR; +} + /* * Wait until events become available, if we don't already have some. The * application must reap them itself, as they reside on the shared cq ring. @@ -6995,19 +7011,11 @@ static int io_cqring_wait(struct io_ring_ctx *ctx, int min_events, prepare_to_wait_exclusive(&ctx->wait, &iowq.wq, TASK_INTERRUPTIBLE); /* make sure we run task_work before checking for signals */ - if (io_run_task_work()) + ret = io_run_task_work_sig(); + if (ret > 0) continue; - if (signal_pending(current)) { - if (current->jobctl & JOBCTL_TASK_WORK) { - spin_lock_irq(¤t->sighand->siglock); - current->jobctl &= ~JOBCTL_TASK_WORK; - recalc_sigpending(); - spin_unlock_irq(¤t->sighand->siglock); - continue; - } - ret = -EINTR; + else if (ret < 0) break; - } if (io_should_wake(&iowq, false)) break; schedule(); @@ -9666,8 +9674,16 @@ static int __io_uring_register(struct io_ring_ctx *ctx, unsigned opcode, * after we've killed the percpu ref. */ mutex_unlock(&ctx->uring_lock); - ret = wait_for_completion_interruptible(&ctx->ref_comp); + do { + ret = wait_for_completion_interruptible(&ctx->ref_comp); + if (!ret) + break; + if (io_run_task_work_sig() > 0) + continue; + } while (1); + mutex_lock(&ctx->uring_lock); + if (ret) { percpu_ref_resurrect(&ctx->refs); ret = -EINTR; -- cgit v1.2.3-58-ga151 From dbbe9c642411c359ad0a0e32442eb2e11d3811b5 Mon Sep 17 00:00:00 2001 From: Joseph Qi Date: Tue, 29 Sep 2020 09:01:22 -0600 Subject: io_uring: show sqthread pid and cpu in fdinfo In most cases we'll specify IORING_SETUP_SQPOLL and run multiple io_uring instances in a host. Since all sqthreads are named "io_uring-sq", it's hard to distinguish the relations between application process and its io_uring sqthread. With this patch, application can get its corresponding sqthread pid and cpu through show_fdinfo. Steps: 1. Get io_uring fd first. $ ls -l /proc//fd | grep -w io_uring 2. Then get io_uring instance related info, including corresponding sqthread pid and cpu. $ cat /proc//fdinfo/ pos: 0 flags: 02000002 mnt_id: 13 SqThread: 6929 SqThreadCpu: 2 UserFiles: 1 0: testfile UserBufs: 0 PollList: Signed-off-by: Joseph Qi Reviewed-by: Stefano Garzarella [axboe: fixed for new shared SQPOLL] Signed-off-by: Jens Axboe --- fs/io_uring.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fs/io_uring.c b/fs/io_uring.c index b7a3e7c7f7bf..fcb4e95cb88d 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -9124,6 +9124,7 @@ static int io_uring_show_cred(int id, void *p, void *data) static void __io_uring_show_fdinfo(struct io_ring_ctx *ctx, struct seq_file *m) { + struct io_sq_data *sq = NULL; bool has_lock; int i; @@ -9135,6 +9136,11 @@ static void __io_uring_show_fdinfo(struct io_ring_ctx *ctx, struct seq_file *m) */ has_lock = mutex_trylock(&ctx->uring_lock); + if (has_lock && (ctx->flags & IORING_SETUP_SQPOLL)) + sq = ctx->sq_data; + + seq_printf(m, "SqThread:\t%d\n", sq ? task_pid_nr(sq->thread) : -1); + seq_printf(m, "SqThreadCpu:\t%d\n", sq ? task_cpu(sq->thread) : -1); seq_printf(m, "UserFiles:\t%u\n", ctx->nr_user_files); for (i = 0; has_lock && i < ctx->nr_user_files; i++) { struct fixed_file_table *table; -- cgit v1.2.3-58-ga151 From c4068bf898ddaef791049a366828d9b84b467bda Mon Sep 17 00:00:00 2001 From: Hillf Danton Date: Sat, 26 Sep 2020 21:26:55 +0800 Subject: io-wq: fix use-after-free in io_wq_worker_running The smart syzbot has found a reproducer for the following issue: ================================================================== BUG: KASAN: use-after-free in instrument_atomic_write include/linux/instrumented.h:71 [inline] BUG: KASAN: use-after-free in atomic_inc include/asm-generic/atomic-instrumented.h:240 [inline] BUG: KASAN: use-after-free in io_wqe_inc_running fs/io-wq.c:301 [inline] BUG: KASAN: use-after-free in io_wq_worker_running+0xde/0x110 fs/io-wq.c:613 Write of size 4 at addr ffff8882183db08c by task io_wqe_worker-0/7771 CPU: 0 PID: 7771 Comm: io_wqe_worker-0 Not tainted 5.9.0-rc4-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x198/0x1fd lib/dump_stack.c:118 print_address_description.constprop.0.cold+0xae/0x497 mm/kasan/report.c:383 __kasan_report mm/kasan/report.c:513 [inline] kasan_report.cold+0x1f/0x37 mm/kasan/report.c:530 check_memory_region_inline mm/kasan/generic.c:186 [inline] check_memory_region+0x13d/0x180 mm/kasan/generic.c:192 instrument_atomic_write include/linux/instrumented.h:71 [inline] atomic_inc include/asm-generic/atomic-instrumented.h:240 [inline] io_wqe_inc_running fs/io-wq.c:301 [inline] io_wq_worker_running+0xde/0x110 fs/io-wq.c:613 schedule_timeout+0x148/0x250 kernel/time/timer.c:1879 io_wqe_worker+0x517/0x10e0 fs/io-wq.c:580 kthread+0x3b5/0x4a0 kernel/kthread.c:292 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294 Allocated by task 7768: kasan_save_stack+0x1b/0x40 mm/kasan/common.c:48 kasan_set_track mm/kasan/common.c:56 [inline] __kasan_kmalloc.constprop.0+0xbf/0xd0 mm/kasan/common.c:461 kmem_cache_alloc_node_trace+0x17b/0x3f0 mm/slab.c:3594 kmalloc_node include/linux/slab.h:572 [inline] kzalloc_node include/linux/slab.h:677 [inline] io_wq_create+0x57b/0xa10 fs/io-wq.c:1064 io_init_wq_offload fs/io_uring.c:7432 [inline] io_sq_offload_start fs/io_uring.c:7504 [inline] io_uring_create fs/io_uring.c:8625 [inline] io_uring_setup+0x1836/0x28e0 fs/io_uring.c:8694 do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x44/0xa9 Freed by task 21: kasan_save_stack+0x1b/0x40 mm/kasan/common.c:48 kasan_set_track+0x1c/0x30 mm/kasan/common.c:56 kasan_set_free_info+0x1b/0x30 mm/kasan/generic.c:355 __kasan_slab_free+0xd8/0x120 mm/kasan/common.c:422 __cache_free mm/slab.c:3418 [inline] kfree+0x10e/0x2b0 mm/slab.c:3756 __io_wq_destroy fs/io-wq.c:1138 [inline] io_wq_destroy+0x2af/0x460 fs/io-wq.c:1146 io_finish_async fs/io_uring.c:6836 [inline] io_ring_ctx_free fs/io_uring.c:7870 [inline] io_ring_exit_work+0x1e4/0x6d0 fs/io_uring.c:7954 process_one_work+0x94c/0x1670 kernel/workqueue.c:2269 worker_thread+0x64c/0x1120 kernel/workqueue.c:2415 kthread+0x3b5/0x4a0 kernel/kthread.c:292 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:294 The buggy address belongs to the object at ffff8882183db000 which belongs to the cache kmalloc-1k of size 1024 The buggy address is located 140 bytes inside of 1024-byte region [ffff8882183db000, ffff8882183db400) The buggy address belongs to the page: page:000000009bada22b refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x2183db flags: 0x57ffe0000000200(slab) raw: 057ffe0000000200 ffffea0008604c48 ffffea00086a8648 ffff8880aa040700 raw: 0000000000000000 ffff8882183db000 0000000100000002 0000000000000000 page dumped because: kasan: bad access detected Memory state around the buggy address: ffff8882183daf80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ffff8882183db000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb >ffff8882183db080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ^ ffff8882183db100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ffff8882183db180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb ================================================================== which is down to the comment below, /* all workers gone, wq exit can proceed */ if (!nr_workers && refcount_dec_and_test(&wqe->wq->refs)) complete(&wqe->wq->done); because there might be multiple cases of wqe in a wq and we would wait for every worker in every wqe to go home before releasing wq's resources on destroying. To that end, rework wq's refcount by making it independent of the tracking of workers because after all they are two different things, and keeping it balanced when workers come and go. Note the manager kthread, like other workers, now holds a grab to wq during its lifetime. Finally to help destroy wq, check IO_WQ_BIT_EXIT upon creating worker and do nothing for exiting wq. Cc: stable@vger.kernel.org # v5.5+ Reported-by: syzbot+45fa0a195b941764e0f0@syzkaller.appspotmail.com Reported-by: syzbot+9af99580130003da82b1@syzkaller.appspotmail.com Cc: Pavel Begunkov Signed-off-by: Hillf Danton Signed-off-by: Jens Axboe --- fs/io-wq.c | 116 ++++++++++++++++++++++++++++++------------------------------- 1 file changed, 58 insertions(+), 58 deletions(-) diff --git a/fs/io-wq.c b/fs/io-wq.c index 1be0d70f8673..98f9c74b25d2 100644 --- a/fs/io-wq.c +++ b/fs/io-wq.c @@ -213,7 +213,6 @@ static void io_worker_exit(struct io_worker *worker) { struct io_wqe *wqe = worker->wqe; struct io_wqe_acct *acct = io_wqe_get_acct(wqe, worker); - unsigned nr_workers; /* * If we're not at zero, someone else is holding a brief reference @@ -241,15 +240,11 @@ static void io_worker_exit(struct io_worker *worker) raw_spin_lock_irq(&wqe->lock); } acct->nr_workers--; - nr_workers = wqe->acct[IO_WQ_ACCT_BOUND].nr_workers + - wqe->acct[IO_WQ_ACCT_UNBOUND].nr_workers; raw_spin_unlock_irq(&wqe->lock); - /* all workers gone, wq exit can proceed */ - if (!nr_workers && refcount_dec_and_test(&wqe->wq->refs)) - complete(&wqe->wq->done); - kfree_rcu(worker, rcu); + if (refcount_dec_and_test(&wqe->wq->refs)) + complete(&wqe->wq->done); } static inline bool io_wqe_run_queue(struct io_wqe *wqe) @@ -664,7 +659,7 @@ void io_wq_worker_sleeping(struct task_struct *tsk) static bool create_io_worker(struct io_wq *wq, struct io_wqe *wqe, int index) { - struct io_wqe_acct *acct =&wqe->acct[index]; + struct io_wqe_acct *acct = &wqe->acct[index]; struct io_worker *worker; worker = kzalloc_node(sizeof(*worker), GFP_KERNEL, wqe->node); @@ -697,6 +692,7 @@ static bool create_io_worker(struct io_wq *wq, struct io_wqe *wqe, int index) if (index == IO_WQ_ACCT_UNBOUND) atomic_inc(&wq->user->processes); + refcount_inc(&wq->refs); wake_up_process(worker->task); return true; } @@ -712,28 +708,63 @@ static inline bool io_wqe_need_worker(struct io_wqe *wqe, int index) return acct->nr_workers < acct->max_workers; } +static bool io_wqe_worker_send_sig(struct io_worker *worker, void *data) +{ + send_sig(SIGINT, worker->task, 1); + return false; +} + +/* + * Iterate the passed in list and call the specific function for each + * worker that isn't exiting + */ +static bool io_wq_for_each_worker(struct io_wqe *wqe, + bool (*func)(struct io_worker *, void *), + void *data) +{ + struct io_worker *worker; + bool ret = false; + + list_for_each_entry_rcu(worker, &wqe->all_list, all_list) { + if (io_worker_get(worker)) { + /* no task if node is/was offline */ + if (worker->task) + ret = func(worker, data); + io_worker_release(worker); + if (ret) + break; + } + } + + return ret; +} + +static bool io_wq_worker_wake(struct io_worker *worker, void *data) +{ + wake_up_process(worker->task); + return false; +} + /* * Manager thread. Tasked with creating new workers, if we need them. */ static int io_wq_manager(void *data) { struct io_wq *wq = data; - int workers_to_create = num_possible_nodes(); int node; /* create fixed workers */ - refcount_set(&wq->refs, workers_to_create); + refcount_set(&wq->refs, 1); for_each_node(node) { if (!node_online(node)) continue; - if (!create_io_worker(wq, wq->wqes[node], IO_WQ_ACCT_BOUND)) - goto err; - workers_to_create--; + if (create_io_worker(wq, wq->wqes[node], IO_WQ_ACCT_BOUND)) + continue; + set_bit(IO_WQ_BIT_ERROR, &wq->state); + set_bit(IO_WQ_BIT_EXIT, &wq->state); + goto out; } - while (workers_to_create--) - refcount_dec(&wq->refs); - complete(&wq->done); while (!kthread_should_stop()) { @@ -765,12 +796,18 @@ static int io_wq_manager(void *data) if (current->task_works) task_work_run(); - return 0; -err: - set_bit(IO_WQ_BIT_ERROR, &wq->state); - set_bit(IO_WQ_BIT_EXIT, &wq->state); - if (refcount_sub_and_test(workers_to_create, &wq->refs)) +out: + if (refcount_dec_and_test(&wq->refs)) { complete(&wq->done); + return 0; + } + /* if ERROR is set and we get here, we have workers to wake */ + if (test_bit(IO_WQ_BIT_ERROR, &wq->state)) { + rcu_read_lock(); + for_each_node(node) + io_wq_for_each_worker(wq->wqes[node], io_wq_worker_wake, NULL); + rcu_read_unlock(); + } return 0; } @@ -877,37 +914,6 @@ void io_wq_hash_work(struct io_wq_work *work, void *val) work->flags |= (IO_WQ_WORK_HASHED | (bit << IO_WQ_HASH_SHIFT)); } -static bool io_wqe_worker_send_sig(struct io_worker *worker, void *data) -{ - send_sig(SIGINT, worker->task, 1); - return false; -} - -/* - * Iterate the passed in list and call the specific function for each - * worker that isn't exiting - */ -static bool io_wq_for_each_worker(struct io_wqe *wqe, - bool (*func)(struct io_worker *, void *), - void *data) -{ - struct io_worker *worker; - bool ret = false; - - list_for_each_entry_rcu(worker, &wqe->all_list, all_list) { - if (io_worker_get(worker)) { - /* no task if node is/was offline */ - if (worker->task) - ret = func(worker, data); - io_worker_release(worker); - if (ret) - break; - } - } - - return ret; -} - void io_wq_cancel_all(struct io_wq *wq) { int node; @@ -1140,12 +1146,6 @@ bool io_wq_get(struct io_wq *wq, struct io_wq_data *data) return refcount_inc_not_zero(&wq->use_refs); } -static bool io_wq_worker_wake(struct io_worker *worker, void *data) -{ - wake_up_process(worker->task); - return false; -} - static void __io_wq_destroy(struct io_wq *wq) { int node; -- cgit v1.2.3-58-ga151 From 145cc8c665f406cc189cfcf15a9875689e2c73b8 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Sat, 26 Sep 2020 12:37:46 -0600 Subject: io-wq: kill unused IO_WORKER_F_EXITING This flag is no longer used, remove it. Signed-off-by: Jens Axboe --- fs/io-wq.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/fs/io-wq.c b/fs/io-wq.c index 98f9c74b25d2..0a182f1333e8 100644 --- a/fs/io-wq.c +++ b/fs/io-wq.c @@ -27,9 +27,8 @@ enum { IO_WORKER_F_UP = 1, /* up and active */ IO_WORKER_F_RUNNING = 2, /* account as running */ IO_WORKER_F_FREE = 4, /* worker on free list */ - IO_WORKER_F_EXITING = 8, /* worker exiting */ - IO_WORKER_F_FIXED = 16, /* static idle worker */ - IO_WORKER_F_BOUND = 32, /* is doing bounded work */ + IO_WORKER_F_FIXED = 8, /* static idle worker */ + IO_WORKER_F_BOUND = 16, /* is doing bounded work */ }; enum { -- cgit v1.2.3-58-ga151 From 291b2821e072e16b062c5a0e83f7642143c4399a Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 30 Sep 2020 22:57:01 +0300 Subject: io_uring: simplify io_alloc_req() Extract common code from if/else branches. That is cleaner and optimised even better. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index fcb4e95cb88d..cdd59467576d 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1650,10 +1650,8 @@ static struct io_kiocb *io_get_fallback_req(struct io_ring_ctx *ctx) static struct io_kiocb *io_alloc_req(struct io_ring_ctx *ctx, struct io_submit_state *state) { - gfp_t gfp = GFP_KERNEL | __GFP_NOWARN; - struct io_kiocb *req; - if (!state->free_reqs) { + gfp_t gfp = GFP_KERNEL | __GFP_NOWARN; size_t sz; int ret; @@ -1670,14 +1668,11 @@ static struct io_kiocb *io_alloc_req(struct io_ring_ctx *ctx, goto fallback; ret = 1; } - state->free_reqs = ret - 1; - req = state->reqs[ret - 1]; - } else { - state->free_reqs--; - req = state->reqs[state->free_reqs]; + state->free_reqs = ret; } - return req; + state->free_reqs--; + return state->reqs[state->free_reqs]; fallback: return io_get_fallback_req(ctx); } -- cgit v1.2.3-58-ga151 From 5b09e37e27a878eb50f0eb96fbce8419e932a7d5 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 30 Sep 2020 22:57:15 +0300 Subject: io_uring: io_kiocb_ppos() style change Put brackets around bitwise ops in a complex expression Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index cdd59467576d..513b3a59af37 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -3008,7 +3008,7 @@ static ssize_t io_import_iovec(int rw, struct io_kiocb *req, static inline loff_t *io_kiocb_ppos(struct kiocb *kiocb) { - return kiocb->ki_filp->f_mode & FMODE_STREAM ? NULL : &kiocb->ki_pos; + return (kiocb->ki_filp->f_mode & FMODE_STREAM) ? NULL : &kiocb->ki_pos; } /* -- cgit v1.2.3-58-ga151 From 2d199895d231c0a1af3a49d1f0da777499f352c8 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 30 Sep 2020 22:57:35 +0300 Subject: io_uring: remove F_NEED_CLEANUP check in *prep() REQ_F_NEED_CLEANUP is set only by io_*_prep() and they're guaranteed to be called only once, so there is no one who may have set the flag before. Kill REQ_F_NEED_CLEANUP check in these *prep() handlers. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 513b3a59af37..c0248dc3cdf5 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -3157,7 +3157,7 @@ static int io_read_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe, return -EBADF; /* either don't need iovec imported or already have it */ - if (!req->async_data || req->flags & REQ_F_NEED_CLEANUP) + if (!req->async_data) return 0; return io_rw_prep_async(req, READ, force_nonblock); } @@ -3381,7 +3381,7 @@ static int io_write_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe, return -EBADF; /* either don't need iovec imported or already have it */ - if (!req->async_data || req->flags & REQ_F_NEED_CLEANUP) + if (!req->async_data) return 0; return io_rw_prep_async(req, WRITE, force_nonblock); } @@ -3482,8 +3482,6 @@ static int __io_splice_prep(struct io_kiocb *req, unsigned int valid_flags = SPLICE_F_FD_IN_FIXED | SPLICE_F_ALL; int ret; - if (req->flags & REQ_F_NEED_CLEANUP) - return 0; if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL)) return -EINVAL; @@ -3693,8 +3691,6 @@ static int io_openat_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) if (unlikely(req->ctx->flags & (IORING_SETUP_IOPOLL|IORING_SETUP_SQPOLL))) return -EINVAL; - if (req->flags & REQ_F_NEED_CLEANUP) - return 0; mode = READ_ONCE(sqe->len); flags = READ_ONCE(sqe->open_flags); req->open.how = build_open_how(flags, mode); @@ -3709,8 +3705,6 @@ static int io_openat2_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) if (unlikely(req->ctx->flags & (IORING_SETUP_IOPOLL|IORING_SETUP_SQPOLL))) return -EINVAL; - if (req->flags & REQ_F_NEED_CLEANUP) - return 0; how = u64_to_user_ptr(READ_ONCE(sqe->addr2)); len = READ_ONCE(sqe->len); if (len < OPEN_HOW_SIZE_VER0) @@ -4218,10 +4212,6 @@ static int io_sendmsg_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) if (!async_msg || !io_op_defs[req->opcode].needs_async_data) return 0; - /* iovec is already imported */ - if (req->flags & REQ_F_NEED_CLEANUP) - return 0; - ret = io_sendmsg_copy_hdr(req, async_msg); if (!ret) req->flags |= REQ_F_NEED_CLEANUP; @@ -4448,10 +4438,6 @@ static int io_recvmsg_prep(struct io_kiocb *req, if (!async_msg || !io_op_defs[req->opcode].needs_async_data) return 0; - /* iovec is already imported */ - if (req->flags & REQ_F_NEED_CLEANUP) - return 0; - ret = io_recvmsg_copy_hdr(req, async_msg); if (!ret) req->flags |= REQ_F_NEED_CLEANUP; -- cgit v1.2.3-58-ga151 From a88fc400212fc1d8aa9ca4979f898fd04ca3aab5 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 30 Sep 2020 22:57:53 +0300 Subject: io_uring: set/clear IOCB_NOWAIT into io_read/write Move setting IOCB_NOWAIT from io_prep_rw() into io_read()/io_write(), so it's set/cleared in a single place. Also remove @force_nonblock parameter from io_prep_rw(). Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index c0248dc3cdf5..32053ad84d1a 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -2633,8 +2633,7 @@ static bool io_file_supports_async(struct file *file, int rw) return file->f_op->write_iter != NULL; } -static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe, - bool force_nonblock) +static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe) { struct io_ring_ctx *ctx = req->ctx; struct kiocb *kiocb = &req->rw.kiocb; @@ -2669,9 +2668,6 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe, if (kiocb->ki_flags & IOCB_NOWAIT) req->flags |= REQ_F_NOWAIT; - if (force_nonblock) - kiocb->ki_flags |= IOCB_NOWAIT; - if (ctx->flags & IORING_SETUP_IOPOLL) { if (!(kiocb->ki_flags & IOCB_DIRECT) || !kiocb->ki_filp->f_op->iopoll) @@ -3149,7 +3145,7 @@ static int io_read_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe, { ssize_t ret; - ret = io_prep_rw(req, sqe, force_nonblock); + ret = io_prep_rw(req, sqe); if (ret) return ret; @@ -3284,6 +3280,9 @@ static int io_read(struct io_kiocb *req, bool force_nonblock, /* Ensure we clear previously set non-block flag */ if (!force_nonblock) kiocb->ki_flags &= ~IOCB_NOWAIT; + else + kiocb->ki_flags |= IOCB_NOWAIT; + /* If the file doesn't support async, just async punt */ no_async = force_nonblock && !io_file_supports_async(req->file, READ); @@ -3373,7 +3372,7 @@ static int io_write_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe, { ssize_t ret; - ret = io_prep_rw(req, sqe, force_nonblock); + ret = io_prep_rw(req, sqe); if (ret) return ret; @@ -3408,7 +3407,9 @@ static int io_write(struct io_kiocb *req, bool force_nonblock, /* Ensure we clear previously set non-block flag */ if (!force_nonblock) - req->rw.kiocb.ki_flags &= ~IOCB_NOWAIT; + kiocb->ki_flags &= ~IOCB_NOWAIT; + else + kiocb->ki_flags |= IOCB_NOWAIT; /* If the file doesn't support async, just async punt */ if (force_nonblock && !io_file_supports_async(req->file, WRITE)) -- cgit v1.2.3-58-ga151 From 73debe68b300ca24fdbbe9b6c3787d3e239deb3d Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 30 Sep 2020 22:57:54 +0300 Subject: io_uring: remove nonblock arg from io_{rw}_prep() All io_*_prep() functions including io_{read,write}_prep() are called only during submission where @force_nonblock is always true. Don't keep propagating it and instead remove the @force_nonblock argument from prep() altogether. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 32053ad84d1a..b0844296ad90 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -3122,14 +3122,13 @@ static int io_setup_async_rw(struct io_kiocb *req, const struct iovec *iovec, return 0; } -static inline int io_rw_prep_async(struct io_kiocb *req, int rw, - bool force_nonblock) +static inline int io_rw_prep_async(struct io_kiocb *req, int rw) { struct io_async_rw *iorw = req->async_data; struct iovec *iov = iorw->fast_iov; ssize_t ret; - ret = __io_import_iovec(rw, req, &iov, &iorw->iter, !force_nonblock); + ret = __io_import_iovec(rw, req, &iov, &iorw->iter, false); if (unlikely(ret < 0)) return ret; @@ -3140,8 +3139,7 @@ static inline int io_rw_prep_async(struct io_kiocb *req, int rw, return 0; } -static int io_read_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe, - bool force_nonblock) +static int io_read_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) { ssize_t ret; @@ -3155,7 +3153,7 @@ static int io_read_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe, /* either don't need iovec imported or already have it */ if (!req->async_data) return 0; - return io_rw_prep_async(req, READ, force_nonblock); + return io_rw_prep_async(req, READ); } /* @@ -3367,8 +3365,7 @@ out_free: return ret; } -static int io_write_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe, - bool force_nonblock) +static int io_write_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) { ssize_t ret; @@ -3382,7 +3379,7 @@ static int io_write_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe, /* either don't need iovec imported or already have it */ if (!req->async_data) return 0; - return io_rw_prep_async(req, WRITE, force_nonblock); + return io_rw_prep_async(req, WRITE); } static int io_write(struct io_kiocb *req, bool force_nonblock, @@ -5615,12 +5612,12 @@ static int io_req_defer_prep(struct io_kiocb *req, case IORING_OP_READV: case IORING_OP_READ_FIXED: case IORING_OP_READ: - ret = io_read_prep(req, sqe, true); + ret = io_read_prep(req, sqe); break; case IORING_OP_WRITEV: case IORING_OP_WRITE_FIXED: case IORING_OP_WRITE: - ret = io_write_prep(req, sqe, true); + ret = io_write_prep(req, sqe); break; case IORING_OP_POLL_ADD: ret = io_poll_add_prep(req, sqe); @@ -5851,7 +5848,7 @@ static int io_issue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe, case IORING_OP_READ_FIXED: case IORING_OP_READ: if (sqe) { - ret = io_read_prep(req, sqe, force_nonblock); + ret = io_read_prep(req, sqe); if (ret < 0) break; } @@ -5861,7 +5858,7 @@ static int io_issue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe, case IORING_OP_WRITE_FIXED: case IORING_OP_WRITE: if (sqe) { - ret = io_write_prep(req, sqe, force_nonblock); + ret = io_write_prep(req, sqe); if (ret < 0) break; } -- cgit v1.2.3-58-ga151 From bfe76559833d5d76fc4eebdad7658d22522f8a22 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 30 Sep 2020 22:57:55 +0300 Subject: io_uring: decouple issuing and req preparation io_issue_sqe() does two things at once, trying to prepare request and issuing them. Split it in two and deduplicate with io_defer_prep(). Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 263 +++++++++++++--------------------------------------------- 1 file changed, 57 insertions(+), 206 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index b0844296ad90..5514d88ebcc8 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -5590,120 +5590,95 @@ static int io_files_update(struct io_kiocb *req, bool force_nonblock, return 0; } -static int io_req_defer_prep(struct io_kiocb *req, - const struct io_uring_sqe *sqe) +static int io_req_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) { - ssize_t ret = 0; - - if (!sqe) - return 0; - - if (io_alloc_async_data(req)) - return -EAGAIN; - ret = io_prep_work_files(req); - if (unlikely(ret)) - return ret; - - io_prep_async_work(req); - switch (req->opcode) { case IORING_OP_NOP: - break; + return 0; case IORING_OP_READV: case IORING_OP_READ_FIXED: case IORING_OP_READ: - ret = io_read_prep(req, sqe); - break; + return io_read_prep(req, sqe); case IORING_OP_WRITEV: case IORING_OP_WRITE_FIXED: case IORING_OP_WRITE: - ret = io_write_prep(req, sqe); - break; + return io_write_prep(req, sqe); case IORING_OP_POLL_ADD: - ret = io_poll_add_prep(req, sqe); - break; + return io_poll_add_prep(req, sqe); case IORING_OP_POLL_REMOVE: - ret = io_poll_remove_prep(req, sqe); - break; + return io_poll_remove_prep(req, sqe); case IORING_OP_FSYNC: - ret = io_prep_fsync(req, sqe); - break; + return io_prep_fsync(req, sqe); case IORING_OP_SYNC_FILE_RANGE: - ret = io_prep_sfr(req, sqe); - break; + return io_prep_sfr(req, sqe); case IORING_OP_SENDMSG: case IORING_OP_SEND: - ret = io_sendmsg_prep(req, sqe); - break; + return io_sendmsg_prep(req, sqe); case IORING_OP_RECVMSG: case IORING_OP_RECV: - ret = io_recvmsg_prep(req, sqe); - break; + return io_recvmsg_prep(req, sqe); case IORING_OP_CONNECT: - ret = io_connect_prep(req, sqe); - break; + return io_connect_prep(req, sqe); case IORING_OP_TIMEOUT: - ret = io_timeout_prep(req, sqe, false); - break; + return io_timeout_prep(req, sqe, false); case IORING_OP_TIMEOUT_REMOVE: - ret = io_timeout_remove_prep(req, sqe); - break; + return io_timeout_remove_prep(req, sqe); case IORING_OP_ASYNC_CANCEL: - ret = io_async_cancel_prep(req, sqe); - break; + return io_async_cancel_prep(req, sqe); case IORING_OP_LINK_TIMEOUT: - ret = io_timeout_prep(req, sqe, true); - break; + return io_timeout_prep(req, sqe, true); case IORING_OP_ACCEPT: - ret = io_accept_prep(req, sqe); - break; + return io_accept_prep(req, sqe); case IORING_OP_FALLOCATE: - ret = io_fallocate_prep(req, sqe); - break; + return io_fallocate_prep(req, sqe); case IORING_OP_OPENAT: - ret = io_openat_prep(req, sqe); - break; + return io_openat_prep(req, sqe); case IORING_OP_CLOSE: - ret = io_close_prep(req, sqe); - break; + return io_close_prep(req, sqe); case IORING_OP_FILES_UPDATE: - ret = io_files_update_prep(req, sqe); - break; + return io_files_update_prep(req, sqe); case IORING_OP_STATX: - ret = io_statx_prep(req, sqe); - break; + return io_statx_prep(req, sqe); case IORING_OP_FADVISE: - ret = io_fadvise_prep(req, sqe); - break; + return io_fadvise_prep(req, sqe); case IORING_OP_MADVISE: - ret = io_madvise_prep(req, sqe); - break; + return io_madvise_prep(req, sqe); case IORING_OP_OPENAT2: - ret = io_openat2_prep(req, sqe); - break; + return io_openat2_prep(req, sqe); case IORING_OP_EPOLL_CTL: - ret = io_epoll_ctl_prep(req, sqe); - break; + return io_epoll_ctl_prep(req, sqe); case IORING_OP_SPLICE: - ret = io_splice_prep(req, sqe); - break; + return io_splice_prep(req, sqe); case IORING_OP_PROVIDE_BUFFERS: - ret = io_provide_buffers_prep(req, sqe); - break; + return io_provide_buffers_prep(req, sqe); case IORING_OP_REMOVE_BUFFERS: - ret = io_remove_buffers_prep(req, sqe); - break; + return io_remove_buffers_prep(req, sqe); case IORING_OP_TEE: - ret = io_tee_prep(req, sqe); - break; - default: - printk_once(KERN_WARNING "io_uring: unhandled opcode %d\n", - req->opcode); - ret = -EINVAL; - break; + return io_tee_prep(req, sqe); } - return ret; + printk_once(KERN_WARNING "io_uring: unhandled opcode %d\n", + req->opcode); + return-EINVAL; +} + +static int io_req_defer_prep(struct io_kiocb *req, + const struct io_uring_sqe *sqe) +{ + int ret; + + if (!sqe) + return 0; + if (io_alloc_async_data(req)) + return -EAGAIN; + + ret = io_prep_work_files(req); + if (unlikely(ret)) + return ret; + + io_prep_async_work(req); + + return io_req_prep(req, sqe); } static u32 io_get_sequence(struct io_kiocb *req) @@ -5840,6 +5815,12 @@ static int io_issue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe, struct io_ring_ctx *ctx = req->ctx; int ret; + if (sqe) { + ret = io_req_prep(req, sqe); + if (unlikely(ret < 0)) + return ret; + } + switch (req->opcode) { case IORING_OP_NOP: ret = io_nop(req, cs); @@ -5847,62 +5828,27 @@ static int io_issue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe, case IORING_OP_READV: case IORING_OP_READ_FIXED: case IORING_OP_READ: - if (sqe) { - ret = io_read_prep(req, sqe); - if (ret < 0) - break; - } ret = io_read(req, force_nonblock, cs); break; case IORING_OP_WRITEV: case IORING_OP_WRITE_FIXED: case IORING_OP_WRITE: - if (sqe) { - ret = io_write_prep(req, sqe); - if (ret < 0) - break; - } ret = io_write(req, force_nonblock, cs); break; case IORING_OP_FSYNC: - if (sqe) { - ret = io_prep_fsync(req, sqe); - if (ret < 0) - break; - } ret = io_fsync(req, force_nonblock); break; case IORING_OP_POLL_ADD: - if (sqe) { - ret = io_poll_add_prep(req, sqe); - if (ret) - break; - } ret = io_poll_add(req); break; case IORING_OP_POLL_REMOVE: - if (sqe) { - ret = io_poll_remove_prep(req, sqe); - if (ret < 0) - break; - } ret = io_poll_remove(req); break; case IORING_OP_SYNC_FILE_RANGE: - if (sqe) { - ret = io_prep_sfr(req, sqe); - if (ret < 0) - break; - } ret = io_sync_file_range(req, force_nonblock); break; case IORING_OP_SENDMSG: case IORING_OP_SEND: - if (sqe) { - ret = io_sendmsg_prep(req, sqe); - if (ret < 0) - break; - } if (req->opcode == IORING_OP_SENDMSG) ret = io_sendmsg(req, force_nonblock, cs); else @@ -5910,158 +5856,63 @@ static int io_issue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe, break; case IORING_OP_RECVMSG: case IORING_OP_RECV: - if (sqe) { - ret = io_recvmsg_prep(req, sqe); - if (ret) - break; - } if (req->opcode == IORING_OP_RECVMSG) ret = io_recvmsg(req, force_nonblock, cs); else ret = io_recv(req, force_nonblock, cs); break; case IORING_OP_TIMEOUT: - if (sqe) { - ret = io_timeout_prep(req, sqe, false); - if (ret) - break; - } ret = io_timeout(req); break; case IORING_OP_TIMEOUT_REMOVE: - if (sqe) { - ret = io_timeout_remove_prep(req, sqe); - if (ret) - break; - } ret = io_timeout_remove(req); break; case IORING_OP_ACCEPT: - if (sqe) { - ret = io_accept_prep(req, sqe); - if (ret) - break; - } ret = io_accept(req, force_nonblock, cs); break; case IORING_OP_CONNECT: - if (sqe) { - ret = io_connect_prep(req, sqe); - if (ret) - break; - } ret = io_connect(req, force_nonblock, cs); break; case IORING_OP_ASYNC_CANCEL: - if (sqe) { - ret = io_async_cancel_prep(req, sqe); - if (ret) - break; - } ret = io_async_cancel(req); break; case IORING_OP_FALLOCATE: - if (sqe) { - ret = io_fallocate_prep(req, sqe); - if (ret) - break; - } ret = io_fallocate(req, force_nonblock); break; case IORING_OP_OPENAT: - if (sqe) { - ret = io_openat_prep(req, sqe); - if (ret) - break; - } ret = io_openat(req, force_nonblock); break; case IORING_OP_CLOSE: - if (sqe) { - ret = io_close_prep(req, sqe); - if (ret) - break; - } ret = io_close(req, force_nonblock, cs); break; case IORING_OP_FILES_UPDATE: - if (sqe) { - ret = io_files_update_prep(req, sqe); - if (ret) - break; - } ret = io_files_update(req, force_nonblock, cs); break; case IORING_OP_STATX: - if (sqe) { - ret = io_statx_prep(req, sqe); - if (ret) - break; - } ret = io_statx(req, force_nonblock); break; case IORING_OP_FADVISE: - if (sqe) { - ret = io_fadvise_prep(req, sqe); - if (ret) - break; - } ret = io_fadvise(req, force_nonblock); break; case IORING_OP_MADVISE: - if (sqe) { - ret = io_madvise_prep(req, sqe); - if (ret) - break; - } ret = io_madvise(req, force_nonblock); break; case IORING_OP_OPENAT2: - if (sqe) { - ret = io_openat2_prep(req, sqe); - if (ret) - break; - } ret = io_openat2(req, 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, force_nonblock, cs); break; case IORING_OP_SPLICE: - if (sqe) { - ret = io_splice_prep(req, sqe); - if (ret < 0) - break; - } ret = io_splice(req, force_nonblock); break; case IORING_OP_PROVIDE_BUFFERS: - if (sqe) { - ret = io_provide_buffers_prep(req, sqe); - if (ret) - break; - } ret = io_provide_buffers(req, force_nonblock, cs); break; case IORING_OP_REMOVE_BUFFERS: - if (sqe) { - ret = io_remove_buffers_prep(req, sqe); - if (ret) - break; - } ret = io_remove_buffers(req, force_nonblock, cs); break; case IORING_OP_TEE: - if (sqe) { - ret = io_tee_prep(req, sqe); - if (ret < 0) - break; - } ret = io_tee(req, force_nonblock); break; default: -- cgit v1.2.3-58-ga151 From c1379e247a724b2d4c646f1947d4f78c782c0650 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Wed, 30 Sep 2020 22:57:56 +0300 Subject: io_uring: move req preps out of io_issue_sqe() All request preparations are done only during submission, reflect it in the code by moving io_req_prep() much earlier into io_queue_sqe(). That's much cleaner, because it doen't expose bits to async code which it won't ever use. Also it makes the interface harder to misuse, and there are potential places for bugs. For instance, __io_queue() doesn't clear @sqe before proceeding to a next linked request, that could have been disastrous, but hopefully there are linked requests IFF sqe==NULL, so not actually a bug. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 5514d88ebcc8..f617f1a725e1 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -971,9 +971,7 @@ static int io_prep_work_files(struct io_kiocb *req); static void __io_clean_op(struct io_kiocb *req); static int io_file_get(struct io_submit_state *state, struct io_kiocb *req, int fd, struct file **out_file, bool fixed); -static void __io_queue_sqe(struct io_kiocb *req, - const struct io_uring_sqe *sqe, - struct io_comp_state *cs); +static void __io_queue_sqe(struct io_kiocb *req, struct io_comp_state *cs); static void io_file_put_work(struct work_struct *work); static ssize_t io_import_iovec(int rw, struct io_kiocb *req, @@ -1944,7 +1942,7 @@ static void __io_req_task_submit(struct io_kiocb *req) if (!__io_sq_thread_acquire_mm(ctx)) { mutex_lock(&ctx->uring_lock); - __io_queue_sqe(req, NULL, NULL); + __io_queue_sqe(req, NULL); mutex_unlock(&ctx->uring_lock); } else { __io_req_task_cancel(req, -EFAULT); @@ -5809,18 +5807,12 @@ static void __io_clean_op(struct io_kiocb *req) io_req_drop_files(req); } -static int io_issue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe, - bool force_nonblock, struct io_comp_state *cs) +static int io_issue_sqe(struct io_kiocb *req, bool force_nonblock, + struct io_comp_state *cs) { struct io_ring_ctx *ctx = req->ctx; int ret; - if (sqe) { - ret = io_req_prep(req, sqe); - if (unlikely(ret < 0)) - return ret; - } - switch (req->opcode) { case IORING_OP_NOP: ret = io_nop(req, cs); @@ -5958,7 +5950,7 @@ static struct io_wq_work *io_wq_submit_work(struct io_wq_work *work) if (!ret) { do { - ret = io_issue_sqe(req, NULL, false, NULL); + ret = io_issue_sqe(req, false, NULL); /* * We can get EAGAIN for polled IO even though we're * forcing a sync submission from here, since we can't @@ -6136,8 +6128,7 @@ static struct io_kiocb *io_prep_linked_timeout(struct io_kiocb *req) return nxt; } -static void __io_queue_sqe(struct io_kiocb *req, const struct io_uring_sqe *sqe, - struct io_comp_state *cs) +static void __io_queue_sqe(struct io_kiocb *req, struct io_comp_state *cs) { struct io_kiocb *linked_timeout; struct io_kiocb *nxt; @@ -6157,7 +6148,7 @@ again: old_creds = override_creds(req->work.creds); } - ret = io_issue_sqe(req, sqe, true, cs); + ret = io_issue_sqe(req, true, cs); /* * We async punt it if the file wasn't marked NOWAIT, or if the file @@ -6236,7 +6227,12 @@ fail_req: req->work.flags |= IO_WQ_WORK_CONCURRENT; io_queue_async_work(req); } else { - __io_queue_sqe(req, sqe, cs); + if (sqe) { + ret = io_req_prep(req, sqe); + if (unlikely(ret)) + goto fail_req; + } + __io_queue_sqe(req, cs); } } -- cgit v1.2.3-58-ga151 From 87c4311fd2c28e83545cdfa4702b57db15ed1d9b Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Wed, 30 Sep 2020 21:00:14 -0600 Subject: io_uring: kill callback_head argument for io_req_task_work_add() We always use &req->task_work anyway, no point in passing it in. Signed-off-by: Jens Axboe --- fs/io_uring.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index f617f1a725e1..c409af7bd444 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -1886,8 +1886,7 @@ static struct io_kiocb *io_req_find_next(struct io_kiocb *req) return __io_req_find_next(req); } -static int io_req_task_work_add(struct io_kiocb *req, struct callback_head *cb, - bool twa_signal_ok) +static int io_req_task_work_add(struct io_kiocb *req, bool twa_signal_ok) { struct task_struct *tsk = req->task; struct io_ring_ctx *ctx = req->ctx; @@ -1906,7 +1905,7 @@ static int io_req_task_work_add(struct io_kiocb *req, struct callback_head *cb, if (!(ctx->flags & IORING_SETUP_SQPOLL) && twa_signal_ok) notify = TWA_SIGNAL; - ret = task_work_add(tsk, cb, notify); + ret = task_work_add(tsk, &req->task_work, notify); if (!ret) wake_up_process(tsk); @@ -1965,7 +1964,7 @@ static void io_req_task_queue(struct io_kiocb *req) init_task_work(&req->task_work, io_req_task_submit); percpu_ref_get(&req->ctx->refs); - ret = io_req_task_work_add(req, &req->task_work, true); + ret = io_req_task_work_add(req, true); if (unlikely(ret)) { struct task_struct *tsk; @@ -3185,7 +3184,7 @@ static int io_async_buf_func(struct wait_queue_entry *wait, unsigned mode, /* submit ref gets dropped, acquire a new one */ refcount_inc(&req->refs); - ret = io_req_task_work_add(req, &req->task_work, true); + ret = io_req_task_work_add(req, true); if (unlikely(ret)) { struct task_struct *tsk; @@ -4752,7 +4751,7 @@ static int __io_async_wake(struct io_kiocb *req, struct io_poll_iocb *poll, * of executing it. We can't safely execute it anyway, as we may not * have the needed state needed for it anyway. */ - ret = io_req_task_work_add(req, &req->task_work, twa_signal_ok); + ret = io_req_task_work_add(req, twa_signal_ok); if (unlikely(ret)) { struct task_struct *tsk; -- cgit v1.2.3-58-ga151 From faf7b51c06973f947776af6c8f8a513475a2bfa1 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Wed, 7 Oct 2020 12:48:53 -0600 Subject: io_uring: batch account ->req_issue and task struct references Identical to how we handle the ctx reference counts, increase by the batch we're expecting to submit, and handle any slow path residual, if any. The request alloc-and-issue path is very hot, and this makes a noticeable difference by avoiding an two atomic incs for each individual request. Signed-off-by: Jens Axboe --- fs/io_uring.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index c409af7bd444..85e8ad9970be 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -6429,8 +6429,6 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req, /* one is dropped after submission, the other at completion */ refcount_set(&req->refs, 2); req->task = current; - get_task_struct(req->task); - atomic_long_inc(&req->task->io_uring->req_issue); req->result = 0; if (unlikely(req->opcode >= IORING_OP_LAST)) @@ -6488,6 +6486,9 @@ static int io_submit_sqes(struct io_ring_ctx *ctx, unsigned int nr) if (!percpu_ref_tryget_many(&ctx->refs, nr)) return -EAGAIN; + atomic_long_add(nr, ¤t->io_uring->req_issue); + refcount_add(nr, ¤t->usage); + io_submit_state_start(&state, ctx, nr); for (i = 0; i < nr; i++) { @@ -6530,6 +6531,8 @@ fail_req: int ref_used = (submitted == -EAGAIN) ? 0 : submitted; percpu_ref_put_many(&ctx->refs, nr - ref_used); + atomic_long_sub(nr - ref_used, ¤t->io_uring->req_issue); + put_task_struct_many(current, nr - ref_used); } if (link) io_queue_link_head(link, &state.comp); -- cgit v1.2.3-58-ga151 From ca6484cd308a671811bf39f3119e81966eb476e3 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Thu, 8 Oct 2020 07:46:52 -0600 Subject: io_uring: no need to call xa_destroy() on empty xarray The kernel test robot reports this lockdep issue: [child1:659] mbind (274) returned ENOSYS, marking as inactive. [child1:659] mq_timedsend (279) returned ENOSYS, marking as inactive. [main] 10175 iterations. [F:7781 S:2344 HI:2397] [ 24.610601] [ 24.610743] ================================ [ 24.611083] WARNING: inconsistent lock state [ 24.611437] 5.9.0-rc7-00017-g0f2122045b9462 #5 Not tainted [ 24.611861] -------------------------------- [ 24.612193] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage. [ 24.612660] ksoftirqd/0/7 [HC0[0]:SC1[3]:HE0:SE0] takes: [ 24.613086] f00ed998 (&xa->xa_lock#4){+.?.}-{2:2}, at: xa_destroy+0x43/0xc1 [ 24.613642] {SOFTIRQ-ON-W} state was registered at: [ 24.614024] lock_acquire+0x20c/0x29b [ 24.614341] _raw_spin_lock+0x21/0x30 [ 24.614636] io_uring_add_task_file+0xe8/0x13a [ 24.614987] io_uring_create+0x535/0x6bd [ 24.615297] io_uring_setup+0x11d/0x136 [ 24.615606] __ia32_sys_io_uring_setup+0xd/0xf [ 24.615977] do_int80_syscall_32+0x53/0x6c [ 24.616306] restore_all_switch_stack+0x0/0xb1 [ 24.616677] irq event stamp: 939881 [ 24.616968] hardirqs last enabled at (939880): [<8105592d>] __local_bh_enable_ip+0x13c/0x145 [ 24.617642] hardirqs last disabled at (939881): [<81b6ace3>] _raw_spin_lock_irqsave+0x1b/0x4e [ 24.618321] softirqs last enabled at (939738): [<81b6c7c8>] __do_softirq+0x3f0/0x45a [ 24.618924] softirqs last disabled at (939743): [<81055741>] run_ksoftirqd+0x35/0x61 [ 24.619521] [ 24.619521] other info that might help us debug this: [ 24.620028] Possible unsafe locking scenario: [ 24.620028] [ 24.620492] CPU0 [ 24.620685] ---- [ 24.620894] lock(&xa->xa_lock#4); [ 24.621168] [ 24.621381] lock(&xa->xa_lock#4); [ 24.621695] [ 24.621695] *** DEADLOCK *** [ 24.621695] [ 24.622154] 1 lock held by ksoftirqd/0/7: [ 24.622468] #0: 823bfb94 (rcu_callback){....}-{0:0}, at: rcu_process_callbacks+0xc0/0x155 [ 24.623106] [ 24.623106] stack backtrace: [ 24.623454] CPU: 0 PID: 7 Comm: ksoftirqd/0 Not tainted 5.9.0-rc7-00017-g0f2122045b9462 #5 [ 24.624090] Call Trace: [ 24.624284] ? show_stack+0x40/0x46 [ 24.624551] dump_stack+0x1b/0x1d [ 24.624809] print_usage_bug+0x17a/0x185 [ 24.625142] mark_lock+0x11d/0x1db [ 24.625474] ? print_shortest_lock_dependencies+0x121/0x121 [ 24.625905] __lock_acquire+0x41e/0x7bf [ 24.626206] lock_acquire+0x20c/0x29b [ 24.626517] ? xa_destroy+0x43/0xc1 [ 24.626810] ? lock_acquire+0x20c/0x29b [ 24.627110] _raw_spin_lock_irqsave+0x3e/0x4e [ 24.627450] ? xa_destroy+0x43/0xc1 [ 24.627725] xa_destroy+0x43/0xc1 [ 24.627989] __io_uring_free+0x57/0x71 [ 24.628286] ? get_pid+0x22/0x22 [ 24.628544] __put_task_struct+0xf2/0x163 [ 24.628865] put_task_struct+0x1f/0x2a [ 24.629161] delayed_put_task_struct+0xe2/0xe9 [ 24.629509] rcu_process_callbacks+0x128/0x155 [ 24.629860] __do_softirq+0x1a3/0x45a [ 24.630151] run_ksoftirqd+0x35/0x61 [ 24.630443] smpboot_thread_fn+0x304/0x31a [ 24.630763] kthread+0x124/0x139 [ 24.631016] ? sort_range+0x18/0x18 [ 24.631290] ? kthread_create_worker_on_cpu+0x17/0x17 [ 24.631682] ret_from_fork+0x1c/0x28 which is complaining about xa_destroy() grabbing the xa lock in an IRQ disabling fashion, whereas the io_uring uses cases aren't interrupt safe. This is really an xarray issue, since it should not assume the lock type. But for our use case, since we know the xarray is empty at this point, there's no need to actually call xa_destroy(). So just get rid of it. Fixes: 0f2122045b94 ("io_uring: don't rely on weak ->files references") Reported-by: kernel test robot Signed-off-by: Jens Axboe --- fs/io_uring.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 85e8ad9970be..4df5b14c2e56 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -7688,7 +7688,6 @@ void __io_uring_free(struct task_struct *tsk) struct io_uring_task *tctx = tsk->io_uring; WARN_ON_ONCE(!xa_empty(&tctx->xa)); - xa_destroy(&tctx->xa); kfree(tctx); tsk->io_uring = NULL; } -- cgit v1.2.3-58-ga151 From ed6930c9201cd1e00f74474da2f095796a0d82f6 Mon Sep 17 00:00:00 2001 From: Jens Axboe Date: Thu, 8 Oct 2020 19:09:46 -0600 Subject: io_uring: fix break condition for __io_uring_register() waiting Colin reports that there's unreachable code, since we only ever break if ret == 0. This is correct, and is due to a reversed logic condition in when to break or not. Break out of the loop if we don't process any task work, in that case we do want to return -EINTR. Fixes: af9c1a44f8de ("io_uring: process task work in io_uring_register()") Reported-by: Colin Ian King Signed-off-by: Jens Axboe --- fs/io_uring.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 4df5b14c2e56..299c530c66f9 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -9511,15 +9511,15 @@ static int __io_uring_register(struct io_ring_ctx *ctx, unsigned opcode, ret = wait_for_completion_interruptible(&ctx->ref_comp); if (!ret) break; - if (io_run_task_work_sig() > 0) - continue; + ret = io_run_task_work_sig(); + if (ret < 0) + break; } while (1); mutex_lock(&ctx->uring_lock); if (ret) { percpu_ref_resurrect(&ctx->refs); - ret = -EINTR; goto out_quiesce; } } -- cgit v1.2.3-58-ga151 From ce765372bc443573d1d339a2bf4995de385dea3a Mon Sep 17 00:00:00 2001 From: "Matthew Wilcox (Oracle)" Date: Fri, 9 Oct 2020 13:49:51 +0100 Subject: io_uring: Fix use of XArray in __io_uring_files_cancel We have to drop the lock during each iteration, so there's no advantage to using the advanced API. Convert this to a standard xa_for_each() loop. Reported-by: syzbot+27c12725d8ff0bfe1a13@syzkaller.appspotmail.com Signed-off-by: Matthew Wilcox (Oracle) Signed-off-by: Jens Axboe --- fs/io_uring.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 299c530c66f9..2978cc78538a 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -8665,28 +8665,19 @@ static void io_uring_attempt_task_drop(struct file *file, bool exiting) void __io_uring_files_cancel(struct files_struct *files) { struct io_uring_task *tctx = current->io_uring; - XA_STATE(xas, &tctx->xa, 0); + struct file *file; + unsigned long index; /* make sure overflow events are dropped */ tctx->in_idle = true; - do { - struct io_ring_ctx *ctx; - struct file *file; - - xas_lock(&xas); - file = xas_next_entry(&xas, ULONG_MAX); - xas_unlock(&xas); - - if (!file) - break; - - ctx = file->private_data; + xa_for_each(&tctx->xa, index, file) { + struct io_ring_ctx *ctx = file->private_data; io_uring_cancel_task_requests(ctx, files); if (files) io_uring_del_task_file(file); - } while (1); + } } static inline bool io_uring_task_idle(struct io_uring_task *tctx) -- cgit v1.2.3-58-ga151 From 236434c3438c4da3dfbd6aeeab807577b85e951a Mon Sep 17 00:00:00 2001 From: "Matthew Wilcox (Oracle)" Date: Fri, 9 Oct 2020 13:49:52 +0100 Subject: io_uring: Fix XArray usage in io_uring_add_task_file The xas_store() wasn't paired with an xas_nomem() loop, so if it couldn't allocate memory using GFP_NOWAIT, it would leak the reference to the file descriptor. Also the node pointed to by the xas could be freed between the call to xas_load() under the rcu_read_lock() and the acquisition of the xa_lock. It's easier to just use the normal xa_load/xa_store interface here. Signed-off-by: Matthew Wilcox (Oracle) [axboe: fix missing assign after alloc, cur_uring -> tctx rename] Signed-off-by: Jens Axboe --- fs/io_uring.c | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 2978cc78538a..13fc60ab0490 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -8586,27 +8586,24 @@ static void io_uring_cancel_task_requests(struct io_ring_ctx *ctx, */ static int io_uring_add_task_file(struct file *file) { - if (unlikely(!current->io_uring)) { + struct io_uring_task *tctx = current->io_uring; + + if (unlikely(!tctx)) { int ret; ret = io_uring_alloc_task_context(current); if (unlikely(ret)) return ret; + tctx = current->io_uring; } - if (current->io_uring->last != file) { - XA_STATE(xas, ¤t->io_uring->xa, (unsigned long) file); - void *old; + if (tctx->last != file) { + void *old = xa_load(&tctx->xa, (unsigned long)file); - rcu_read_lock(); - old = xas_load(&xas); - if (old != file) { + if (!old) { get_file(file); - xas_lock(&xas); - xas_store(&xas, file); - xas_unlock(&xas); + xa_store(&tctx->xa, (unsigned long)file, file, GFP_KERNEL); } - rcu_read_unlock(); - current->io_uring->last = file; + tctx->last = file; } return 0; -- cgit v1.2.3-58-ga151 From 5e2ed8c4f45093698855b1f45cdf43efbf6dd498 Mon Sep 17 00:00:00 2001 From: "Matthew Wilcox (Oracle)" Date: Fri, 9 Oct 2020 13:49:53 +0100 Subject: io_uring: Convert advanced XArray uses to the normal API There are no bugs here that I've spotted, it's just easier to use the normal API and there are no performance advantages to using the more verbose advanced API. Signed-off-by: Matthew Wilcox (Oracle) Signed-off-by: Jens Axboe --- fs/io_uring.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 13fc60ab0490..09494ca1b990 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -8615,27 +8615,17 @@ static int io_uring_add_task_file(struct file *file) static void io_uring_del_task_file(struct file *file) { struct io_uring_task *tctx = current->io_uring; - XA_STATE(xas, &tctx->xa, (unsigned long) file); if (tctx->last == file) tctx->last = NULL; - - xas_lock(&xas); - file = xas_store(&xas, NULL); - xas_unlock(&xas); - + file = xa_erase(&tctx->xa, (unsigned long)file); if (file) fput(file); } static void __io_uring_attempt_task_drop(struct file *file) { - XA_STATE(xas, ¤t->io_uring->xa, (unsigned long) file); - struct file *old; - - rcu_read_lock(); - old = xas_load(&xas); - rcu_read_unlock(); + struct file *old = xa_load(¤t->io_uring->xa, (unsigned long)file); if (old == file) io_uring_del_task_file(file); -- cgit v1.2.3-58-ga151 From 5bf5e464f1acb1c031b4a290d63760bcb074c027 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sat, 10 Oct 2020 18:34:05 +0100 Subject: io_uring: don't io_prep_async_work() linked reqs There is no real reason left for preparing io-wq work context for linked requests in advance, remove it as this might become a bottleneck in some cases. Reported-by: Roman Gershman Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 09494ca1b990..272abe03a79e 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -5672,9 +5672,6 @@ static int io_req_defer_prep(struct io_kiocb *req, ret = io_prep_work_files(req); if (unlikely(ret)) return ret; - - io_prep_async_work(req); - return io_req_prep(req, sqe); } -- cgit v1.2.3-58-ga151 From 233295130e53c8dfe6dbef3f52634c3f7e44cd6a Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sat, 10 Oct 2020 18:34:06 +0100 Subject: io_uring: clean up ->files grabbing Move work.files grabbing into io_prep_async_work() to all other work resources initialisation. We don't need to keep it separately now, as ->ring_fd/file are gone. It also allows to not grab it when a request is not going to io-wq. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 52 +++++++++++++--------------------------------------- 1 file changed, 13 insertions(+), 39 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 272abe03a79e..3a65bcba5a7b 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -967,7 +967,6 @@ static void io_queue_linked_timeout(struct io_kiocb *req); static int __io_sqe_files_update(struct io_ring_ctx *ctx, struct io_uring_files_update *ip, unsigned nr_args); -static int io_prep_work_files(struct io_kiocb *req); static void __io_clean_op(struct io_kiocb *req); static int io_file_get(struct io_submit_state *state, struct io_kiocb *req, int fd, struct file **out_file, bool fixed); @@ -1222,16 +1221,28 @@ static bool io_req_clean_work(struct io_kiocb *req) static void io_prep_async_work(struct io_kiocb *req) { const struct io_op_def *def = &io_op_defs[req->opcode]; + struct io_ring_ctx *ctx = req->ctx; io_req_init_async(req); if (req->flags & REQ_F_ISREG) { - if (def->hash_reg_file || (req->ctx->flags & IORING_SETUP_IOPOLL)) + if (def->hash_reg_file || (ctx->flags & IORING_SETUP_IOPOLL)) io_wq_hash_work(&req->work, file_inode(req->file)); } else { if (def->unbound_nonreg_file) req->work.flags |= IO_WQ_WORK_UNBOUND; } + if (!req->work.files && io_op_defs[req->opcode].file_table && + !(req->flags & REQ_F_NO_FILE_TABLE)) { + req->work.files = get_files_struct(current); + get_nsproxy(current->nsproxy); + req->work.nsproxy = current->nsproxy; + req->flags |= REQ_F_INFLIGHT; + + spin_lock_irq(&ctx->inflight_lock); + list_add(&req->inflight_entry, &ctx->inflight_list); + spin_unlock_irq(&ctx->inflight_lock); + } if (!req->work.mm && def->needs_mm) { mmgrab(current->mm); req->work.mm = current->mm; @@ -5662,16 +5673,10 @@ static int io_req_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) static int io_req_defer_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe) { - int ret; - if (!sqe) return 0; if (io_alloc_async_data(req)) return -EAGAIN; - - ret = io_prep_work_files(req); - if (unlikely(ret)) - return ret; return io_req_prep(req, sqe); } @@ -6015,33 +6020,6 @@ static int io_req_set_file(struct io_submit_state *state, struct io_kiocb *req, return io_file_get(state, req, fd, &req->file, fixed); } -static int io_grab_files(struct io_kiocb *req) -{ - struct io_ring_ctx *ctx = req->ctx; - - io_req_init_async(req); - - if (req->work.files || (req->flags & REQ_F_NO_FILE_TABLE)) - return 0; - - req->work.files = get_files_struct(current); - get_nsproxy(current->nsproxy); - req->work.nsproxy = current->nsproxy; - req->flags |= REQ_F_INFLIGHT; - - spin_lock_irq(&ctx->inflight_lock); - list_add(&req->inflight_entry, &ctx->inflight_list); - spin_unlock_irq(&ctx->inflight_lock); - return 0; -} - -static inline int io_prep_work_files(struct io_kiocb *req) -{ - if (!io_op_defs[req->opcode].file_table) - return 0; - return io_grab_files(req); -} - static enum hrtimer_restart io_link_timeout_fn(struct hrtimer *timer) { struct io_timeout_data *data = container_of(timer, @@ -6153,9 +6131,6 @@ again: if (ret == -EAGAIN && !(req->flags & REQ_F_NOWAIT)) { if (!io_arm_poll_handler(req)) { punt: - ret = io_prep_work_files(req); - if (unlikely(ret)) - goto err; /* * Queued up for async execution, worker will release * submit reference when the iocb is actually submitted. @@ -6169,7 +6144,6 @@ punt: } if (unlikely(ret)) { -err: /* un-prep timeout, so it'll be killed as any other linked */ req->flags &= ~REQ_F_LINK_TIMEOUT; req_set_fail_links(req); -- cgit v1.2.3-58-ga151 From 479f517be57185087c2ae3c5e5aea0c3c6d4e5cc Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sat, 10 Oct 2020 18:34:07 +0100 Subject: io_uring: kill extra check in fixed io_file_get() ctx->nr_user_files == 0 IFF ctx->file_data == NULL and there fixed files are not used. Hence, verifying fds only against ctx->nr_user_files is enough. Remove the other check from hot path. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 3a65bcba5a7b..39c37cef9ce0 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -5987,8 +5987,7 @@ static int io_file_get(struct io_submit_state *state, struct io_kiocb *req, struct file *file; if (fixed) { - if (unlikely(!ctx->file_data || - (unsigned) fd >= ctx->nr_user_files)) + if (unlikely((unsigned int)fd >= ctx->nr_user_files)) return -EBADF; fd = array_index_nospec(fd, ctx->nr_user_files); file = io_file_from_index(ctx, fd); -- cgit v1.2.3-58-ga151 From 8371adf53c3c5ebfde1172608d3f4e126729cb10 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sat, 10 Oct 2020 18:34:08 +0100 Subject: io_uring: simplify io_file_get() Keep ->needs_file_no_error check out of io_file_get(), and let callers handle it. It makes it more straightforward. Also, as the only error it can hand back -EBADF, make it return a file or NULL. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 39c37cef9ce0..ffdaea55e820 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -968,8 +968,8 @@ static int __io_sqe_files_update(struct io_ring_ctx *ctx, struct io_uring_files_update *ip, unsigned nr_args); static void __io_clean_op(struct io_kiocb *req); -static int io_file_get(struct io_submit_state *state, struct io_kiocb *req, - int fd, struct file **out_file, bool fixed); +static struct file *io_file_get(struct io_submit_state *state, + struct io_kiocb *req, int fd, bool fixed); static void __io_queue_sqe(struct io_kiocb *req, struct io_comp_state *cs); static void io_file_put_work(struct work_struct *work); @@ -3486,7 +3486,6 @@ static int __io_splice_prep(struct io_kiocb *req, { struct io_splice* sp = &req->splice; unsigned int valid_flags = SPLICE_F_FD_IN_FIXED | SPLICE_F_ALL; - int ret; if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL)) return -EINVAL; @@ -3498,10 +3497,10 @@ static int __io_splice_prep(struct io_kiocb *req, if (unlikely(sp->flags & ~valid_flags)) return -EINVAL; - ret = io_file_get(NULL, req, READ_ONCE(sqe->splice_fd_in), &sp->file_in, - (sp->flags & SPLICE_F_FD_IN_FIXED)); - if (ret) - return ret; + sp->file_in = io_file_get(NULL, req, READ_ONCE(sqe->splice_fd_in), + (sp->flags & SPLICE_F_FD_IN_FIXED)); + if (!sp->file_in) + return -EBADF; req->flags |= REQ_F_NEED_CLEANUP; if (!S_ISREG(file_inode(sp->file_in)->i_mode)) { @@ -5980,15 +5979,15 @@ static inline struct file *io_file_from_index(struct io_ring_ctx *ctx, return table->files[index & IORING_FILE_TABLE_MASK]; } -static int io_file_get(struct io_submit_state *state, struct io_kiocb *req, - int fd, struct file **out_file, bool fixed) +static struct file *io_file_get(struct io_submit_state *state, + struct io_kiocb *req, int fd, bool fixed) { struct io_ring_ctx *ctx = req->ctx; struct file *file; if (fixed) { if (unlikely((unsigned int)fd >= ctx->nr_user_files)) - return -EBADF; + return NULL; fd = array_index_nospec(fd, ctx->nr_user_files); file = io_file_from_index(ctx, fd); if (file) { @@ -6000,11 +5999,7 @@ static int io_file_get(struct io_submit_state *state, struct io_kiocb *req, file = __io_file_get(state, fd); } - if (file || io_op_defs[req->opcode].needs_file_no_error) { - *out_file = file; - return 0; - } - return -EBADF; + return file; } static int io_req_set_file(struct io_submit_state *state, struct io_kiocb *req, @@ -6016,7 +6011,10 @@ static int io_req_set_file(struct io_submit_state *state, struct io_kiocb *req, if (unlikely(!fixed && io_async_submit(req->ctx))) return -EBADF; - return io_file_get(state, req, fd, &req->file, fixed); + req->file = io_file_get(state, req, fd, fixed); + if (req->file || io_op_defs[req->opcode].needs_file_no_error) + return 0; + return -EBADF; } static enum hrtimer_restart io_link_timeout_fn(struct hrtimer *timer) -- cgit v1.2.3-58-ga151 From 71b547c048eb10d54627932019411549b1e4cfb7 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sat, 10 Oct 2020 18:34:09 +0100 Subject: io_uring: improve submit_state.ios_left accounting state->ios_left isn't decremented for requests that don't need a file, so it might be larger than number of SQEs left. That in some circumstances makes us to grab more files that is needed so imposing extra put. Deaccount one ios_left for each request. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index ffdaea55e820..250eefbe13cb 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -2581,7 +2581,6 @@ static struct file *__io_file_get(struct io_submit_state *state, int fd) if (state->file) { if (state->fd == fd) { state->has_refs--; - state->ios_left--; return state->file; } __io_state_file_put(state); @@ -2591,8 +2590,7 @@ static struct file *__io_file_get(struct io_submit_state *state, int fd) return NULL; state->fd = fd; - state->ios_left--; - state->has_refs = state->ios_left; + state->has_refs = state->ios_left - 1; return state->file; } @@ -6386,7 +6384,7 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req, struct io_submit_state *state) { unsigned int sqe_flags; - int id; + int id, ret; req->opcode = READ_ONCE(sqe->opcode); req->user_data = READ_ONCE(sqe->user_data); @@ -6432,7 +6430,9 @@ static int io_init_req(struct io_ring_ctx *ctx, struct io_kiocb *req, if (!io_op_defs[req->opcode].needs_file) return 0; - return io_req_set_file(state, req, READ_ONCE(sqe->fd)); + ret = io_req_set_file(state, req, READ_ONCE(sqe->fd)); + state->ios_left--; + return ret; } static int io_submit_sqes(struct io_ring_ctx *ctx, unsigned int nr) -- cgit v1.2.3-58-ga151 From 0bdf7a2ddb7d8b28d1c9f505e7f32aa2972d461b Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sat, 10 Oct 2020 18:34:10 +0100 Subject: io_uring: use a separate struct for timeout_remove Don't use struct io_timeout for both IORING_OP_TIMEOUT and IORING_OP_TIMEOUT_REMOVE, they're quite different. Split them in two, that allows to remove an unused field in struct io_timeout, and btw kill ->flags not used by either. This also easier to follow, especially for timeout remove. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 250eefbe13cb..09b8f2c9ae7e 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -434,13 +434,16 @@ struct io_cancel { struct io_timeout { struct file *file; - u64 addr; - int flags; u32 off; u32 target_seq; struct list_head list; }; +struct io_timeout_rem { + struct file *file; + u64 addr; +}; + struct io_rw { /* NOTE: kiocb has the file as the first member, so don't do it here */ struct kiocb kiocb; @@ -644,6 +647,7 @@ struct io_kiocb { struct io_sync sync; struct io_cancel cancel; struct io_timeout timeout; + struct io_timeout_rem timeout_rem; struct io_connect connect; struct io_sr_msg sr_msg; struct io_open open; @@ -5360,14 +5364,10 @@ static int io_timeout_remove_prep(struct io_kiocb *req, return -EINVAL; if (unlikely(req->flags & (REQ_F_FIXED_FILE | REQ_F_BUFFER_SELECT))) return -EINVAL; - if (sqe->ioprio || sqe->buf_index || sqe->len) - return -EINVAL; - - req->timeout.addr = READ_ONCE(sqe->addr); - req->timeout.flags = READ_ONCE(sqe->timeout_flags); - if (req->timeout.flags) + if (sqe->ioprio || sqe->buf_index || sqe->len || sqe->timeout_flags) return -EINVAL; + req->timeout_rem.addr = READ_ONCE(sqe->addr); return 0; } @@ -5380,7 +5380,7 @@ static int io_timeout_remove(struct io_kiocb *req) int ret; spin_lock_irq(&ctx->completion_lock); - ret = io_timeout_cancel(ctx, req->timeout.addr); + ret = io_timeout_cancel(ctx, req->timeout_rem.addr); io_cqring_fill_event(req, ret); io_commit_cqring(ctx); -- cgit v1.2.3-58-ga151 From a71976f3fa474d0aa9b33fc2ecaa67af6103bb71 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sat, 10 Oct 2020 18:34:11 +0100 Subject: io_uring: remove timeout.list after hrtimer cancel Remove timeouts from ctx->timeout_list after hrtimer_try_to_cancel() successfully cancels it. With this we don't need to care whether there was a race and it was removed in io_timeout_fn(), and that will be handy for following patches. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 09b8f2c9ae7e..3ce72d48eb21 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -5301,16 +5301,10 @@ static enum hrtimer_restart io_timeout_fn(struct hrtimer *timer) unsigned long flags; spin_lock_irqsave(&ctx->completion_lock, flags); + list_del_init(&req->timeout.list); atomic_set(&req->ctx->cq_timeouts, atomic_read(&req->ctx->cq_timeouts) + 1); - /* - * We could be racing with timeout deletion. If the list is empty, - * then timeout lookup already found it and will be handling it. - */ - if (!list_empty(&req->timeout.list)) - list_del_init(&req->timeout.list); - io_cqring_fill_event(req, -ETIME); io_commit_cqring(ctx); spin_unlock_irqrestore(&ctx->completion_lock, flags); @@ -5326,11 +5320,10 @@ static int __io_timeout_cancel(struct io_kiocb *req) struct io_timeout_data *io = req->async_data; int ret; - list_del_init(&req->timeout.list); - ret = hrtimer_try_to_cancel(&io->timer); if (ret == -1) return -EALREADY; + list_del_init(&req->timeout.list); req_set_fail_links(req); req->flags |= REQ_F_COMP_LOCKED; -- cgit v1.2.3-58-ga151 From 062d04d73168d1d7109b75600a53a6a361d1fda8 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sat, 10 Oct 2020 18:34:12 +0100 Subject: io_uring: clean leftovers after splitting issue Kill extra if in io_issue_sqe() and place send/recv[msg] calls appropriately under switch's cases. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 3ce72d48eb21..2e0105c373ae 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -5831,18 +5831,16 @@ static int io_issue_sqe(struct io_kiocb *req, bool force_nonblock, ret = io_sync_file_range(req, force_nonblock); break; case IORING_OP_SENDMSG: + ret = io_sendmsg(req, force_nonblock, cs); + break; case IORING_OP_SEND: - if (req->opcode == IORING_OP_SENDMSG) - ret = io_sendmsg(req, force_nonblock, cs); - else - ret = io_send(req, force_nonblock, cs); + ret = io_send(req, force_nonblock, cs); break; case IORING_OP_RECVMSG: + ret = io_recvmsg(req, force_nonblock, cs); + break; case IORING_OP_RECV: - if (req->opcode == IORING_OP_RECVMSG) - ret = io_recvmsg(req, force_nonblock, cs); - else - ret = io_recv(req, force_nonblock, cs); + ret = io_recv(req, force_nonblock, cs); break; case IORING_OP_TIMEOUT: ret = io_timeout(req); -- cgit v1.2.3-58-ga151 From 692d836351ffa05016521a769543af51c9dc3e9e Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sat, 10 Oct 2020 18:34:13 +0100 Subject: io_uring: don't delay io_init_req() error check Don't postpone io_init_req() error checks and do that right after calling it. There is no control-flow statements or dependencies with sqe/submitted accounting, so do those earlier, that makes the code flow a bit more natural. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 2e0105c373ae..22d1fb9cc80f 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -6466,12 +6466,11 @@ static int io_submit_sqes(struct io_ring_ctx *ctx, unsigned int nr) submitted = -EAGAIN; break; } - - err = io_init_req(ctx, req, sqe, &state); io_consume_sqe(ctx); /* will complete beyond this point, count as submitted */ submitted++; + err = io_init_req(ctx, req, sqe, &state); if (unlikely(err)) { fail_req: io_put_req(req); -- cgit v1.2.3-58-ga151 From 5398ae698525572d4eb0531854158ece94385318 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sat, 10 Oct 2020 18:34:14 +0100 Subject: io_uring: clean file_data access in files_register Keep file_data in a local var and replace with it complex references such as ctx->file_data. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 69 ++++++++++++++++++++++++++++------------------------------- 1 file changed, 33 insertions(+), 36 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index 22d1fb9cc80f..c3ca82f20f3d 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -7099,13 +7099,13 @@ static int io_sqe_files_scm(struct io_ring_ctx *ctx) } #endif -static int io_sqe_alloc_file_tables(struct io_ring_ctx *ctx, unsigned nr_tables, - unsigned nr_files) +static int io_sqe_alloc_file_tables(struct fixed_file_data *file_data, + unsigned nr_tables, unsigned nr_files) { int i; for (i = 0; i < nr_tables; i++) { - struct fixed_file_table *table = &ctx->file_data->table[i]; + struct fixed_file_table *table = &file_data->table[i]; unsigned this_files; this_files = min(nr_files, IORING_MAX_FILES_TABLE); @@ -7120,7 +7120,7 @@ static int io_sqe_alloc_file_tables(struct io_ring_ctx *ctx, unsigned nr_tables, return 0; for (i = 0; i < nr_tables; i++) { - struct fixed_file_table *table = &ctx->file_data->table[i]; + struct fixed_file_table *table = &file_data->table[i]; kfree(table->files); } return 1; @@ -7287,6 +7287,7 @@ static int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg, int fd, ret = 0; unsigned i; struct fixed_file_ref_node *ref_node; + struct fixed_file_data *file_data; if (ctx->file_data) return -EBUSY; @@ -7295,37 +7296,33 @@ static int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg, if (nr_args > IORING_MAX_FIXED_FILES) return -EMFILE; - ctx->file_data = kzalloc(sizeof(*ctx->file_data), GFP_KERNEL); - if (!ctx->file_data) + file_data = kzalloc(sizeof(*ctx->file_data), GFP_KERNEL); + if (!file_data) return -ENOMEM; - ctx->file_data->ctx = ctx; - init_completion(&ctx->file_data->done); - INIT_LIST_HEAD(&ctx->file_data->ref_list); - spin_lock_init(&ctx->file_data->lock); + file_data->ctx = ctx; + init_completion(&file_data->done); + INIT_LIST_HEAD(&file_data->ref_list); + spin_lock_init(&file_data->lock); nr_tables = DIV_ROUND_UP(nr_args, IORING_MAX_FILES_TABLE); - ctx->file_data->table = kcalloc(nr_tables, - sizeof(struct fixed_file_table), - GFP_KERNEL); - if (!ctx->file_data->table) { - kfree(ctx->file_data); - ctx->file_data = NULL; + file_data->table = kcalloc(nr_tables, sizeof(file_data->table), + GFP_KERNEL); + if (!file_data->table) { + kfree(file_data); return -ENOMEM; } - if (percpu_ref_init(&ctx->file_data->refs, io_file_ref_kill, + if (percpu_ref_init(&file_data->refs, io_file_ref_kill, PERCPU_REF_ALLOW_REINIT, GFP_KERNEL)) { - kfree(ctx->file_data->table); - kfree(ctx->file_data); - ctx->file_data = NULL; + kfree(file_data->table); + kfree(file_data); return -ENOMEM; } - if (io_sqe_alloc_file_tables(ctx, nr_tables, nr_args)) { - percpu_ref_exit(&ctx->file_data->refs); - kfree(ctx->file_data->table); - kfree(ctx->file_data); - ctx->file_data = NULL; + if (io_sqe_alloc_file_tables(file_data, nr_tables, nr_args)) { + percpu_ref_exit(&file_data->refs); + kfree(file_data->table); + kfree(file_data); return -ENOMEM; } @@ -7342,7 +7339,7 @@ static int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg, continue; } - table = &ctx->file_data->table[i >> IORING_FILE_TABLE_SHIFT]; + table = &file_data->table[i >> IORING_FILE_TABLE_SHIFT]; index = i & IORING_FILE_TABLE_MASK; file = fget(fd); @@ -7372,16 +7369,16 @@ static int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg, fput(file); } for (i = 0; i < nr_tables; i++) - kfree(ctx->file_data->table[i].files); + kfree(file_data->table[i].files); - percpu_ref_exit(&ctx->file_data->refs); - kfree(ctx->file_data->table); - kfree(ctx->file_data); - ctx->file_data = NULL; + percpu_ref_exit(&file_data->refs); + kfree(file_data->table); + kfree(file_data); ctx->nr_user_files = 0; return ret; } + ctx->file_data = file_data; ret = io_sqe_files_scm(ctx); if (ret) { io_sqe_files_unregister(ctx); @@ -7394,11 +7391,11 @@ static int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg, return PTR_ERR(ref_node); } - ctx->file_data->cur_refs = &ref_node->refs; - spin_lock(&ctx->file_data->lock); - list_add(&ref_node->node, &ctx->file_data->ref_list); - spin_unlock(&ctx->file_data->lock); - percpu_ref_get(&ctx->file_data->refs); + file_data->cur_refs = &ref_node->refs; + spin_lock(&file_data->lock); + list_add(&ref_node->node, &file_data->ref_list); + spin_unlock(&file_data->lock); + percpu_ref_get(&file_data->refs); return ret; } -- cgit v1.2.3-58-ga151 From 600cf3f8b3f68ad22ee9af4cf24b0a96512f7fbe Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sat, 10 Oct 2020 18:34:15 +0100 Subject: io_uring: refactor *files_register()'s error paths Don't keep repeating cleaning sequences in error paths, write it once in the and use labels. It's less error prone and looks cleaner. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 78 ++++++++++++++++++++++++----------------------------------- 1 file changed, 32 insertions(+), 46 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index c3ca82f20f3d..fc4ef725ae09 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -7282,10 +7282,9 @@ static int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg, unsigned nr_args) { __s32 __user *fds = (__s32 __user *) arg; - unsigned nr_tables; + unsigned nr_tables, i; struct file *file; - int fd, ret = 0; - unsigned i; + int fd, ret = -ENOMEM; struct fixed_file_ref_node *ref_node; struct fixed_file_data *file_data; @@ -7307,45 +7306,32 @@ static int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg, nr_tables = DIV_ROUND_UP(nr_args, IORING_MAX_FILES_TABLE); file_data->table = kcalloc(nr_tables, sizeof(file_data->table), GFP_KERNEL); - if (!file_data->table) { - kfree(file_data); - return -ENOMEM; - } + if (!file_data->table) + goto out_free; if (percpu_ref_init(&file_data->refs, io_file_ref_kill, - PERCPU_REF_ALLOW_REINIT, GFP_KERNEL)) { - kfree(file_data->table); - kfree(file_data); - return -ENOMEM; - } + PERCPU_REF_ALLOW_REINIT, GFP_KERNEL)) + goto out_free; - if (io_sqe_alloc_file_tables(file_data, nr_tables, nr_args)) { - percpu_ref_exit(&file_data->refs); - kfree(file_data->table); - kfree(file_data); - return -ENOMEM; - } + if (io_sqe_alloc_file_tables(file_data, nr_tables, nr_args)) + goto out_ref; for (i = 0; i < nr_args; i++, ctx->nr_user_files++) { struct fixed_file_table *table; unsigned index; - ret = -EFAULT; - if (copy_from_user(&fd, &fds[i], sizeof(fd))) - break; + if (copy_from_user(&fd, &fds[i], sizeof(fd))) { + ret = -EFAULT; + goto out_fput; + } /* allow sparse sets */ - if (fd == -1) { - ret = 0; + if (fd == -1) continue; - } - table = &file_data->table[i >> IORING_FILE_TABLE_SHIFT]; - index = i & IORING_FILE_TABLE_MASK; file = fget(fd); - ret = -EBADF; if (!file) - break; + goto out_fput; /* * Don't allow io_uring instances to be registered. If UNIX @@ -7356,28 +7342,13 @@ static int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg, */ if (file->f_op == &io_uring_fops) { fput(file); - break; + goto out_fput; } - ret = 0; + table = &file_data->table[i >> IORING_FILE_TABLE_SHIFT]; + index = i & IORING_FILE_TABLE_MASK; table->files[index] = file; } - if (ret) { - for (i = 0; i < ctx->nr_user_files; i++) { - file = io_file_from_index(ctx, i); - if (file) - fput(file); - } - for (i = 0; i < nr_tables; i++) - kfree(file_data->table[i].files); - - percpu_ref_exit(&file_data->refs); - kfree(file_data->table); - kfree(file_data); - ctx->nr_user_files = 0; - return ret; - } - ctx->file_data = file_data; ret = io_sqe_files_scm(ctx); if (ret) { @@ -7397,6 +7368,21 @@ static int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg, spin_unlock(&file_data->lock); percpu_ref_get(&file_data->refs); return ret; +out_fput: + for (i = 0; i < ctx->nr_user_files; i++) { + file = io_file_from_index(ctx, i); + if (file) + fput(file); + } + for (i = 0; i < nr_tables; i++) + kfree(file_data->table[i].files); + ctx->nr_user_files = 0; +out_ref: + percpu_ref_exit(&file_data->refs); +out_free: + kfree(file_data->table); + kfree(file_data); + return ret; } static int io_sqe_file_register(struct io_ring_ctx *ctx, struct file *file, -- cgit v1.2.3-58-ga151 From b2e9685283127f30e7f2b466af0046ff9bd27a86 Mon Sep 17 00:00:00 2001 From: Pavel Begunkov Date: Sat, 10 Oct 2020 18:34:16 +0100 Subject: io_uring: keep a pointer ref_node in file_data ->cur_refs of struct fixed_file_data always points to percpu_ref embedded into struct fixed_file_ref_node. Don't overuse container_of() and offsetting, and point directly to fixed_file_ref_node. Signed-off-by: Pavel Begunkov Signed-off-by: Jens Axboe --- fs/io_uring.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/fs/io_uring.c b/fs/io_uring.c index fc4ef725ae09..c729ee8033f8 100644 --- a/fs/io_uring.c +++ b/fs/io_uring.c @@ -210,7 +210,7 @@ struct fixed_file_data { struct fixed_file_table *table; struct io_ring_ctx *ctx; - struct percpu_ref *cur_refs; + struct fixed_file_ref_node *node; struct percpu_ref refs; struct completion done; struct list_head ref_list; @@ -5980,7 +5980,7 @@ static struct file *io_file_get(struct io_submit_state *state, fd = array_index_nospec(fd, ctx->nr_user_files); file = io_file_from_index(ctx, fd); if (file) { - req->fixed_file_refs = ctx->file_data->cur_refs; + req->fixed_file_refs = &ctx->file_data->node->refs; percpu_ref_get(req->fixed_file_refs); } } else { @@ -7362,7 +7362,7 @@ static int io_sqe_files_register(struct io_ring_ctx *ctx, void __user *arg, return PTR_ERR(ref_node); } - file_data->cur_refs = &ref_node->refs; + file_data->node = ref_node; spin_lock(&file_data->lock); list_add(&ref_node->node, &file_data->ref_list); spin_unlock(&file_data->lock); @@ -7432,14 +7432,12 @@ static int io_queue_file_removal(struct fixed_file_data *data, struct file *file) { struct io_file_put *pfile; - struct percpu_ref *refs = data->cur_refs; - struct fixed_file_ref_node *ref_node; + struct fixed_file_ref_node *ref_node = data->node; pfile = kzalloc(sizeof(*pfile), GFP_KERNEL); if (!pfile) return -ENOMEM; - ref_node = container_of(refs, struct fixed_file_ref_node, refs); pfile->file = file; list_add(&pfile->list, &ref_node->file_list); @@ -7522,10 +7520,10 @@ static int __io_sqe_files_update(struct io_ring_ctx *ctx, } if (needs_switch) { - percpu_ref_kill(data->cur_refs); + percpu_ref_kill(&data->node->refs); spin_lock(&data->lock); list_add(&ref_node->node, &data->ref_list); - data->cur_refs = &ref_node->refs; + data->node = ref_node; spin_unlock(&data->lock); percpu_ref_get(&ctx->file_data->refs); } else -- cgit v1.2.3-58-ga151