public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH 1/1] io_uring: fix lost getsockopt completions
@ 2024-07-16 18:05 Pavel Begunkov
  2024-07-16 18:48 ` Breno Leitao
  2024-07-18 20:34 ` Jens Axboe
  0 siblings, 2 replies; 3+ messages in thread
From: Pavel Begunkov @ 2024-07-16 18:05 UTC (permalink / raw)
  To: io-uring; +Cc: Jens Axboe, asml.silence, Kanchan Joshi, Breno Leitao

There is a report that iowq executed getsockopt never completes. The
reason being that io_uring_cmd_sock() can return a positive result, and
io_uring_cmd() propagates it back to core io_uring, instead of IOU_OK.
In case of io_wq_submit_work(), the request will be dropped without
completing it.

The offending code was introduced by a hack in
a9c3eda7eada9 ("io_uring: fix submission-failure handling for uring-cmd"),
however it was fine until getsockopt was introduced and started
returning positive results.

The right solution is to always return IOU_OK, since
e0b23d9953b0c ("io_uring: optimise ltimeout for inline execution"),
we should be able to do it without problems, however for the sake of
backporting and minimising side effects, let's keep returning negative
return codes and otherwise do IOU_OK.

Link: https://github.com/axboe/liburing/issues/1181
Cc: [email protected]
Fixes: 8e9fad0e70b7b ("io_uring: Add io_uring command support for sockets")
Signed-off-by: Pavel Begunkov <[email protected]>
---
 io_uring/uring_cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
index 21ac5fb2d5f0..a54163a83968 100644
--- a/io_uring/uring_cmd.c
+++ b/io_uring/uring_cmd.c
@@ -265,7 +265,7 @@ int io_uring_cmd(struct io_kiocb *req, unsigned int issue_flags)
 		req_set_fail(req);
 	io_req_uring_cleanup(req, issue_flags);
 	io_req_set_res(req, ret, 0);
-	return ret;
+	return ret < 0 ? ret : IOU_OK;
 }
 
 int io_uring_cmd_import_fixed(u64 ubuf, unsigned long len, int rw,
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/1] io_uring: fix lost getsockopt completions
  2024-07-16 18:05 [PATCH 1/1] io_uring: fix lost getsockopt completions Pavel Begunkov
@ 2024-07-16 18:48 ` Breno Leitao
  2024-07-18 20:34 ` Jens Axboe
  1 sibling, 0 replies; 3+ messages in thread
From: Breno Leitao @ 2024-07-16 18:48 UTC (permalink / raw)
  To: Pavel Begunkov; +Cc: io-uring, Jens Axboe, Kanchan Joshi

On Tue, Jul 16, 2024 at 07:05:46PM +0100, Pavel Begunkov wrote:
> There is a report that iowq executed getsockopt never completes. The
> reason being that io_uring_cmd_sock() can return a positive result, and
> io_uring_cmd() propagates it back to core io_uring, instead of IOU_OK.
> In case of io_wq_submit_work(), the request will be dropped without
> completing it.
> 
> The offending code was introduced by a hack in
> a9c3eda7eada9 ("io_uring: fix submission-failure handling for uring-cmd"),
> however it was fine until getsockopt was introduced and started
> returning positive results.
> 
> The right solution is to always return IOU_OK, since
> e0b23d9953b0c ("io_uring: optimise ltimeout for inline execution"),
> we should be able to do it without problems, however for the sake of
> backporting and minimising side effects, let's keep returning negative
> return codes and otherwise do IOU_OK.
> 
> Link: https://github.com/axboe/liburing/issues/1181
> Cc: [email protected]
> Fixes: 8e9fad0e70b7b ("io_uring: Add io_uring command support for sockets")
> Signed-off-by: Pavel Begunkov <[email protected]>

Reviewed-by: Breno Leitao <[email protected]>

Thanks for the fix.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/1] io_uring: fix lost getsockopt completions
  2024-07-16 18:05 [PATCH 1/1] io_uring: fix lost getsockopt completions Pavel Begunkov
  2024-07-16 18:48 ` Breno Leitao
@ 2024-07-18 20:34 ` Jens Axboe
  1 sibling, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2024-07-18 20:34 UTC (permalink / raw)
  To: io-uring, Pavel Begunkov; +Cc: Kanchan Joshi, Breno Leitao


On Tue, 16 Jul 2024 19:05:46 +0100, Pavel Begunkov wrote:
> There is a report that iowq executed getsockopt never completes. The
> reason being that io_uring_cmd_sock() can return a positive result, and
> io_uring_cmd() propagates it back to core io_uring, instead of IOU_OK.
> In case of io_wq_submit_work(), the request will be dropped without
> completing it.
> 
> The offending code was introduced by a hack in
> a9c3eda7eada9 ("io_uring: fix submission-failure handling for uring-cmd"),
> however it was fine until getsockopt was introduced and started
> returning positive results.
> 
> [...]

Applied, thanks!

[1/1] io_uring: fix lost getsockopt completions
      commit: 2554b855a2f8605407a2018ca55cabb1af1feb61

Best regards,
-- 
Jens Axboe




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-07-18 20:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-16 18:05 [PATCH 1/1] io_uring: fix lost getsockopt completions Pavel Begunkov
2024-07-16 18:48 ` Breno Leitao
2024-07-18 20:34 ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox