All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hao Xu <hao.xu@linux.dev>
To: io-uring@vger.kernel.org
Cc: Jens Axboe <axboe@kernel.dk>, Pavel Begunkov <asml.silence@gmail.com>
Subject: [PATCH v2] io_uring: kbuf: add comments for some tricky code
Date: Fri, 17 Jun 2022 13:04:29 +0800	[thread overview]
Message-ID: <20220617050429.94293-1-hao.xu@linux.dev> (raw)

From: Hao Xu <howeyxu@tencent.com>

Add comments to explain why it is always under uring lock when
incrementing head in __io_kbuf_recycle. And rectify one comemnt about
kbuf consuming in iowq case.

Signed-off-by: Hao Xu <howeyxu@tencent.com>
---

v1->v2:
 - modify comments to make it look better
 - remove weird chars which turns out to be some helper line by some vim plugin

 io_uring/kbuf.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
index 07dbb0d17aae..d641d1f9450f 100644
--- a/io_uring/kbuf.c
+++ b/io_uring/kbuf.c
@@ -52,6 +52,13 @@ void __io_kbuf_recycle(struct io_kiocb *req, unsigned issue_flags)
 	if (req->flags & REQ_F_BUFFER_RING) {
 		if (req->buf_list) {
 			if (req->flags & REQ_F_PARTIAL_IO) {
+				/*
+				 * If we end up here, then the io_uring_lock has
+				 * been kept held since we retrieved the buffer.
+				 * For the io-wq case, we already cleared
+				 * req->buf_list when the buffer was retrieved,
+				 * hence it cannot be set here for that case.
+				 */
 				req->buf_list->head++;
 				req->buf_list = NULL;
 			} else {
@@ -163,12 +170,13 @@ static void __user *io_ring_buffer_select(struct io_kiocb *req, size_t *len,
 	if (issue_flags & IO_URING_F_UNLOCKED || !file_can_poll(req->file)) {
 		/*
 		 * If we came in unlocked, we have no choice but to consume the
-		 * buffer here. This does mean it'll be pinned until the IO
-		 * completes. But coming in unlocked means we're in io-wq
-		 * context, hence there should be no further retry. For the
-		 * locked case, the caller must ensure to call the commit when
-		 * the transfer completes (or if we get -EAGAIN and must poll
-		 * or retry).
+		 * buffer here, otherwise nothing ensures that the buffer won't
+		 * get used by others. This does mean it'll be pinned until the
+		 * IO completes, coming in unlocked means we're being called from
+		 * io-wq context and there may be further retries in async hybrid
+		 * mode. For the locked case, the caller must call commit when
+		 * the transfer completes (or if we get -EAGAIN and must poll of
+		 * retry).
 		 */
 		req->buf_list = NULL;
 		bl->head++;

base-commit: de4873338bd3e284abffa7c28b3b653244fb655c
-- 
2.25.1


             reply	other threads:[~2022-06-17  5:04 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-17  5:04 Hao Xu [this message]
2022-06-17 11:38 ` [PATCH v2] io_uring: kbuf: add comments for some tricky code Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220617050429.94293-1-hao.xu@linux.dev \
    --to=hao.xu@linux.dev \
    --cc=asml.silence@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=io-uring@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.