Re: [PATCH V2 03/16] ublk: let ublk_fill_io_cmd() cover more things

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

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@redhat.com> 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@redhat.com>
> > ---
> >  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