[PATCH for-5.20 1/6] io_uring: poll: remove unnecessary req->ref set

[PATCH for-5.20 1/6] io_uring: poll: remove unnecessary req->ref set

[Hao Xu]

From: Hao Xu <howeyxu@tencent.com>

We now don't need to set req->refcount for poll requests since the
reworked poll code ensures no request release race.

Signed-off-by: Hao Xu <howeyxu@tencent.com>
---
 io_uring/poll.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/io_uring/poll.c b/io_uring/poll.c
index 0df5eca93b16..73584c4e3e9b 100644
--- a/io_uring/poll.c
+++ b/io_uring/poll.c
@@ -683,7 +683,6 @@ int io_poll_add_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	if ((flags & IORING_POLL_ADD_MULTI) && (req->flags & REQ_F_CQE_SKIP))
 		return -EINVAL;
 
-	io_req_set_refcount(req);
 	req->apoll_events = poll->events = io_poll_parse_events(sqe, flags);
 	return 0;
 }
-- 
2.25.1

[PATCH 5.19 2/6] io_uring: openclose: fix bug of closing wrong fixed file

[Hao Xu]

From: Hao Xu <howeyxu@tencent.com>

Don't update ret until fixed file is closed, otherwise the file slot
becomes the error code.

Fixes: a7c41b4687f5 ("io_uring: let IORING_OP_FILES_UPDATE support choosing fixed file slots")
Signed-off-by: Hao Xu <howeyxu@tencent.com>
---
 io_uring/rsrc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
index d78e7f2ea91f..cf8c85d1fb59 100644
--- a/io_uring/rsrc.c
+++ b/io_uring/rsrc.c
@@ -705,8 +705,8 @@ static int io_files_update_with_index_alloc(struct io_kiocb *req,
 		if (ret < 0)
 			break;
 		if (copy_to_user(&fds[done], &ret, sizeof(ret))) {
-			ret = -EFAULT;
 			__io_close_fixed(req, issue_flags, ret);
+			ret = -EFAULT;
 			break;
 		}
 	}
-- 
2.25.1

Re: [PATCH 5.19 2/6] io_uring: openclose: fix bug of closing wrong fixed file

[Pavel Begunkov]

On 6/11/22 13:22, Hao Xu wrote:
> From: Hao Xu <howeyxu@tencent.com>
> 
> Don't update ret until fixed file is closed, otherwise the file slot
> becomes the error code.
> 
> Fixes: a7c41b4687f5 ("io_uring: let IORING_OP_FILES_UPDATE support choosing fixed file slots")
> Signed-off-by: Hao Xu <howeyxu@tencent.com>
> ---
>   io_uring/rsrc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c

Something strange is going on here, io_uring/rsrc.c was only queued for
5.20, but it's marked 5.19, weird.


> index d78e7f2ea91f..cf8c85d1fb59 100644
> --- a/io_uring/rsrc.c
> +++ b/io_uring/rsrc.c
> @@ -705,8 +705,8 @@ static int io_files_update_with_index_alloc(struct io_kiocb *req,
>   		if (ret < 0)
>   			break;
>   		if (copy_to_user(&fds[done], &ret, sizeof(ret))) {
> -			ret = -EFAULT;
>   			__io_close_fixed(req, issue_flags, ret);
> +			ret = -EFAULT;
>   			break;
>   		}
>   	}

-- 
Pavel Begunkov

Re: [PATCH 5.19 2/6] io_uring: openclose: fix bug of closing wrong fixed file

[Pavel Begunkov]

On 6/11/22 13:22, Hao Xu wrote:
> From: Hao Xu <howeyxu@tencent.com>
> 
> Don't update ret until fixed file is closed, otherwise the file slot
> becomes the error code.

I rebased and queued this and 6/6, will send them out together
later, thanks

https://github.com/isilence/linux/tree/io_uring/io_uring-5.19

> 
> Fixes: a7c41b4687f5 ("io_uring: let IORING_OP_FILES_UPDATE support choosing fixed file slots")
> Signed-off-by: Hao Xu <howeyxu@tencent.com>
> ---
>   io_uring/rsrc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
> index d78e7f2ea91f..cf8c85d1fb59 100644
> --- a/io_uring/rsrc.c
> +++ b/io_uring/rsrc.c
> @@ -705,8 +705,8 @@ static int io_files_update_with_index_alloc(struct io_kiocb *req,
>   		if (ret < 0)
>   			break;
>   		if (copy_to_user(&fds[done], &ret, sizeof(ret))) {
> -			ret = -EFAULT;
>   			__io_close_fixed(req, issue_flags, ret);
> +			ret = -EFAULT;
>   			break;
>   		}
>   	}

