On Mon, Jul 7, 2025 at 9:18 PM Ming Lei <ming.lei@xxxxxxxxxx> wrote:
>
> Let ublk_fill_io_cmd() cover more things:
>
> - store io command result
>
> - clear UBLK_IO_FLAG_OWNED_BY_SRV
>
> It is fine to do above for ublk_fetch(), ublk_commit_and_fetch() and
> handling UBLK_IO_NEED_GET_DATA.
>
> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
> ---
> drivers/block/ublk_drv.c | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index d7b5ee96978a..1ab2dc74424f 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -2003,11 +2003,16 @@ static inline int ublk_check_cmd_op(u32 cmd_op)
> }
>
> static inline void ublk_fill_io_cmd(struct ublk_io *io,
> - struct io_uring_cmd *cmd, unsigned long buf_addr)
> + struct io_uring_cmd *cmd, unsigned long buf_addr,
> + int result)
> {
> io->cmd = cmd;
> io->flags |= UBLK_IO_FLAG_ACTIVE;
> io->addr = buf_addr;
> + io->res = result;
Hmm, only a single caller (ublk_commit_and_fetch()) needs to set
io->res. It seems a bit weird to move it here.
> +
> + /* now this cmd slot is owned by ublk driver */
> + io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
I would have a slight preference for keeping the two updates of
io->flags next to each other. The compiler may be able to optimize
that better.
Best,
Caleb
> }
>
> static inline void ublk_prep_cancel(struct io_uring_cmd *cmd,
> @@ -2165,7 +2170,7 @@ static int ublk_fetch(struct io_uring_cmd *cmd, struct ublk_queue *ubq,
> goto out;
> }
>
> - ublk_fill_io_cmd(io, cmd, buf_addr);
> + ublk_fill_io_cmd(io, cmd, buf_addr, 0);
> WRITE_ONCE(io->task, get_task_struct(current));
> ublk_mark_io_ready(ub, ubq);
> out:
> @@ -2221,11 +2226,7 @@ static int ublk_commit_and_fetch(const struct ublk_queue *ubq,
> return ret;
> }
>
> - ublk_fill_io_cmd(io, cmd, ub_cmd->addr);
> -
> - /* now this cmd slot is owned by ublk driver */
> - io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
> - io->res = ub_cmd->result;
> + ublk_fill_io_cmd(io, cmd, ub_cmd->addr, ub_cmd->result);
>
> if (req_op(req) == REQ_OP_ZONE_APPEND)
> req->__sector = ub_cmd->zone_append_lba;
> @@ -2345,8 +2346,7 @@ static int __ublk_ch_uring_cmd(struct io_uring_cmd *cmd,
> * request
> */
> req = io->req;
> - ublk_fill_io_cmd(io, cmd, ub_cmd->addr);
> - io->flags &= ~UBLK_IO_FLAG_OWNED_BY_SRV;
> + ublk_fill_io_cmd(io, cmd, ub_cmd->addr, 0);
> if (likely(ublk_get_data(ubq, io, req))) {
> __ublk_prep_compl_io_cmd(io, req);
> return UBLK_IO_RES_OK;
> --
> 2.47.0
>
