On Thu, Jul 03, 2025 at 04:19:50PM -0400, Caleb Sander Mateos wrote:
> On Wed, Jul 2, 2025 at 12:04 AM Ming Lei <ming.lei@xxxxxxxxxx> wrote:
> >
> > Move check & clearing UBLK_IO_FLAG_AUTO_BUF_REG to
> > ublk_handle_auto_buf_reg(), also return buffer index from this helper.
> >
> > Also move ublk_set_auto_buf_reg() to this single helper too.
> >
> > Add ublk_config_io_buf() for setting up ublk io buffer, covers both
> > ublk buffer copy or auto buffer register.
> >
> > Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
> > ---
> > drivers/block/ublk_drv.c | 132 ++++++++++++++++++++++-----------------
> > 1 file changed, 76 insertions(+), 56 deletions(-)
> >
> > diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> > index 1780f9ce3a24..ba1b2672e7a8 100644
> > --- a/drivers/block/ublk_drv.c
> > +++ b/drivers/block/ublk_drv.c
> > @@ -48,6 +48,8 @@
> >
> > #define UBLK_MINORS (1U << MINORBITS)
> >
> > +#define UBLK_INVALID_BUF_IDX ((unsigned short)-1)
>
> u16?
Yeah.
>
> > +
> > /* private ioctl command mirror */
> > #define UBLK_CMD_DEL_DEV_ASYNC _IOC_NR(UBLK_U_CMD_DEL_DEV_ASYNC)
> > #define UBLK_CMD_UPDATE_SIZE _IOC_NR(UBLK_U_CMD_UPDATE_SIZE)
> > @@ -1983,16 +1985,53 @@ static inline int ublk_check_cmd_op(u32 cmd_op)
> > return 0;
> > }
> >
> > +static inline int ublk_set_auto_buf_reg(struct io_uring_cmd *cmd)
> > +{
> > + struct ublk_uring_cmd_pdu *pdu = ublk_get_uring_cmd_pdu(cmd);
> > +
> > + pdu->buf = ublk_sqe_addr_to_auto_buf_reg(READ_ONCE(cmd->sqe->addr));
> > +
> > + if (pdu->buf.reserved0 || pdu->buf.reserved1)
> > + return -EINVAL;
> > +
> > + if (pdu->buf.flags & ~UBLK_AUTO_BUF_REG_F_MASK)
> > + return -EINVAL;
> > + return 0;
> > +}
> > +
> > +static int ublk_handle_auto_buf_reg(struct ublk_io *io,
> > + struct io_uring_cmd *cmd,
> > + u16 *buf_idx)
> > +{
> > + if (io->flags & UBLK_IO_FLAG_AUTO_BUF_REG) {
> > + io->flags &= ~UBLK_IO_FLAG_AUTO_BUF_REG;
> > +
> > + /*
> > + * `UBLK_F_AUTO_BUF_REG` only works iff `UBLK_IO_FETCH_REQ`
> > + * and `UBLK_IO_COMMIT_AND_FETCH_REQ` are issued from same
> > + * `io_ring_ctx`.
> > + *
> > + * If this uring_cmd's io_ring_ctx isn't same with the
> > + * one for registering the buffer, it is ublk server's
> > + * responsibility for unregistering the buffer, otherwise
> > + * this ublk request gets stuck.
> > + */
> > + if (io->buf_ctx_handle == io_uring_cmd_ctx_handle(cmd) &&
> > + buf_idx)
>
> Not sure the buf_idx check here is necessary. This io->flags &
> UBLK_IO_FLAG_AUTO_BUF_REG path should only be reachable from
> ublk_commit_and_fetch(), not ublk_fetch() or UBLK_IO_NEED_GET_DATA.
Good catch, UBLK_IO_FLAG_AUTO_BUF_REG can only be set from
ublk_commit_and_fetch(), will fix it in next version.
Thanks,
Ming
