From: Noah Goldstein <goldstein.w.n@gmail.com>
To: Pavel Begunkov <asml.silence@gmail.com>
Cc: "open list:IO_URING" <io-uring@vger.kernel.org>,
Jens Axboe <axboe@kernel.dk>
Subject: Re: [PATCH 7/8] io_uring: arm poll for non-nowait files
Date: Sat, 16 Oct 2021 17:57:31 -0500 [thread overview]
Message-ID: <CAFUsyfLH46EOJHvGJRToE0GApdjdX4UhO7DgnV9S4di4O1CCMQ@mail.gmail.com> (raw)
In-Reply-To: <6e72153f8de78d836b9b7595a2a6f1c6a9f137b1.1634314022.git.asml.silence@gmail.com>
On Sat, Oct 16, 2021 at 5:19 PM Pavel Begunkov <asml.silence@gmail.com> 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 <asml.silence@gmail.com>
> ---
> 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
>
next prev parent reply other threads:[~2021-10-16 22:57 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
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 ` [PATCH 3/8] io_uring: optimise io_import_iovec fixed path Pavel Begunkov
2021-10-15 16:09 ` [PATCH 4/8] io_uring: return iovec from __io_import_iovec Pavel Begunkov
2021-10-15 16:09 ` [PATCH 5/8] io_uring: optimise fixed rw rsrc node setting Pavel Begunkov
2021-10-15 16:09 ` [PATCH 6/8] io_uring: clean io_prep_rw() Pavel Begunkov
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 [this message]
2021-10-16 23:16 ` Pavel Begunkov
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAFUsyfLH46EOJHvGJRToE0GApdjdX4UhO7DgnV9S4di4O1CCMQ@mail.gmail.com \
--to=goldstein.w.n@gmail.com \
--cc=asml.silence@gmail.com \
--cc=axboe@kernel.dk \
--cc=io-uring@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).