| Message ID | 20220823151022.3136-1-joshi.k@samsung.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | io_uring: fix submission-failure handling for uring-cmd | expand |
On 8/23/22 9:10 AM, Kanchan Joshi wrote: > If ->uring_cmd returned an error value different from -EAGAIN or > -EIOCBQUEUED, it gets overridden with IOU_OK. This invites trouble > as caller (io_uring core code) handles IOU_OK differently than other > error codes. > Fix this by returning the actual error code. Not sure if this is strictly needed, as the cqe error is set just fine. But I guess some places also check return value of the issue path. Applied.
On Tue, Aug 23, 2022 at 09:47:39AM -0600, Jens Axboe wrote: >On 8/23/22 9:10 AM, Kanchan Joshi wrote: >> If ->uring_cmd returned an error value different from -EAGAIN or >> -EIOCBQUEUED, it gets overridden with IOU_OK. This invites trouble >> as caller (io_uring core code) handles IOU_OK differently than other >> error codes. >> Fix this by returning the actual error code. > >Not sure if this is strictly needed, as the cqe error is set just >fine. But I guess some places also check return value of the issue >path. So I was testing iopoll support and ran into this issue - submission failed (expected one), control came back to this point, error code got converted to IOU_OK, and it started polling endlessly for a command that never got submitted. io_issue_sqe continued to invoke io_iopoll_req_issued() rather than bailing out.
On 8/23/22 10:47, Kanchan Joshi wrote: > On Tue, Aug 23, 2022 at 09:47:39AM -0600, Jens Axboe wrote: >> On 8/23/22 9:10 AM, Kanchan Joshi wrote: >>> If ->uring_cmd returned an error value different from -EAGAIN or >>> -EIOCBQUEUED, it gets overridden with IOU_OK. This invites trouble >>> as caller (io_uring core code) handles IOU_OK differently than other >>> error codes. >>> Fix this by returning the actual error code. >> >> Not sure if this is strictly needed, as the cqe error is set just >> fine. But I guess some places also check return value of the issue >> path. > > So I was testing iopoll support and ran into this issue - submission > failed (expected one), control came back to this point, error code > got converted to IOU_OK, and it started polling endlessly for a command > that never got submitted. > io_issue_sqe continued to invoke io_iopoll_req_issued() rather than > bailing out. Ah ok, yes for iopoll it'd make a difference...
diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c index b0e7feeed365..6f99dbd5d550 100644 --- a/io_uring/uring_cmd.c +++ b/io_uring/uring_cmd.c @@ -119,7 +119,7 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags) if (ret < 0) req_set_fail(req); io_req_set_res(req, ret, 0); - return IOU_OK; + return ret; } return IOU_ISSUE_SKIP_COMPLETE;
If ->uring_cmd returned an error value different from -EAGAIN or -EIOCBQUEUED, it gets overridden with IOU_OK. This invites trouble as caller (io_uring core code) handles IOU_OK differently than other error codes. Fix this by returning the actual error code. Signed-off-by: Kanchan Joshi <joshi.k@samsung.com> --- io_uring/uring_cmd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)