* [PATCH for-next 0/4] for-next cleanups
@ 2021-11-23 0:07 Pavel Begunkov
2021-11-23 0:07 ` [PATCH 1/4] io_uring: simplify reissue in kiocb_done Pavel Begunkov
` (4 more replies)
0 siblings, 5 replies; 7+ messages in thread
From: Pavel Begunkov @ 2021-11-23 0:07 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, Pavel Begunkov
random 5.17 cleanups
Pavel Begunkov (4):
io_uring: simplify reissue in kiocb_done
io_uring: improve send/recv error handling
io_uring: clean __io_import_iovec()
io_uring: improve argument types of kiocb_done()
fs/io_uring.c | 102 ++++++++++++++++++++++++++------------------------
1 file changed, 53 insertions(+), 49 deletions(-)
--
2.33.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/4] io_uring: simplify reissue in kiocb_done
2021-11-23 0:07 [PATCH for-next 0/4] for-next cleanups Pavel Begunkov
@ 2021-11-23 0:07 ` Pavel Begunkov
2021-11-23 0:07 ` [PATCH 2/4] io_uring: improve send/recv error handling Pavel Begunkov
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Pavel Begunkov @ 2021-11-23 0:07 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, Pavel Begunkov
Simplify failed resubmission prep in kiocb_done(), it's a bit ugly with
conditional logic and hand handling cflags / select buffers. Instead,
punt to tw and use io_req_task_complete() already handling all the
cases.
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index e98e7ce3dc39..e4f3ac35e447 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2948,17 +2948,10 @@ static void kiocb_done(struct kiocb *kiocb, ssize_t ret,
if (io_resubmit_prep(req)) {
io_req_task_queue_reissue(req);
} else {
- unsigned int cflags = io_put_rw_kbuf(req);
- struct io_ring_ctx *ctx = req->ctx;
-
req_set_fail(req);
- if (issue_flags & IO_URING_F_UNLOCKED) {
- mutex_lock(&ctx->uring_lock);
- __io_req_complete(req, issue_flags, ret, cflags);
- mutex_unlock(&ctx->uring_lock);
- } else {
- __io_req_complete(req, issue_flags, ret, cflags);
- }
+ req->result = ret;
+ req->io_task_work.func = io_req_task_complete;
+ io_req_task_work_add(req);
}
}
}
--
2.33.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/4] io_uring: improve send/recv error handling
2021-11-23 0:07 [PATCH for-next 0/4] for-next cleanups Pavel Begunkov
2021-11-23 0:07 ` [PATCH 1/4] io_uring: simplify reissue in kiocb_done Pavel Begunkov
@ 2021-11-23 0:07 ` Pavel Begunkov
2021-11-23 0:07 ` [PATCH 3/4] io_uring: clean __io_import_iovec() Pavel Begunkov
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Pavel Begunkov @ 2021-11-23 0:07 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, Pavel Begunkov
Hide all error handling under common if block, removes two extra ifs on
the success path and keeps the handling more condensed.
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 55 +++++++++++++++++++++++++++++----------------------
1 file changed, 31 insertions(+), 24 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index e4f3ac35e447..8932c4ce70b9 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -4788,17 +4788,18 @@ static int io_sendmsg(struct io_kiocb *req, unsigned int issue_flags)
min_ret = iov_iter_count(&kmsg->msg.msg_iter);
ret = __sys_sendmsg_sock(sock, &kmsg->msg, flags);
- if ((issue_flags & IO_URING_F_NONBLOCK) && ret == -EAGAIN)
- return io_setup_async_msg(req, kmsg);
- if (ret == -ERESTARTSYS)
- ret = -EINTR;
+ if (ret < min_ret) {
+ if (ret == -EAGAIN && (issue_flags & IO_URING_F_NONBLOCK))
+ return io_setup_async_msg(req, kmsg);
+ if (ret == -ERESTARTSYS)
+ ret = -EINTR;
+ req_set_fail(req);
+ }
/* fast path, check for non-NULL to avoid function call */
if (kmsg->free_iov)
kfree(kmsg->free_iov);
req->flags &= ~REQ_F_NEED_CLEANUP;
- if (ret < min_ret)
- req_set_fail(req);
__io_req_complete(req, issue_flags, ret, 0);
return 0;
}
@@ -4834,13 +4835,13 @@ static int io_send(struct io_kiocb *req, unsigned int issue_flags)
msg.msg_flags = flags;
ret = sock_sendmsg(sock, &msg);
- if ((issue_flags & IO_URING_F_NONBLOCK) && ret == -EAGAIN)
- return -EAGAIN;
- if (ret == -ERESTARTSYS)
- ret = -EINTR;
-
- if (ret < min_ret)
+ if (ret < min_ret) {
+ if (ret == -EAGAIN && (issue_flags & IO_URING_F_NONBLOCK))
+ return -EAGAIN;
+ if (ret == -ERESTARTSYS)
+ ret = -EINTR;
req_set_fail(req);
+ }
__io_req_complete(req, issue_flags, ret, 0);
return 0;
}
@@ -5017,10 +5018,15 @@ static int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
ret = __sys_recvmsg_sock(sock, &kmsg->msg, req->sr_msg.umsg,
kmsg->uaddr, flags);
- if (force_nonblock && ret == -EAGAIN)
- return io_setup_async_msg(req, kmsg);
- if (ret == -ERESTARTSYS)
- ret = -EINTR;
+ if (ret < min_ret) {
+ if (ret == -EAGAIN && force_nonblock)
+ return io_setup_async_msg(req, kmsg);
+ if (ret == -ERESTARTSYS)
+ ret = -EINTR;
+ req_set_fail(req);
+ } else if ((flags & MSG_WAITALL) && (kmsg->msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))) {
+ req_set_fail(req);
+ }
if (req->flags & REQ_F_BUFFER_SELECTED)
cflags = io_put_recv_kbuf(req);
@@ -5028,8 +5034,6 @@ static int io_recvmsg(struct io_kiocb *req, unsigned int issue_flags)
if (kmsg->free_iov)
kfree(kmsg->free_iov);
req->flags &= ~REQ_F_NEED_CLEANUP;
- if (ret < min_ret || ((flags & MSG_WAITALL) && (kmsg->msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))))
- req_set_fail(req);
__io_req_complete(req, issue_flags, ret, cflags);
return 0;
}
@@ -5076,15 +5080,18 @@ static int io_recv(struct io_kiocb *req, unsigned int issue_flags)
min_ret = iov_iter_count(&msg.msg_iter);
ret = sock_recvmsg(sock, &msg, flags);
- if (force_nonblock && ret == -EAGAIN)
- return -EAGAIN;
- if (ret == -ERESTARTSYS)
- ret = -EINTR;
out_free:
+ if (ret < min_ret) {
+ if (ret == -EAGAIN && force_nonblock)
+ return -EAGAIN;
+ if (ret == -ERESTARTSYS)
+ ret = -EINTR;
+ req_set_fail(req);
+ } else if ((flags & MSG_WAITALL) && (msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))) {
+ req_set_fail(req);
+ }
if (req->flags & REQ_F_BUFFER_SELECTED)
cflags = io_put_recv_kbuf(req);
- if (ret < min_ret || ((flags & MSG_WAITALL) && (msg.msg_flags & (MSG_TRUNC | MSG_CTRUNC))))
- req_set_fail(req);
__io_req_complete(req, issue_flags, ret, cflags);
return 0;
}
--
2.33.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/4] io_uring: clean __io_import_iovec()
2021-11-23 0:07 [PATCH for-next 0/4] for-next cleanups Pavel Begunkov
2021-11-23 0:07 ` [PATCH 1/4] io_uring: simplify reissue in kiocb_done Pavel Begunkov
2021-11-23 0:07 ` [PATCH 2/4] io_uring: improve send/recv error handling Pavel Begunkov
@ 2021-11-23 0:07 ` Pavel Begunkov
2021-11-23 7:30 ` Dan Carpenter
2021-11-23 0:07 ` [PATCH 4/4] io_uring: improve argument types of kiocb_done() Pavel Begunkov
2021-11-23 19:24 ` [PATCH for-next 0/4] for-next cleanups Jens Axboe
4 siblings, 1 reply; 7+ messages in thread
From: Pavel Begunkov @ 2021-11-23 0:07 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, Pavel Begunkov, Dan Carpenter
Apparently, implicit 0 to NULL conversion with ERR_PTR is not
recommended and makes some tooling like Smatch to complain. Handle it
explicitly, compilers are perfectly capable to optimise it out.
Link: https://lore.kernel.org/all/20211108134937.GA2863@kili/
Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 8932c4ce70b9..3b44867d4499 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -3179,10 +3179,12 @@ static struct iovec *__io_import_iovec(int rw, struct io_kiocb *req,
size_t sqe_len;
ssize_t ret;
- BUILD_BUG_ON(ERR_PTR(0) != NULL);
-
- if (opcode == IORING_OP_READ_FIXED || opcode == IORING_OP_WRITE_FIXED)
- return ERR_PTR(io_import_fixed(req, rw, iter));
+ if (opcode == IORING_OP_READ_FIXED || opcode == IORING_OP_WRITE_FIXED) {
+ ret = io_import_fixed(req, rw, iter);
+ if (ret)
+ return ERR_PTR(ret);
+ return NULL;
+ }
/* buffer index only valid with fixed read/write, or buffer select */
if (unlikely(req->buf_index && !(req->flags & REQ_F_BUFFER_SELECT)))
@@ -3200,15 +3202,18 @@ static struct iovec *__io_import_iovec(int rw, struct io_kiocb *req,
}
ret = import_single_range(rw, buf, sqe_len, s->fast_iov, iter);
- return ERR_PTR(ret);
+ if (ret)
+ return ERR_PTR(ret);
+ return NULL;
}
iovec = s->fast_iov;
if (req->flags & REQ_F_BUFFER_SELECT) {
ret = io_iov_buffer_select(req, iovec, issue_flags);
- if (!ret)
- iov_iter_init(iter, rw, iovec, 1, iovec->iov_len);
- return ERR_PTR(ret);
+ if (ret)
+ return ERR_PTR(ret);
+ iov_iter_init(iter, rw, iovec, 1, iovec->iov_len);
+ return NULL;
}
ret = __import_iovec(rw, buf, sqe_len, UIO_FASTIOV, &iovec, iter,
--
2.33.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/4] io_uring: improve argument types of kiocb_done()
2021-11-23 0:07 [PATCH for-next 0/4] for-next cleanups Pavel Begunkov
` (2 preceding siblings ...)
2021-11-23 0:07 ` [PATCH 3/4] io_uring: clean __io_import_iovec() Pavel Begunkov
@ 2021-11-23 0:07 ` Pavel Begunkov
2021-11-23 19:24 ` [PATCH for-next 0/4] for-next cleanups Jens Axboe
4 siblings, 0 replies; 7+ messages in thread
From: Pavel Begunkov @ 2021-11-23 0:07 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, Pavel Begunkov
kiocb_done() accepts a pointer to struct kiocb, pass struct io_kiocb
(i.e. io_uring's request) instead so we can get rid of useless
container_of().
Signed-off-by: Pavel Begunkov <[email protected]>
---
fs/io_uring.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/fs/io_uring.c b/fs/io_uring.c
index 3b44867d4499..2158d5c4fd0e 100644
--- a/fs/io_uring.c
+++ b/fs/io_uring.c
@@ -2922,10 +2922,9 @@ static inline void io_rw_done(struct kiocb *kiocb, ssize_t ret)
}
}
-static void kiocb_done(struct kiocb *kiocb, ssize_t ret,
+static void kiocb_done(struct io_kiocb *req, ssize_t ret,
unsigned int issue_flags)
{
- struct io_kiocb *req = container_of(kiocb, struct io_kiocb, rw.kiocb);
struct io_async_rw *io = req->async_data;
/* add previously done IO, if any */
@@ -2937,11 +2936,11 @@ static void kiocb_done(struct kiocb *kiocb, ssize_t ret,
}
if (req->flags & REQ_F_CUR_POS)
- req->file->f_pos = kiocb->ki_pos;
- if (ret >= 0 && (kiocb->ki_complete == io_complete_rw))
+ req->file->f_pos = req->rw.kiocb.ki_pos;
+ if (ret >= 0 && (req->rw.kiocb.ki_complete == io_complete_rw))
__io_complete_rw(req, ret, 0, issue_flags);
else
- io_rw_done(kiocb, ret);
+ io_rw_done(&req->rw.kiocb, ret);
if (req->flags & REQ_F_REISSUE) {
req->flags &= ~REQ_F_REISSUE;
@@ -3584,7 +3583,7 @@ static int io_read(struct io_kiocb *req, unsigned int issue_flags)
iov_iter_restore(&s->iter, &s->iter_state);
} while (ret > 0);
done:
- kiocb_done(kiocb, ret, issue_flags);
+ kiocb_done(req, ret, issue_flags);
out_free:
/* it's faster to check here then delegate to kfree */
if (iovec)
@@ -3681,7 +3680,7 @@ static int io_write(struct io_kiocb *req, unsigned int issue_flags)
if (ret2 == -EAGAIN && (req->ctx->flags & IORING_SETUP_IOPOLL))
goto copy_iov;
done:
- kiocb_done(kiocb, ret2, issue_flags);
+ kiocb_done(req, ret2, issue_flags);
} else {
copy_iov:
iov_iter_restore(&s->iter, &s->iter_state);
--
2.33.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/4] io_uring: clean __io_import_iovec()
2021-11-23 0:07 ` [PATCH 3/4] io_uring: clean __io_import_iovec() Pavel Begunkov
@ 2021-11-23 7:30 ` Dan Carpenter
0 siblings, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2021-11-23 7:30 UTC (permalink / raw)
To: Pavel Begunkov; +Cc: io-uring, Jens Axboe
On Tue, Nov 23, 2021 at 12:07:48AM +0000, Pavel Begunkov wrote:
> Apparently, implicit 0 to NULL conversion with ERR_PTR is not
> recommended and makes some tooling like Smatch to complain. Handle it
> explicitly, compilers are perfectly capable to optimise it out.
>
> Link: https://lore.kernel.org/all/20211108134937.GA2863@kili/
> Reported-by: Dan Carpenter <[email protected]>
> Signed-off-by: Pavel Begunkov <[email protected]>
I like that this this code is now really explicit that the NULL returns
are intentional. I had figured that out from the git log already.
Passing zero to ERR_PTR() is valid. Linus complained about this Smatch
warning. But I'm not going to delete the check because probably around
70% (complete guess) of the cases in new code are bugs. In older
kernels the Smatch warnings tend to be 100% false positives because we
fix the real bugs. Also the kbuild-bot hits a bunch of error pointer
false positives for cross compile builds but I don't have a cross
compile system set up so I haven't debugged that. :/ It has something
to do with pointers being treated as signed on those arches.
But what I really like to see is documentation explaining when a
function is going to return NULL vs an error pointer.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH for-next 0/4] for-next cleanups
2021-11-23 0:07 [PATCH for-next 0/4] for-next cleanups Pavel Begunkov
` (3 preceding siblings ...)
2021-11-23 0:07 ` [PATCH 4/4] io_uring: improve argument types of kiocb_done() Pavel Begunkov
@ 2021-11-23 19:24 ` Jens Axboe
4 siblings, 0 replies; 7+ messages in thread
From: Jens Axboe @ 2021-11-23 19:24 UTC (permalink / raw)
To: io-uring, Pavel Begunkov
On Tue, 23 Nov 2021 00:07:45 +0000, Pavel Begunkov wrote:
> random 5.17 cleanups
>
> Pavel Begunkov (4):
> io_uring: simplify reissue in kiocb_done
> io_uring: improve send/recv error handling
> io_uring: clean __io_import_iovec()
> io_uring: improve argument types of kiocb_done()
>
> [...]
Applied, thanks!
[1/4] io_uring: simplify reissue in kiocb_done
commit: 06bdea20c1076471f7ab7d3ad7f35cbcbd59a8e3
[2/4] io_uring: improve send/recv error handling
commit: 7297ce3d59449de49d3c9e1f64ae25488750a1fc
[3/4] io_uring: clean __io_import_iovec()
commit: f3251183b298912e09297cb22614361c63122e82
[4/4] io_uring: improve argument types of kiocb_done()
commit: 2ea537ca02b12e6e03dfcac82013ff289a75eed8
Best regards,
--
Jens Axboe
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-11-23 19:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-11-23 0:07 [PATCH for-next 0/4] for-next cleanups Pavel Begunkov
2021-11-23 0:07 ` [PATCH 1/4] io_uring: simplify reissue in kiocb_done Pavel Begunkov
2021-11-23 0:07 ` [PATCH 2/4] io_uring: improve send/recv error handling Pavel Begunkov
2021-11-23 0:07 ` [PATCH 3/4] io_uring: clean __io_import_iovec() Pavel Begunkov
2021-11-23 7:30 ` Dan Carpenter
2021-11-23 0:07 ` [PATCH 4/4] io_uring: improve argument types of kiocb_done() Pavel Begunkov
2021-11-23 19:24 ` [PATCH for-next 0/4] for-next cleanups Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox