在 2021/9/6 上午3:44, Jens Axboe 写道:
On 9/4/21 4:46 PM, Pavel Begunkov wrote:
On 9/4/21 7:40 PM, Jens Axboe wrote:
On 9/4/21 9:34 AM, Hao Xu wrote:
在 2021/9/4 上午12:29, Jens Axboe 写道:
On 9/3/21 5:00 AM, Hao Xu wrote:
Update io_accept_prep() to enable multishot mode for accept operation.
Signed-off-by: Hao Xu <haoxu@xxxxxxxxxxxxxxxxx>
---
fs/io_uring.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index eb81d37dce78..34612646ae3c 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -4861,6 +4861,7 @@ static int io_recv(struct io_kiocb *req, unsigned int issue_flags)
static int io_accept_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
{
struct io_accept *accept = &req->accept;
+ bool is_multishot;
if (unlikely(req->ctx->flags & IORING_SETUP_IOPOLL))
return -EINVAL;
@@ -4872,14 +4873,23 @@ static int io_accept_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
accept->flags = READ_ONCE(sqe->accept_flags);
accept->nofile = rlimit(RLIMIT_NOFILE);
+ is_multishot = accept->flags & IORING_ACCEPT_MULTISHOT;
+ if (is_multishot && (req->flags & REQ_F_FORCE_ASYNC))
+ return -EINVAL;
I like the idea itself as I think it makes a lot of sense to just have
an accept sitting there and generating multiple CQEs, but I'm a bit
puzzled by how you pass it in. accept->flags is the accept4(2) flags,
which can currently be:
SOCK_NONBLOCK
SOCK_CLOEXEC
While there's not any overlap here, that is mostly by chance I think. A
cleaner separation is needed here, what happens if some other accept4(2)
flag is enabled and it just happens to be the same as
IORING_ACCEPT_MULTISHOT?
Make sense, how about a new IOSQE flag, I saw not many
entries left there.
Not quite sure what the best approach would be... The mshot flag only
makes sense for a few request types, so a bit of a shame to have to
waste an IOSQE flag on it. Especially when the flags otherwise passed in
are so sparse, there's plenty of bits there.
Hence while it may not be the prettiest, perhaps using accept->flags is
ok and we just need some careful code to ensure that we never have any
overlap.
Or we can alias with some of the almost-never-used fields like
->ioprio or ->buf_index.
It's not a bad idea, as long as we can safely use flags from eg ioprio
for cases where ioprio would never be used. In that sense it's probably
safer than using buf_index.
The alternative is, as has been brougt up before, adding a flags2 and
reserving the last flag in ->flags to say "there are flags in flags2".
May I ask when do we have to use a bit in ->flags to do this? My
understanding is just leverage a __u8 in pad2[2] in io_uring_sqe to
deliver ext_flags.
Not exactly super pretty either, but we'll need to extend them at some
point.
