Re: [PATCH 2/2] io_uring: kbuf: add comments for some tricky code

[Hao Xu <hao.xu@linux.dev>]

Ping this one..
On 6/14/22 20:01, Hao Xu wrote:
> 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>
> ---
>   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 9cdbc018fd64..37f06456bf30 100644
> --- a/io_uring/kbuf.c
> +++ b/io_uring/kbuf.c
> @@ -50,6 +50,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 reach here, uring_lock has been
> +				¦* holden. Because in iowq, we already
> +				¦* cleared req->buf_list to NULL when got
> +				¦* the buffer from the ring, which means
> +				¦* we cannot be here in that case.
> +				 */
>   				req->buf_list->head++;
>   				req->buf_list = NULL;
>   			} else {
> @@ -128,12 +135,13 @@ static void __user *io_ring_buffer_select(struct io_kiocb *req, size_t *len,
>   	if (issue_flags & IO_URING_F_UNLOCKED) {
>   		/*
>   		 * 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 the buffer not being
> +		 * used by others. This does mean it'll be pinned until the IO
> +		 * completes though coming in unlocked means we're in io-wq
> +		 * context and there may be further retries in async hybrid mode.
> +		 * 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).
>   		 */
>   		req->buf_list = NULL;
>   		bl->head++;