| Message ID | 20220617141201.170314-1-hao.xu@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | io_uring: net: fix bug of completing multishot accept twice | expand |
On 6/17/22 8:12 AM, Hao Xu wrote: > From: Hao Xu <howeyxu@tencent.com> > > Now that we use centralized completion in io_issue_sqe, we should skip > that for multishot accept requests since we complete them in the > specific op function. > > Fixes: 34106529422e ("io_uring: never defer-complete multi-apoll") > Signed-off-by: Hao Xu <howeyxu@tencent.com> > --- > > I retrieved the history: > > in 4e86a2c98013 ("io_uring: implement multishot mode for accept") > we add the multishot accept, it repeatly completes cqe in io_accept() > until get -EAGAIN [1], then it returns 0 to io_issue_sqe(). > io_issue_sqe() does nothing to it then. > > in 09eaa49e078c ("io_uring: handle completions in the core") > we add __io_req_complete() for IOU_OK in io_issue_sqe(). This causes at > [1], we do call __io_req_complete().But since IO_URING_F_COMPLETE_DEFER > is set, it does nothing. > > in 34106529422e ("io_uring: never defer-complete multi-apoll") > we remove IO_URING_F_COMPLETE_DEFER, but unluckily the multishot accept > test is broken, we didn't find the error. > > So it just has infuence to for-5.20, I'll update the liburing test > today. Do you mind if I fold this into: 09eaa49e078c ("io_uring: handle completions in the core") as I'm continually rebasing the 5.20 branch until 5.19 is fully sorted?
On 6/17/22 22:23, Jens Axboe wrote: > On 6/17/22 8:12 AM, Hao Xu wrote: >> From: Hao Xu <howeyxu@tencent.com> >> >> Now that we use centralized completion in io_issue_sqe, we should skip >> that for multishot accept requests since we complete them in the >> specific op function. >> >> Fixes: 34106529422e ("io_uring: never defer-complete multi-apoll") >> Signed-off-by: Hao Xu <howeyxu@tencent.com> >> --- >> >> I retrieved the history: >> >> in 4e86a2c98013 ("io_uring: implement multishot mode for accept") >> we add the multishot accept, it repeatly completes cqe in io_accept() >> until get -EAGAIN [1], then it returns 0 to io_issue_sqe(). >> io_issue_sqe() does nothing to it then. >> >> in 09eaa49e078c ("io_uring: handle completions in the core") >> we add __io_req_complete() for IOU_OK in io_issue_sqe(). This causes at >> [1], we do call __io_req_complete().But since IO_URING_F_COMPLETE_DEFER >> is set, it does nothing. >> >> in 34106529422e ("io_uring: never defer-complete multi-apoll") >> we remove IO_URING_F_COMPLETE_DEFER, but unluckily the multishot accept >> test is broken, we didn't find the error. >> >> So it just has infuence to for-5.20, I'll update the liburing test >> today. > > Do you mind if I fold this into: > > 09eaa49e078c ("io_uring: handle completions in the core") > > as I'm continually rebasing the 5.20 branch until 5.19 is fully sorted? > Please do, that is better.
On 6/17/22 8:34 AM, Hao Xu wrote: > On 6/17/22 22:23, Jens Axboe wrote: >> On 6/17/22 8:12 AM, Hao Xu wrote: >>> From: Hao Xu <howeyxu@tencent.com> >>> >>> Now that we use centralized completion in io_issue_sqe, we should skip >>> that for multishot accept requests since we complete them in the >>> specific op function. >>> >>> Fixes: 34106529422e ("io_uring: never defer-complete multi-apoll") >>> Signed-off-by: Hao Xu <howeyxu@tencent.com> >>> --- >>> >>> I retrieved the history: >>> >>> in 4e86a2c98013 ("io_uring: implement multishot mode for accept") >>> we add the multishot accept, it repeatly completes cqe in io_accept() >>> until get -EAGAIN [1], then it returns 0 to io_issue_sqe(). >>> io_issue_sqe() does nothing to it then. >>> >>> in 09eaa49e078c ("io_uring: handle completions in the core") >>> we add __io_req_complete() for IOU_OK in io_issue_sqe(). This causes at >>> [1], we do call __io_req_complete().But since IO_URING_F_COMPLETE_DEFER >>> is set, it does nothing. >>> >>> in 34106529422e ("io_uring: never defer-complete multi-apoll") >>> we remove IO_URING_F_COMPLETE_DEFER, but unluckily the multishot accept >>> test is broken, we didn't find the error. >>> >>> So it just has infuence to for-5.20, I'll update the liburing test >>> today. >> >> Do you mind if I fold this into: >> >> 09eaa49e078c ("io_uring: handle completions in the core") >> >> as I'm continually rebasing the 5.20 branch until 5.19 is fully sorted? >> > > Please do, that is better. Done, thanks.
diff --git a/io_uring/net.c b/io_uring/net.c index 207803758222..5097df5b2c46 100644 --- a/io_uring/net.c +++ b/io_uring/net.c @@ -630,7 +630,7 @@ int io_accept(struct io_kiocb *req, unsigned int issue_flags) */ if ((req->flags & IO_APOLL_MULTI_POLLED) == IO_APOLL_MULTI_POLLED) - ret = 0; + ret = IOU_ISSUE_SKIP_COMPLETE; return ret; } if (ret == -ERESTARTSYS)