-- 
Pavel Begunkov

Re: [PATCH 5.19 2/6] io_uring: openclose: fix bug of closing wrong fixed file

[Hao Xu]

On 6/13/22 01:47, Pavel Begunkov wrote:
> On 6/11/22 13:22, Hao Xu wrote:
>> From: Hao Xu <howeyxu@tencent.com>
>>
>> Don't update ret until fixed file is closed, otherwise the file slot
>> becomes the error code.
> 
> I rebased and queued this and 6/6, will send them out together
> later, thanks
> 
> https://github.com/isilence/linux/tree/io_uring/io_uring-5.19
> 

Thanks for rebasing it, was busy with something yesterday.

Thanks,
Hao

[PATCH 5.19 3/6] io_uring: openclose: fix bug of unexpected return value in IORING_CLOSE_FD_AND_FILE_SLOT mode

[Hao Xu]

From: Hao Xu <howeyxu@tencent.com>

In IORING_CLOSE_FD_AND_FILE_SLOT mode, if we successfully close fixed
file but fails in close->fd >= fdt->max_fds check, cqe-res = 0 is
returned, which should be -EBADF.

Fixes: a7c41b4687f5 ("io_uring: let IORING_OP_FILES_UPDATE support choosing fixed file slots")
Signed-off-by: Hao Xu <howeyxu@tencent.com>
---
 io_uring/openclose.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/io_uring/openclose.c b/io_uring/openclose.c
index b35bf5f66dd9..4eb1f23e028a 100644
--- a/io_uring/openclose.c
+++ b/io_uring/openclose.c
@@ -248,7 +248,7 @@ int io_close(struct io_kiocb *req, unsigned int issue_flags)
 	struct io_close *close = io_kiocb_to_cmd(req);
 	struct fdtable *fdt;
 	struct file *file;
-	int ret = -EBADF;
+	int ret;
 
 	if (close->file_slot) {
 		ret = io_close_fixed(req, issue_flags);
@@ -256,6 +256,7 @@ int io_close(struct io_kiocb *req, unsigned int issue_flags)
 			goto err;
 	}
 
+	ret = -EBADF;
 	spin_lock(&files->file_lock);
 	fdt = files_fdtable(files);
 	if (close->fd >= fdt->max_fds) {
-- 
2.25.1

[PATCH for-5.20 4/6] io_uring: openclose: support separate return value for IORING_CLOSE_FD_AND_FILE_SLOT

[Hao Xu]

From: Hao Xu <howeyxu@tencent.com>

In IORING_CLOSE_FD_AND_FILE_SLOT mode, we just stop and return error
code if either of fixed or normal file close fails. But we can actually
continue to do the close even one of them fails. What we need to do is
put the two result in two place: for normal file close, put the result
in cqe->res like the previous behaviour, while for fixed file close,
put it in cqe->flags. Users should check both member to get the status
of fixed and normal file close.

Signed-off-by: Hao Xu <howeyxu@tencent.com>
---
 io_uring/openclose.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/io_uring/openclose.c b/io_uring/openclose.c
index 4eb1f23e028a..da930081c03c 100644
--- a/io_uring/openclose.c
+++ b/io_uring/openclose.c
@@ -248,12 +248,15 @@ int io_close(struct io_kiocb *req, unsigned int issue_flags)
 	struct io_close *close = io_kiocb_to_cmd(req);
 	struct fdtable *fdt;
 	struct file *file;
-	int ret;
+	int ret, ret2;
 
 	if (close->file_slot) {
 		ret = io_close_fixed(req, issue_flags);
-		if (ret || !(close->flags & IORING_CLOSE_FD_AND_FILE_SLOT))
+		if (!(close->flags & IORING_CLOSE_FD_AND_FILE_SLOT))
 			goto err;
+		else
+			ret2 = ret;
+
 	}
 
 	ret = -EBADF;
@@ -286,6 +289,6 @@ int io_close(struct io_kiocb *req, unsigned int issue_flags)
 err:
 	if (ret < 0)
 		req_set_fail(req);
-	io_req_set_res(req, ret, 0);
+	io_req_set_res(req, ret, ret2);
 	return IOU_OK;
 }
-- 
2.25.1

[PATCH for-5.20 5/6] io_uring: remove duplicate kbuf release

[Hao Xu]

From: Hao Xu <howeyxu@tencent.com>

__io_req_complete_put() is called by __io_req_complete_post, and before
that we already put kbuf, no need to do that in __io_req_complete_put()
again.

