On Sat, Oct 16, 2021 at 6:26 PM Pavel Begunkov [off-list ref] wrote:

On 10/16/21 23:52, Noah Goldstein wrote:

quoted

On Thu, Oct 14, 2021 at 10:13 AM Pavel Begunkov [off-list ref] wrote:

quoted

-       /* If the file doesn't support async, just async punt */
-       if (force_nonblock && !io_file_supports_nowait(req, WRITE))
-               goto copy_iov;
+               /* file path doesn't support NOWAIT for non-direct_IO */
+               if (force_nonblock && !(kiocb->ki_flags & IOCB_DIRECT) &&

You can drop this 'force_nonblock' no?

Indeed

quoted

quoted

+                   (req->flags & REQ_F_ISREG))
+                       goto copy_iov;

-       /* file path doesn't support NOWAIT for non-direct_IO */
-       if (force_nonblock && !(kiocb->ki_flags & IOCB_DIRECT) &&
-           (req->flags & REQ_F_ISREG))
-               goto copy_iov;
+               kiocb->ki_flags |= IOCB_NOWAIT;
+       } else {
+               /* Ensure we clear previously set non-block flag */
+               kiocb->ki_flags &= ~IOCB_NOWAIT;
+       }

         ret = rw_verify_area(WRITE, req->file, io_kiocb_ppos(kiocb), req->result);
         if (unlikely(ret))

...

What swapping order of conditions below:
if ((req->ctx->flags & IORING_SETUP_IOPOLL) && ret2 == -EAGAIN)

The ret2 check will almost certainly be faster than 2x deref.

Makes sense. Want to send a patch?

Done.

--
Pavel Begunkov

As an aside regarding the reorganization of io_write/io_read, maybe
it's worth it
to add seperate versions of the function for w/w.o force_nonblock?
Isn't something
that will change during the function and seems to really just be
adding 4-5 branches
(of runtime and code complexity) to each where factoring it would just
be +1 branch.