* [PATCH 1/8] io_uring: optimise req->ctx reloads
2021-10-15 16:09 [PATCH for-next 0/8] further rw cleanups+optimisisation Pavel Begunkov
@ 2021-10-15 16:09 ` Pavel Begunkov
2021-10-15 16:09 ` [PATCH 2/8] io_uring: kill io_wq_current_is_worker() in iopoll Pavel Begunkov
` (7 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Pavel Begunkov @ 2021-10-15 16:09 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
Don't load req->ctx in advance, it takes an extra register and the field
stays valid even after opcode handlers. It also optimises out req->ctx
load in io_iopoll_req_issued() once it's inlined.
Signed-off-by: Pavel Begunkov <[email protected]>
---
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 d9796002ff9d..c1a00535e130 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -6591,7 +6591,6 @@ static void io_clean_op(struct io_kiocb *req)
static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
{
- struct io_ring_ctx *ctx = req->ctx;
const struct cred *creds = NULL;
int ret;
@@ -6718,7 +6717,7 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
if (ret)
return ret;
/* If the op doesn't have a file, we're not polling for it */
- if ((ctx->flags & IORING_SETUP_IOPOLL) && req->file)
+ if ((req->ctx->flags & IORING_SETUP_IOPOLL) && req->file)
io_iopoll_req_issued(req);
return 0;
--
2.33.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 2/8] io_uring: kill io_wq_current_is_worker() in iopoll
2021-10-15 16:09 [PATCH for-next 0/8] further rw cleanups+optimisisation Pavel Begunkov
2021-10-15 16:09 ` [PATCH 1/8] io_uring: optimise req->ctx reloads Pavel Begunkov
@ 2021-10-15 16:09 ` Pavel Begunkov
2021-10-15 16:09 ` [PATCH 3/8] io_uring: optimise io_import_iovec fixed path Pavel Begunkov
` (6 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Pavel Begunkov @ 2021-10-15 16:09 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
Don't decide about locking based on io_wq_current_is_worker(), it's not
consistent with all other code and is expensive, use issue_flags.
Signed-off-by: Pavel Begunkov <[email protected]>
---
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 c1a00535e130..9fdbdf1cdb78 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2708,13 +2708,13 @@ static void io_complete_rw_iopoll(struct kiocb *kiocb, long res, long res2)
* find it from a io_do_iopoll() thread before the issuer is done
* accessing the kiocb cookie.
*/
-static void io_iopoll_req_issued(struct io_kiocb *req)
+static void io_iopoll_req_issued(struct io_kiocb *req, unsigned int issue_flags)
{
struct io_ring_ctx *ctx = req->ctx;
- const bool in_async = io_wq_current_is_worker();
+ const bool need_lock = !(issue_flags & IO_URING_F_NONBLOCK);
/* workqueue context doesn't hold uring_lock, grab it now */
- if (unlikely(in_async))
+ if (unlikely(need_lock))
mutex_lock(&ctx->uring_lock);
/*
@@ -2750,7 +2750,7 @@ static void io_iopoll_req_issued(struct io_kiocb *req)
else
wq_list_add_tail(&req->comp_list, &ctx->iopoll_list);
- if (unlikely(in_async)) {
+ if (unlikely(need_lock)) {
/*
* If IORING_SETUP_SQPOLL is enabled, sqes are either handle
* in sq thread task context or in io worker task context. If
@@ -6718,7 +6718,7 @@ static int io_issue_sqe(struct io_kiocb *req, unsigned int issue_flags)
return ret;
/* If the op doesn't have a file, we're not polling for it */
if ((req->ctx->flags & IORING_SETUP_IOPOLL) && req->file)
- io_iopoll_req_issued(req);
+ io_iopoll_req_issued(req, issue_flags);
return 0;
}
--
2.33.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 3/8] io_uring: optimise io_import_iovec fixed path
2021-10-15 16:09 [PATCH for-next 0/8] further rw cleanups+optimisisation Pavel Begunkov
2021-10-15 16:09 ` [PATCH 1/8] io_uring: optimise req->ctx reloads Pavel Begunkov
2021-10-15 16:09 ` [PATCH 2/8] io_uring: kill io_wq_current_is_worker() in iopoll Pavel Begunkov
@ 2021-10-15 16:09 ` Pavel Begunkov
2021-10-15 16:09 ` [PATCH 4/8] io_uring: return iovec from __io_import_iovec Pavel Begunkov
` (5 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Pavel Begunkov @ 2021-10-15 16:09 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
Delay loading req->rw.{addr,len} in io_import_iovec until it's really
needed, so removing extra loads for the fixed path, which doesn't use
them.
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 9fdbdf1cdb78..f354f4ae4f8c 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -3162,9 +3162,9 @@ static int __io_import_iovec(int rw, struct io_kiocb *req, struct iovec **iovec,
struct io_rw_state *s, unsigned int issue_flags)
{
struct iov_iter *iter = &s->iter;
- void __user *buf = u64_to_user_ptr(req->rw.addr);
- size_t sqe_len = req->rw.len;
u8 opcode = req->opcode;
+ void __user *buf;
+ size_t sqe_len;
ssize_t ret;
if (opcode == IORING_OP_READ_FIXED || opcode == IORING_OP_WRITE_FIXED) {
@@ -3173,9 +3173,12 @@ static int __io_import_iovec(int rw, struct io_kiocb *req, struct iovec **iovec,
}
/* buffer index only valid with fixed read/write, or buffer select */
- if (req->buf_index && !(req->flags & REQ_F_BUFFER_SELECT))
+ if (unlikely(req->buf_index && !(req->flags & REQ_F_BUFFER_SELECT)))
return -EINVAL;
+ buf = u64_to_user_ptr(req->rw.addr);
+ sqe_len = req->rw.len;
+
if (opcode == IORING_OP_READ || opcode == IORING_OP_WRITE) {
if (req->flags & REQ_F_BUFFER_SELECT) {
buf = io_rw_buffer_select(req, &sqe_len, issue_flags);
--
2.33.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 4/8] io_uring: return iovec from __io_import_iovec
2021-10-15 16:09 [PATCH for-next 0/8] further rw cleanups+optimisisation Pavel Begunkov
` (2 preceding siblings ...)
2021-10-15 16:09 ` [PATCH 3/8] io_uring: optimise io_import_iovec fixed path Pavel Begunkov
@ 2021-10-15 16:09 ` Pavel Begunkov
2021-10-15 16:09 ` [PATCH 5/8] io_uring: optimise fixed rw rsrc node setting Pavel Begunkov
` (4 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Pavel Begunkov @ 2021-10-15 16:09 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
We pass iovec** into __io_import_iovec(), which should keep it,
initialise and modify accordingly. It's expensive, return it directly
from __io_import_iovec encoding errors with ERR_PTR if needed.
io_import_iovec keeps the old interface, but it's inline and so
everything is optimised nicely.
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 45 +++++++++++++++++++++++----------------------
1 file changed, 23 insertions(+), 22 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index f354f4ae4f8c..a2514d2937c0 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -3158,23 +3158,25 @@ static ssize_t io_iov_buffer_select(struct io_kiocb *req, struct iovec *iov,
return __io_iov_buffer_select(req, iov, issue_flags);
}
-static int __io_import_iovec(int rw, struct io_kiocb *req, struct iovec **iovec,
- struct io_rw_state *s, unsigned int issue_flags)
+static struct iovec *__io_import_iovec(int rw, struct io_kiocb *req,
+ struct io_rw_state *s,
+ unsigned int issue_flags)
{
struct iov_iter *iter = &s->iter;
u8 opcode = req->opcode;
+ struct iovec *iovec;
void __user *buf;
size_t sqe_len;
ssize_t ret;
- if (opcode == IORING_OP_READ_FIXED || opcode == IORING_OP_WRITE_FIXED) {
- *iovec = NULL;
- return io_import_fixed(req, rw, iter);
- }
+ BUILD_BUG_ON(ERR_PTR(0) != NULL);
+
+ if (opcode == IORING_OP_READ_FIXED || opcode == IORING_OP_WRITE_FIXED)
+ return ERR_PTR(io_import_fixed(req, rw, iter));
/* buffer index only valid with fixed read/write, or buffer select */
if (unlikely(req->buf_index && !(req->flags & REQ_F_BUFFER_SELECT)))
- return -EINVAL;
+ return ERR_PTR(-EINVAL);
buf = u64_to_user_ptr(req->rw.addr);
sqe_len = req->rw.len;
@@ -3183,40 +3185,39 @@ static int __io_import_iovec(int rw, struct io_kiocb *req, struct iovec **iovec,
if (req->flags & REQ_F_BUFFER_SELECT) {
buf = io_rw_buffer_select(req, &sqe_len, issue_flags);
if (IS_ERR(buf))
- return PTR_ERR(buf);
+ return ERR_PTR(PTR_ERR(buf));
req->rw.len = sqe_len;
}
ret = import_single_range(rw, buf, sqe_len, s->fast_iov, iter);
- *iovec = NULL;
- return ret;
+ return ERR_PTR(ret);
}
- *iovec = s->fast_iov;
-
+ iovec = s->fast_iov;
if (req->flags & REQ_F_BUFFER_SELECT) {
- ret = io_iov_buffer_select(req, *iovec, issue_flags);
+ ret = io_iov_buffer_select(req, iovec, issue_flags);
if (!ret)
- iov_iter_init(iter, rw, *iovec, 1, (*iovec)->iov_len);
- *iovec = NULL;
- return ret;
+ iov_iter_init(iter, rw, iovec, 1, iovec->iov_len);
+ return ERR_PTR(ret);
}
- return __import_iovec(rw, buf, sqe_len, UIO_FASTIOV, iovec, iter,
+ ret = __import_iovec(rw, buf, sqe_len, UIO_FASTIOV, &iovec, iter,
req->ctx->compat);
+ if (unlikely(ret < 0))
+ return ERR_PTR(ret);
+ return iovec;
}
static inline int io_import_iovec(int rw, struct io_kiocb *req,
struct iovec **iovec, struct io_rw_state *s,
unsigned int issue_flags)
{
- int ret;
+ *iovec = __io_import_iovec(rw, req, s, issue_flags);
+ if (unlikely(IS_ERR(*iovec)))
+ return PTR_ERR(*iovec);
- ret = __io_import_iovec(rw, req, iovec, s, issue_flags);
- if (unlikely(ret < 0))
- return ret;
iov_iter_save_state(&s->iter, &s->iter_state);
- return ret;
+ return 0;
}
static inline loff_t *io_kiocb_ppos(struct kiocb *kiocb)
--
2.33.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 5/8] io_uring: optimise fixed rw rsrc node setting
2021-10-15 16:09 [PATCH for-next 0/8] further rw cleanups+optimisisation Pavel Begunkov
` (3 preceding siblings ...)
2021-10-15 16:09 ` [PATCH 4/8] io_uring: return iovec from __io_import_iovec Pavel Begunkov
@ 2021-10-15 16:09 ` Pavel Begunkov
2021-10-15 16:09 ` [PATCH 6/8] io_uring: clean io_prep_rw() Pavel Begunkov
` (3 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Pavel Begunkov @ 2021-10-15 16:09 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
Move fixed rw io_req_set_rsrc_node() from rw prep into
io_import_fixed(), if we're using fixed buffers it will always be called
during submission as we save the state in advance,
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index a2514d2937c0..64ef04c9628d 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2874,12 +2874,7 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe,
kiocb->ki_complete = io_complete_rw;
}
- if (req->opcode == IORING_OP_READ_FIXED ||
- req->opcode == IORING_OP_WRITE_FIXED) {
- req->imu = NULL;
- io_req_set_rsrc_node(req, ctx);
- }
-
+ req->imu = NULL;
req->rw.addr = READ_ONCE(sqe->addr);
req->rw.len = READ_ONCE(sqe->len);
req->buf_index = READ_ONCE(sqe->buf_index);
@@ -3008,13 +3003,15 @@ static int __io_import_fixed(struct io_kiocb *req, int rw, struct iov_iter *iter
static int io_import_fixed(struct io_kiocb *req, int rw, struct iov_iter *iter)
{
- struct io_ring_ctx *ctx = req->ctx;
struct io_mapped_ubuf *imu = req->imu;
u16 index, buf_index = req->buf_index;
if (likely(!imu)) {
+ struct io_ring_ctx *ctx = req->ctx;
+
if (unlikely(buf_index >= ctx->nr_user_bufs))
return -EFAULT;
+ io_req_set_rsrc_node(req, ctx);
index = array_index_nospec(buf_index, ctx->nr_user_bufs);
imu = READ_ONCE(ctx->user_bufs[index]);
req->imu = imu;
--
2.33.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 6/8] io_uring: clean io_prep_rw()
2021-10-15 16:09 [PATCH for-next 0/8] further rw cleanups+optimisisation Pavel Begunkov
` (4 preceding siblings ...)
2021-10-15 16:09 ` [PATCH 5/8] io_uring: optimise fixed rw rsrc node setting Pavel Begunkov
@ 2021-10-15 16:09 ` Pavel Begunkov
2021-10-15 16:09 ` [PATCH 7/8] io_uring: arm poll for non-nowait files Pavel Begunkov
` (2 subsequent siblings)
8 siblings, 0 replies; 12+ messages in thread
From: Pavel Begunkov @ 2021-10-15 16:09 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
We already store req->file in a variable in io_prep_rw(), just use it
instead of a couple of left references to kicob->ki_filp.
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 64ef04c9628d..ce9a1b89da3f 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2835,8 +2835,8 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe,
req->flags |= REQ_F_CUR_POS;
kiocb->ki_pos = file->f_pos;
}
- kiocb->ki_hint = ki_hint_validate(file_write_hint(kiocb->ki_filp));
- kiocb->ki_flags = iocb_flags(kiocb->ki_filp);
+ kiocb->ki_hint = ki_hint_validate(file_write_hint(file));
+ kiocb->ki_flags = iocb_flags(file);
ret = kiocb_set_rw_flags(kiocb, READ_ONCE(sqe->rw_flags));
if (unlikely(ret))
return ret;
@@ -2861,8 +2861,7 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe,
kiocb->ki_ioprio = get_current_ioprio();
if (ctx->flags & IORING_SETUP_IOPOLL) {
- if (!(kiocb->ki_flags & IOCB_DIRECT) ||
- !kiocb->ki_filp->f_op->iopoll)
+ if (!(kiocb->ki_flags & IOCB_DIRECT) || !file->f_op->iopoll)
return -EOPNOTSUPP;
kiocb->ki_flags |= IOCB_HIPRI | IOCB_ALLOC_CACHE;
--
2.33.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH 7/8] io_uring: arm poll for non-nowait files
2021-10-15 16:09 [PATCH for-next 0/8] further rw cleanups+optimisisation Pavel Begunkov
` (5 preceding siblings ...)
2021-10-15 16:09 ` [PATCH 6/8] io_uring: clean io_prep_rw() Pavel Begunkov
@ 2021-10-15 16:09 ` Pavel Begunkov
2021-10-16 22:57 ` Noah Goldstein
2021-10-15 16:09 ` [PATCH 8/8] io_uring: simplify io_file_supports_nowait() Pavel Begunkov
2021-10-16 3:36 ` [PATCH for-next 0/8] further rw cleanups+optimisisation Jens Axboe
8 siblings, 1 reply; 12+ messages in thread
From: Pavel Begunkov @ 2021-10-15 16:09 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
Don't check if we can do nowait before arming apoll, there are several
reasons for that. First, we don't care much about files that don't
support nowait. Second, it may be useful -- we don't want to be taking
away extra workers from io-wq when it can go in some async. Even if it
will go through io-wq eventually, it make difference in the numbers of
workers actually used. And the last one, it's needed to clean nowait in
future commits.
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 65 +++++++++++++++++----------------------------------
1 file changed, 21 insertions(+), 44 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index ce9a1b89da3f..c9acb4d2a1ff 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -732,8 +732,7 @@ enum {
REQ_F_ARM_LTIMEOUT_BIT,
REQ_F_ASYNC_DATA_BIT,
/* keep async read/write and isreg together and in order */
- REQ_F_NOWAIT_READ_BIT,
- REQ_F_NOWAIT_WRITE_BIT,
+ REQ_F_SUPPORT_NOWAIT_BIT,
REQ_F_ISREG_BIT,
/* not a real bit, just to check we're not overflowing the space */
@@ -774,10 +773,8 @@ enum {
REQ_F_COMPLETE_INLINE = BIT(REQ_F_COMPLETE_INLINE_BIT),
/* caller should reissue async */
REQ_F_REISSUE = BIT(REQ_F_REISSUE_BIT),
- /* supports async reads */
- REQ_F_NOWAIT_READ = BIT(REQ_F_NOWAIT_READ_BIT),
- /* supports async writes */
- REQ_F_NOWAIT_WRITE = BIT(REQ_F_NOWAIT_WRITE_BIT),
+ /* supports async reads/writes */
+ REQ_F_SUPPORT_NOWAIT = BIT(REQ_F_SUPPORT_NOWAIT_BIT),
/* regular file */
REQ_F_ISREG = BIT(REQ_F_ISREG_BIT),
/* has creds assigned */
@@ -1390,18 +1387,13 @@ static bool req_need_defer(struct io_kiocb *req, u32 seq)
return false;
}
-#define FFS_ASYNC_READ 0x1UL
-#define FFS_ASYNC_WRITE 0x2UL
-#ifdef CONFIG_64BIT
-#define FFS_ISREG 0x4UL
-#else
-#define FFS_ISREG 0x0UL
-#endif
-#define FFS_MASK ~(FFS_ASYNC_READ|FFS_ASYNC_WRITE|FFS_ISREG)
+#define FFS_NOWAIT 0x1UL
+#define FFS_ISREG 0x2UL
+#define FFS_MASK ~(FFS_NOWAIT|FFS_ISREG)
static inline bool io_req_ffs_set(struct io_kiocb *req)
{
- return IS_ENABLED(CONFIG_64BIT) && (req->flags & REQ_F_FIXED_FILE);
+ return req->flags & REQ_F_FIXED_FILE;
}
static inline void io_req_track_inflight(struct io_kiocb *req)
@@ -2775,7 +2767,7 @@ static bool io_bdev_nowait(struct block_device *bdev)
* any file. For now, just ensure that anything potentially problematic is done
* inline.
*/
-static bool __io_file_supports_nowait(struct file *file, int rw)
+static bool __io_file_supports_nowait(struct file *file)
{
umode_t mode = file_inode(file)->i_mode;
@@ -2798,24 +2790,14 @@ static bool __io_file_supports_nowait(struct file *file, int rw)
/* any ->read/write should understand O_NONBLOCK */
if (file->f_flags & O_NONBLOCK)
return true;
-
- if (!(file->f_mode & FMODE_NOWAIT))
- return false;
-
- if (rw == READ)
- return file->f_op->read_iter != NULL;
-
- return file->f_op->write_iter != NULL;
+ return file->f_mode & FMODE_NOWAIT;
}
-static bool io_file_supports_nowait(struct io_kiocb *req, int rw)
+static inline bool io_file_supports_nowait(struct io_kiocb *req)
{
- if (rw == READ && (req->flags & REQ_F_NOWAIT_READ))
- return true;
- else if (rw == WRITE && (req->flags & REQ_F_NOWAIT_WRITE))
+ if (likely(req->flags & REQ_F_SUPPORT_NOWAIT))
return true;
-
- return __io_file_supports_nowait(req->file, rw);
+ return __io_file_supports_nowait(req->file);
}
static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe,
@@ -2847,7 +2829,7 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe,
* reliably. If not, or it IOCB_NOWAIT is set, don't retry.
*/
if ((kiocb->ki_flags & IOCB_NOWAIT) ||
- ((file->f_flags & O_NONBLOCK) && !io_file_supports_nowait(req, rw)))
+ ((file->f_flags & O_NONBLOCK) && !io_file_supports_nowait(req)))
req->flags |= REQ_F_NOWAIT;
ioprio = READ_ONCE(sqe->ioprio);
@@ -3238,7 +3220,8 @@ static ssize_t loop_rw_iter(int rw, struct io_kiocb *req, struct iov_iter *iter)
*/
if (kiocb->ki_flags & IOCB_HIPRI)
return -EOPNOTSUPP;
- if (kiocb->ki_flags & IOCB_NOWAIT)
+ if ((kiocb->ki_flags & IOCB_NOWAIT) &&
+ !(kiocb->ki_filp->f_flags & O_NONBLOCK))
return -EAGAIN;
while (iov_iter_count(iter)) {
@@ -3478,7 +3461,7 @@ static int io_read(struct io_kiocb *req, unsigned int issue_flags)
if (force_nonblock) {
/* If the file doesn't support async, just async punt */
- if (unlikely(!io_file_supports_nowait(req, READ))) {
+ if (unlikely(!io_file_supports_nowait(req))) {
ret = io_setup_async_rw(req, iovec, s, true);
return ret ?: -EAGAIN;
}
@@ -3602,7 +3585,7 @@ static int io_write(struct io_kiocb *req, unsigned int issue_flags)
if (force_nonblock) {
/* If the file doesn't support async, just async punt */
- if (unlikely(!io_file_supports_nowait(req, WRITE)))
+ if (unlikely(!io_file_supports_nowait(req)))
goto copy_iov;
/* file path doesn't support NOWAIT for non-direct_IO */
@@ -3634,7 +3617,7 @@ static int io_write(struct io_kiocb *req, unsigned int issue_flags)
}
kiocb->ki_flags |= IOCB_WRITE;
- if (req->file->f_op->write_iter)
+ if (likely(req->file->f_op->write_iter))
ret2 = call_write_iter(req->file, kiocb, &s->iter);
else if (req->file->f_op->write)
ret2 = loop_rw_iter(WRITE, req, &s->iter);
@@ -5613,10 +5596,6 @@ static int io_arm_poll_handler(struct io_kiocb *req)
mask |= POLLOUT | POLLWRNORM;
}
- /* if we can't nonblock try, then no point in arming a poll handler */
- if (!io_file_supports_nowait(req, rw))
- return IO_APOLL_ABORTED;
-
apoll = kmalloc(sizeof(*apoll), GFP_ATOMIC);
if (unlikely(!apoll))
return IO_APOLL_ABORTED;
@@ -6788,10 +6767,8 @@ static void io_fixed_file_set(struct io_fixed_file *file_slot, struct file *file
{
unsigned long file_ptr = (unsigned long) file;
- if (__io_file_supports_nowait(file, READ))
- file_ptr |= FFS_ASYNC_READ;
- if (__io_file_supports_nowait(file, WRITE))
- file_ptr |= FFS_ASYNC_WRITE;
+ if (__io_file_supports_nowait(file))
+ file_ptr |= FFS_NOWAIT;
if (S_ISREG(file_inode(file)->i_mode))
file_ptr |= FFS_ISREG;
file_slot->file_ptr = file_ptr;
@@ -6810,7 +6787,7 @@ static inline struct file *io_file_get_fixed(struct io_ring_ctx *ctx,
file = (struct file *) (file_ptr & FFS_MASK);
file_ptr &= ~FFS_MASK;
/* mask in overlapping REQ_F and FFS bits */
- req->flags |= (file_ptr << REQ_F_NOWAIT_READ_BIT);
+ req->flags |= (file_ptr << REQ_F_SUPPORT_NOWAIT_BIT);
io_req_set_rsrc_node(req, ctx);
return file;
}
--
2.33.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH 7/8] io_uring: arm poll for non-nowait files
2021-10-15 16:09 ` [PATCH 7/8] io_uring: arm poll for non-nowait files Pavel Begunkov
@ 2021-10-16 22:57 ` Noah Goldstein
2021-10-16 23:16 ` Pavel Begunkov
0 siblings, 1 reply; 12+ messages in thread
From: Noah Goldstein @ 2021-10-16 22:57 UTC (permalink / raw)
To: Pavel Begunkov; +Cc: open list:IO_URING, Jens Axboe
On Sat, Oct 16, 2021 at 5:19 PM Pavel Begunkov <[email protected]> wrote:
>
> Don't check if we can do nowait before arming apoll, there are several
> reasons for that. First, we don't care much about files that don't
> support nowait. Second, it may be useful -- we don't want to be taking
> away extra workers from io-wq when it can go in some async. Even if it
> will go through io-wq eventually, it make difference in the numbers of
> workers actually used. And the last one, it's needed to clean nowait in
> future commits.
>
> Signed-off-by: Pavel Begunkov <[email protected]>
> ---
> fs/io_uring.c | 65 +++++++++++++++++----------------------------------
> 1 file changed, 21 insertions(+), 44 deletions(-)
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index ce9a1b89da3f..c9acb4d2a1ff 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -732,8 +732,7 @@ enum {
> REQ_F_ARM_LTIMEOUT_BIT,
> REQ_F_ASYNC_DATA_BIT,
> /* keep async read/write and isreg together and in order */
> - REQ_F_NOWAIT_READ_BIT,
> - REQ_F_NOWAIT_WRITE_BIT,
> + REQ_F_SUPPORT_NOWAIT_BIT,
> REQ_F_ISREG_BIT,
>
> /* not a real bit, just to check we're not overflowing the space */
> @@ -774,10 +773,8 @@ enum {
> REQ_F_COMPLETE_INLINE = BIT(REQ_F_COMPLETE_INLINE_BIT),
> /* caller should reissue async */
> REQ_F_REISSUE = BIT(REQ_F_REISSUE_BIT),
> - /* supports async reads */
> - REQ_F_NOWAIT_READ = BIT(REQ_F_NOWAIT_READ_BIT),
> - /* supports async writes */
> - REQ_F_NOWAIT_WRITE = BIT(REQ_F_NOWAIT_WRITE_BIT),
> + /* supports async reads/writes */
> + REQ_F_SUPPORT_NOWAIT = BIT(REQ_F_SUPPORT_NOWAIT_BIT),
> /* regular file */
> REQ_F_ISREG = BIT(REQ_F_ISREG_BIT),
> /* has creds assigned */
> @@ -1390,18 +1387,13 @@ static bool req_need_defer(struct io_kiocb *req, u32 seq)
> return false;
> }
>
> -#define FFS_ASYNC_READ 0x1UL
> -#define FFS_ASYNC_WRITE 0x2UL
> -#ifdef CONFIG_64BIT
> -#define FFS_ISREG 0x4UL
> -#else
> -#define FFS_ISREG 0x0UL
> -#endif
> -#define FFS_MASK ~(FFS_ASYNC_READ|FFS_ASYNC_WRITE|FFS_ISREG)
> +#define FFS_NOWAIT 0x1UL
> +#define FFS_ISREG 0x2UL
> +#define FFS_MASK ~(FFS_NOWAIT|FFS_ISREG)
>
> static inline bool io_req_ffs_set(struct io_kiocb *req)
> {
> - return IS_ENABLED(CONFIG_64BIT) && (req->flags & REQ_F_FIXED_FILE);
> + return req->flags & REQ_F_FIXED_FILE;
> }
>
> static inline void io_req_track_inflight(struct io_kiocb *req)
> @@ -2775,7 +2767,7 @@ static bool io_bdev_nowait(struct block_device *bdev)
> * any file. For now, just ensure that anything potentially problematic is done
> * inline.
> */
> -static bool __io_file_supports_nowait(struct file *file, int rw)
> +static bool __io_file_supports_nowait(struct file *file)
> {
> umode_t mode = file_inode(file)->i_mode;
>
> @@ -2798,24 +2790,14 @@ static bool __io_file_supports_nowait(struct file *file, int rw)
> /* any ->read/write should understand O_NONBLOCK */
> if (file->f_flags & O_NONBLOCK)
> return true;
> -
> - if (!(file->f_mode & FMODE_NOWAIT))
> - return false;
> -
> - if (rw == READ)
> - return file->f_op->read_iter != NULL;
> -
> - return file->f_op->write_iter != NULL;
> + return file->f_mode & FMODE_NOWAIT;
> }
>
> -static bool io_file_supports_nowait(struct io_kiocb *req, int rw)
> +static inline bool io_file_supports_nowait(struct io_kiocb *req)
> {
> - if (rw == READ && (req->flags & REQ_F_NOWAIT_READ))
> - return true;
> - else if (rw == WRITE && (req->flags & REQ_F_NOWAIT_WRITE))
> + if (likely(req->flags & REQ_F_SUPPORT_NOWAIT))
> return true;
> -
> - return __io_file_supports_nowait(req->file, rw);
> + return __io_file_supports_nowait(req->file);
> }
>
> static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe,
> @@ -2847,7 +2829,7 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe,
> * reliably. If not, or it IOCB_NOWAIT is set, don't retry.
> */
> if ((kiocb->ki_flags & IOCB_NOWAIT) ||
> - ((file->f_flags & O_NONBLOCK) && !io_file_supports_nowait(req, rw)))
> + ((file->f_flags & O_NONBLOCK) && !io_file_supports_nowait(req)))
> req->flags |= REQ_F_NOWAIT;
>
> ioprio = READ_ONCE(sqe->ioprio);
> @@ -3238,7 +3220,8 @@ static ssize_t loop_rw_iter(int rw, struct io_kiocb *req, struct iov_iter *iter)
> */
> if (kiocb->ki_flags & IOCB_HIPRI)
> return -EOPNOTSUPP;
> - if (kiocb->ki_flags & IOCB_NOWAIT)
> + if ((kiocb->ki_flags & IOCB_NOWAIT) &&
> + !(kiocb->ki_filp->f_flags & O_NONBLOCK))
> return -EAGAIN;
Instead of 2x branches on what appears to be the error (not hot path)
what about:
if (kiocb->ki_flags & (IOCB_HIPRI | IOCB_NOWAIT)) {
if (kiocb->ki_flags & IOCB_HIPRI)
return -EOPNOTSUPP;
if (!(kiocb->ki_filp->f_flags & O_NONBLOCK)) {
return -EAGAIN;
}
}
>
> while (iov_iter_count(iter)) {
> @@ -3478,7 +3461,7 @@ static int io_read(struct io_kiocb *req, unsigned int issue_flags)
>
> if (force_nonblock) {
> /* If the file doesn't support async, just async punt */
> - if (unlikely(!io_file_supports_nowait(req, READ))) {
> + if (unlikely(!io_file_supports_nowait(req))) {
> ret = io_setup_async_rw(req, iovec, s, true);
> return ret ?: -EAGAIN;
> }
> @@ -3602,7 +3585,7 @@ static int io_write(struct io_kiocb *req, unsigned int issue_flags)
>
> if (force_nonblock) {
> /* If the file doesn't support async, just async punt */
> - if (unlikely(!io_file_supports_nowait(req, WRITE)))
> + if (unlikely(!io_file_supports_nowait(req)))
> goto copy_iov;
>
> /* file path doesn't support NOWAIT for non-direct_IO */
> @@ -3634,7 +3617,7 @@ static int io_write(struct io_kiocb *req, unsigned int issue_flags)
> }
> kiocb->ki_flags |= IOCB_WRITE;
>
> - if (req->file->f_op->write_iter)
> + if (likely(req->file->f_op->write_iter))
> ret2 = call_write_iter(req->file, kiocb, &s->iter);
> else if (req->file->f_op->write)
> ret2 = loop_rw_iter(WRITE, req, &s->iter);
> @@ -5613,10 +5596,6 @@ static int io_arm_poll_handler(struct io_kiocb *req)
> mask |= POLLOUT | POLLWRNORM;
> }
>
> - /* if we can't nonblock try, then no point in arming a poll handler */
> - if (!io_file_supports_nowait(req, rw))
> - return IO_APOLL_ABORTED;
> -
> apoll = kmalloc(sizeof(*apoll), GFP_ATOMIC);
> if (unlikely(!apoll))
> return IO_APOLL_ABORTED;
> @@ -6788,10 +6767,8 @@ static void io_fixed_file_set(struct io_fixed_file *file_slot, struct file *file
> {
> unsigned long file_ptr = (unsigned long) file;
>
> - if (__io_file_supports_nowait(file, READ))
> - file_ptr |= FFS_ASYNC_READ;
> - if (__io_file_supports_nowait(file, WRITE))
> - file_ptr |= FFS_ASYNC_WRITE;
> + if (__io_file_supports_nowait(file))
> + file_ptr |= FFS_NOWAIT;
> if (S_ISREG(file_inode(file)->i_mode))
> file_ptr |= FFS_ISREG;
> file_slot->file_ptr = file_ptr;
> @@ -6810,7 +6787,7 @@ static inline struct file *io_file_get_fixed(struct io_ring_ctx *ctx,
> file = (struct file *) (file_ptr & FFS_MASK);
> file_ptr &= ~FFS_MASK;
> /* mask in overlapping REQ_F and FFS bits */
> - req->flags |= (file_ptr << REQ_F_NOWAIT_READ_BIT);
> + req->flags |= (file_ptr << REQ_F_SUPPORT_NOWAIT_BIT);
> io_req_set_rsrc_node(req, ctx);
> return file;
> }
> --
> 2.33.0
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH 7/8] io_uring: arm poll for non-nowait files
2021-10-16 22:57 ` Noah Goldstein
@ 2021-10-16 23:16 ` Pavel Begunkov
0 siblings, 0 replies; 12+ messages in thread
From: Pavel Begunkov @ 2021-10-16 23:16 UTC (permalink / raw)
To: Noah Goldstein; +Cc: open list:IO_URING, Jens Axboe
On 10/16/21 23:57, Noah Goldstein wrote:
> On Sat, Oct 16, 2021 at 5:19 PM Pavel Begunkov <[email protected]> wrote:
>> @@ -3238,7 +3220,8 @@ static ssize_t loop_rw_iter(int rw, struct io_kiocb *req, struct iov_iter *iter)
>> */
>> if (kiocb->ki_flags & IOCB_HIPRI)
>> return -EOPNOTSUPP;
>> - if (kiocb->ki_flags & IOCB_NOWAIT)
>> + if ((kiocb->ki_flags & IOCB_NOWAIT) &&
>> + !(kiocb->ki_filp->f_flags & O_NONBLOCK))
>> return -EAGAIN;
>
> Instead of 2x branches on what appears to be the error (not hot path)
The whole loop_rw_iter() is a slow path, we don't care enough
--
Pavel Begunkov
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 8/8] io_uring: simplify io_file_supports_nowait()
2021-10-15 16:09 [PATCH for-next 0/8] further rw cleanups+optimisisation Pavel Begunkov
` (6 preceding siblings ...)
2021-10-15 16:09 ` [PATCH 7/8] io_uring: arm poll for non-nowait files Pavel Begunkov
@ 2021-10-15 16:09 ` Pavel Begunkov
2021-10-16 3:36 ` [PATCH for-next 0/8] further rw cleanups+optimisisation Jens Axboe
8 siblings, 0 replies; 12+ messages in thread
From: Pavel Begunkov @ 2021-10-15 16:09 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, asml.silence
Make sure that REQ_F_SUPPORT_NOWAIT is always set io_prep_rw(), and so
we can stop caring about setting it down the line simplifying
io_file_supports_nowait().
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 34 ++++++++++++++++++++++------------
1 file changed, 22 insertions(+), 12 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index c9acb4d2a1ff..b6f7fb5c910b 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2767,10 +2767,8 @@ static bool io_bdev_nowait(struct block_device *bdev)
* any file. For now, just ensure that anything potentially problematic is done
* inline.
*/
-static bool __io_file_supports_nowait(struct file *file)
+static bool __io_file_supports_nowait(struct file *file, umode_t mode)
{
- umode_t mode = file_inode(file)->i_mode;
-
if (S_ISBLK(mode)) {
if (IS_ENABLED(CONFIG_BLOCK) &&
io_bdev_nowait(I_BDEV(file->f_mapping->host)))
@@ -2793,11 +2791,26 @@ static bool __io_file_supports_nowait(struct file *file)
return file->f_mode & FMODE_NOWAIT;
}
+/*
+ * If we tracked the file through the SCM inflight mechanism, we could support
+ * any file. For now, just ensure that anything potentially problematic is done
+ * inline.
+ */
+static unsigned int io_file_get_flags(struct file *file)
+{
+ umode_t mode = file_inode(file)->i_mode;
+ unsigned int res = 0;
+
+ if (S_ISREG(mode))
+ res |= FFS_ISREG;
+ if (__io_file_supports_nowait(file, mode))
+ res |= FFS_NOWAIT;
+ return res;
+}
+
static inline bool io_file_supports_nowait(struct io_kiocb *req)
{
- if (likely(req->flags & REQ_F_SUPPORT_NOWAIT))
- return true;
- return __io_file_supports_nowait(req->file);
+ return req->flags & REQ_F_SUPPORT_NOWAIT;
}
static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe,
@@ -2809,8 +2822,8 @@ static int io_prep_rw(struct io_kiocb *req, const struct io_uring_sqe *sqe,
unsigned ioprio;
int ret;
- if (!io_req_ffs_set(req) && S_ISREG(file_inode(file)->i_mode))
- req->flags |= REQ_F_ISREG;
+ if (!io_req_ffs_set(req))
+ req->flags |= io_file_get_flags(file) << REQ_F_SUPPORT_NOWAIT_BIT;
kiocb->ki_pos = READ_ONCE(sqe->off);
if (kiocb->ki_pos == -1 && !(file->f_mode & FMODE_STREAM)) {
@@ -6767,10 +6780,7 @@ static void io_fixed_file_set(struct io_fixed_file *file_slot, struct file *file
{
unsigned long file_ptr = (unsigned long) file;
- if (__io_file_supports_nowait(file))
- file_ptr |= FFS_NOWAIT;
- if (S_ISREG(file_inode(file)->i_mode))
- file_ptr |= FFS_ISREG;
+ file_ptr |= io_file_get_flags(file);
file_slot->file_ptr = file_ptr;
}
--
2.33.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH for-next 0/8] further rw cleanups+optimisisation
2021-10-15 16:09 [PATCH for-next 0/8] further rw cleanups+optimisisation Pavel Begunkov
` (7 preceding siblings ...)
2021-10-15 16:09 ` [PATCH 8/8] io_uring: simplify io_file_supports_nowait() Pavel Begunkov
@ 2021-10-16 3:36 ` Jens Axboe
8 siblings, 0 replies; 12+ messages in thread
From: Jens Axboe @ 2021-10-16 3:36 UTC (permalink / raw)
To: io-uring, Pavel Begunkov; +Cc: Jens Axboe
On Fri, 15 Oct 2021 17:09:10 +0100, Pavel Begunkov wrote:
> Some not difficult code reshuffling.
>
> Default test with nullblk: around +1% throughput
>
> Pavel Begunkov (8):
> io_uring: optimise req->ctx reloads
> io_uring: kill io_wq_current_is_worker() in iopoll
> io_uring: optimise io_import_iovec fixed path
> io_uring: return iovec from __io_import_iovec
> io_uring: optimise fixed rw rsrc node setting
> io_uring: clean io_prep_rw()
> io_uring: arm poll for non-nowait files
> io_uring: simplify io_file_supports_nowait()
>
> [...]
Applied, thanks!
[1/8] io_uring: optimise req->ctx reloads
commit: 5d946c9385d88990143a2a150ff24fd9d80f9ed2
[2/8] io_uring: kill io_wq_current_is_worker() in iopoll
commit: 62768ee791cb7c55ffd74bb52ea384bc7457b247
[3/8] io_uring: optimise io_import_iovec fixed path
commit: 406e1233ec43ee8cdfc13a17a2bebd169e75d7a6
[4/8] io_uring: return iovec from __io_import_iovec
commit: 200a80aa207869f9e2a0e5b4487d39664f55a85d
[5/8] io_uring: optimise fixed rw rsrc node setting
commit: 95462452d4c8469490e9396bcf31b582716063a5
[6/8] io_uring: clean io_prep_rw()
commit: 8b0286cb37b407b04ec2a0c9f2f7908fa606af76
[7/8] io_uring: arm poll for non-nowait files
commit: 7070f9ad7468e52c5bd36c6270aa4c6466f6bbf3
[8/8] io_uring: simplify io_file_supports_nowait()
commit: 785d7baa96560c726b68d591a233532f1a203743
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 12+ messages in thread