Signed-off-by: Hao Xu <howeyxu@tencent.com>
---
 io_uring/io_uring.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
index 1572ebe3cff1..a94ddd0ba507 100644
--- a/io_uring/io_uring.c
+++ b/io_uring/io_uring.c
@@ -1245,12 +1245,6 @@ static void __io_req_complete_put(struct io_kiocb *req)
 			}
 		}
 		io_req_put_rsrc(req);
-		/*
-		 * Selected buffer deallocation in io_clean_op() assumes that
-		 * we don't hold ->completion_lock. Clean them here to avoid
-		 * deadlocks.
-		 */
-		io_put_kbuf_comp(req);
 		io_dismantle_req(req);
 		io_put_task(req->task, 1);
 		wq_list_add_head(&req->comp_list, &ctx->locked_free_list);
-- 
2.25.1

[PATCH 5.19 6/6] io_uring: kbuf: fix bug of not consuming ring buffer in partial io case

[Hao Xu]

From: Hao Xu <howeyxu@tencent.com>

When we use ring-mapped provided buffer, we should consume it before
arm poll if partial io has been done. Otherwise the buffer may be used
by other requests and thus we lost the data.

Fixes: c7fb19428d67 ("io_uring: add support for ring mapped supplied buffers")
Signed-off-by: Hao Xu <howeyxu@tencent.com>
---
 io_uring/kbuf.c |  9 +++++++--
 io_uring/kbuf.h | 10 ++++++++--
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/io_uring/kbuf.c b/io_uring/kbuf.c
index d2b2b4728381..0680b63af1d2 100644
--- a/io_uring/kbuf.c
+++ b/io_uring/kbuf.c
@@ -49,8 +49,13 @@ void __io_kbuf_recycle(struct io_kiocb *req, unsigned issue_flags)
 	 */
 	if (req->flags & REQ_F_BUFFER_RING) {
 		if (req->buf_list) {
-			req->buf_index = req->buf_list->bgid;
-			req->flags &= ~REQ_F_BUFFER_RING;
+			if (req->flags & REQ_F_PARTIAL_IO) {
+				req->buf_list->head++;
+				req->buf_list = NULL;
+			} else {
+				req->buf_index = req->buf_list->bgid;
+				req->flags &= ~REQ_F_BUFFER_RING;
+			}
 		}
 		return;
 	}
diff --git a/io_uring/kbuf.h b/io_uring/kbuf.h
index b58d9d20c97e..9ecb175e60a9 100644
--- a/io_uring/kbuf.h
+++ b/io_uring/kbuf.h
@@ -58,8 +58,14 @@ static inline void io_kbuf_recycle(struct io_kiocb *req, unsigned issue_flags)
 {
 	if (!(req->flags & (REQ_F_BUFFER_SELECTED|REQ_F_BUFFER_RING)))
 		return;
-	/* don't recycle if we already did IO to this buffer */
-	if (req->flags & REQ_F_PARTIAL_IO)
+	/*
+	 * For legacy provided buffer mode, don't recycle if we already did
+	 * IO to this buffer. For ring-mapped provided buffer mode, we should
+	 * increment ring->head to explicitly monopolize the buffer to avoid
+	 * multiple use.
+	 */
+	if ((req->flags & REQ_F_BUFFER_SELECTED) &&
+	    (req->flags & REQ_F_PARTIAL_IO))
 		return;
 	__io_kbuf_recycle(req, issue_flags);
 }
-- 
2.25.1

Re: [PATCH for-5.20 1/6] io_uring: poll: remove unnecessary req->ref set

[Pavel Begunkov]

On 6/11/22 13:22, Hao Xu wrote:
> From: Hao Xu <howeyxu@tencent.com>
> 
> We now don't need to set req->refcount for poll requests since the
> reworked poll code ensures no request release race.

Nice, I took this one into the 5.20 branch, cheers

https://github.com/isilence/linux/tree/io_uring/for-5.20

> 
> Signed-off-by: Hao Xu <howeyxu@tencent.com>
> ---
>   io_uring/poll.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/io_uring/poll.c b/io_uring/poll.c
> index 0df5eca93b16..73584c4e3e9b 100644
> --- a/io_uring/poll.c
> +++ b/io_uring/poll.c
> @@ -683,7 +683,6 @@ int io_poll_add_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>   	if ((flags & IORING_POLL_ADD_MULTI) && (req->flags & REQ_F_CQE_SKIP))
>   		return -EINVAL;
>   
> -	io_req_set_refcount(req);
>   	req->apoll_events = poll->events = io_poll_parse_events(sqe, flags);
>   	return 0;
>   }

-- 
Pavel Begunkov