On Fri, Jul 11, 2025 at 09:25:27AM -0400, Caleb Sander Mateos wrote:
> 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.
OK, it can be kept outside ublk_fill_io_cmd() for ublk_commit_and_fetch()
only.
>
> > +
> > + /* 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.
OK.
thanks,
Ming
