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

[Ming Lei <ming.lei@redhat.com>]

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