[PATCH] ublk: setup ublk_io correctly in case of ublk_get_data() failure

[PATCH] ublk: setup ublk_io correctly in case of ublk_get_data() failure

[Ming Lei]

If ublk_get_data() fails, -EIOCBQUEUED is returned and the current command
becomes ASYNC. And the only reason is that mapping data can't move on,
because of no enough pages or pending signal, then the current ublk request
has to be requeued.

Once the request need to be requeued, we have to setup `ublk_io` correctly,
including io->cmd and flags, otherwise the request may not be forwarded to
ublk server successfully.

Fixes: 9810362a57cb ("ublk: don't call ublk_dispatch_req() for NEED_GET_DATA")
Reported-by: Changhui Zhong <czhong@redhat.com>
Closes: https://lore.kernel.org/linux-block/CAGVVp+VN9QcpHUz_0nasFf5q9i1gi8H8j-G-6mkBoqa3TyjRHA@mail.gmail.com/
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 drivers/block/ublk_drv.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index d36f44f5ee80..03ac394c69a7 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -1148,8 +1148,8 @@ static inline void __ublk_complete_rq(struct request *req)
 	blk_mq_end_request(req, res);
 }
 
-static void ublk_complete_io_cmd(struct ublk_io *io, struct request *req,
-				 int res, unsigned issue_flags)
+static struct io_uring_cmd *__ublk_prep_compl_io_cmd(struct ublk_io *io,
+						     struct request *req)
 {
 	/* read cmd first because req will overwrite it */
 	struct io_uring_cmd *cmd = io->cmd;
@@ -1164,6 +1164,13 @@ static void ublk_complete_io_cmd(struct ublk_io *io, struct request *req,
 	io->flags &= ~UBLK_IO_FLAG_ACTIVE;
 
 	io->req = req;
+	return cmd;
+}
+
+static void ublk_complete_io_cmd(struct ublk_io *io, struct request *req,
+				 int res, unsigned issue_flags)
+{
+	struct io_uring_cmd *cmd = __ublk_prep_compl_io_cmd(io, req);
 
 	/* tell ublksrv one io request is coming */
 	io_uring_cmd_done(cmd, res, 0, issue_flags);
@@ -2148,10 +2155,9 @@ static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
 	return 0;
 }
 
-static bool ublk_get_data(const struct ublk_queue *ubq, struct ublk_io *io)
+static bool ublk_get_data(const struct ublk_queue *ubq, struct ublk_io *io,
+			  struct request *req)
 {
-	struct request *req = io->req;
-
 	/*
 	 * We have handled UBLK_IO_NEED_GET_DATA command,
 	 * so clear UBLK_IO_FLAG_NEED_GET_DATA now and just
@@ -2178,6 +2184,7 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
 	u32 cmd_op = cmd->cmd_op;
 	unsigned tag = ub_cmd->tag;
 	int ret = -EINVAL;
+	struct request *req;
 
 	pr_devel("%s: received: cmd op %d queue %d tag %d result %d\n",
 			__func__, cmd->cmd_op, ub_cmd->q_id, tag,
@@ -2237,10 +2244,19 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
 		break;
 	case UBLK_IO_NEED_GET_DATA:
 		io->addr = ub_cmd->addr;
-		if (!ublk_get_data(ubq, io))
-			return -EIOCBQUEUED;
-
-		return UBLK_IO_RES_OK;
+		/*
+		 * ublk_get_data() may fail and fallback to requeue, so keep
+		 * uring_cmd active first and prepare for handling new requeued
+		 * request
+		 */
+		req = io->req;
+		ublk_fill_io_cmd(io, cmd, io->addr);
+		io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
+		if (likely(ublk_get_data(ubq, io, req))) {
+			__ublk_prep_compl_io_cmd(io, req);
+			return UBLK_IO_RES_OK;
+		}
+		break;
 	default:
 		goto out;
 	}
-- 
2.49.0

Re: [PATCH] ublk: setup ublk_io correctly in case of ublk_get_data() failure

[Ming Lei]

On Tue, Jun 24, 2025 at 10:20:49AM +0800, Ming Lei wrote:
> If ublk_get_data() fails, -EIOCBQUEUED is returned and the current command
> becomes ASYNC. And the only reason is that mapping data can't move on,
> because of no enough pages or pending signal, then the current ublk request
> has to be requeued.
> 
> Once the request need to be requeued, we have to setup `ublk_io` correctly,
> including io->cmd and flags, otherwise the request may not be forwarded to
> ublk server successfully.
> 
> Fixes: 9810362a57cb ("ublk: don't call ublk_dispatch_req() for NEED_GET_DATA")
> Reported-by: Changhui Zhong <czhong@redhat.com>
> Closes: https://lore.kernel.org/linux-block/CAGVVp+VN9QcpHUz_0nasFf5q9i1gi8H8j-G-6mkBoqa3TyjRHA@mail.gmail.com/
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  drivers/block/ublk_drv.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index d36f44f5ee80..03ac394c69a7 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -1148,8 +1148,8 @@ static inline void __ublk_complete_rq(struct request *req)
>  	blk_mq_end_request(req, res);
>  }
>  
> -static void ublk_complete_io_cmd(struct ublk_io *io, struct request *req,
> -				 int res, unsigned issue_flags)
> +static struct io_uring_cmd *__ublk_prep_compl_io_cmd(struct ublk_io *io,
> +						     struct request *req)
>  {
>  	/* read cmd first because req will overwrite it */
>  	struct io_uring_cmd *cmd = io->cmd;
> @@ -1164,6 +1164,13 @@ static void ublk_complete_io_cmd(struct ublk_io *io, struct request *req,
>  	io->flags &= ~UBLK_IO_FLAG_ACTIVE;
>  
>  	io->req = req;
> +	return cmd;
> +}
> +
> +static void ublk_complete_io_cmd(struct ublk_io *io, struct request *req,
> +				 int res, unsigned issue_flags)
> +{
> +	struct io_uring_cmd *cmd = __ublk_prep_compl_io_cmd(io, req);
>  
>  	/* tell ublksrv one io request is coming */
>  	io_uring_cmd_done(cmd, res, 0, issue_flags);
> @@ -2148,10 +2155,9 @@ static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
>  	return 0;
>  }
>  
> -static bool ublk_get_data(const struct ublk_queue *ubq, struct ublk_io *io)
> +static bool ublk_get_data(const struct ublk_queue *ubq, struct ublk_io *io,
> +			  struct request *req)
>  {
> -	struct request *req = io->req;
> -
>  	/*
>  	 * We have handled UBLK_IO_NEED_GET_DATA command,
>  	 * so clear UBLK_IO_FLAG_NEED_GET_DATA now and just
> @@ -2178,6 +2184,7 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
>  	u32 cmd_op = cmd->cmd_op;
>  	unsigned tag = ub_cmd->tag;
>  	int ret = -EINVAL;
> +	struct request *req;
>  
>  	pr_devel("%s: received: cmd op %d queue %d tag %d result %d\n",
>  			__func__, cmd->cmd_op, ub_cmd->q_id, tag,
> @@ -2237,10 +2244,19 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
>  		break;
>  	case UBLK_IO_NEED_GET_DATA:
>  		io->addr = ub_cmd->addr;
> -		if (!ublk_get_data(ubq, io))
> -			return -EIOCBQUEUED;
> -
> -		return UBLK_IO_RES_OK;
> +		/*
> +		 * ublk_get_data() may fail and fallback to requeue, so keep
> +		 * uring_cmd active first and prepare for handling new requeued
> +		 * request
> +		 */
> +		req = io->req;
> +		ublk_fill_io_cmd(io, cmd, io->addr);

The above io->addr assignment can be moved to ublk_fill_io_cmd(), so please ignore
this one.


Thanks, 
Ming