Re: [PATCH V6 04/24] ublk: handle UBLK_U_IO_PREP_IO_CMDS

[Jens Axboe <axboe@kernel.dk>]

Claude says:

This commit implements handling of UBLK_U_IO_PREP_IO_CMDS for batch I/O
preparation. It adds a per-io spinlock and walks the uring_cmd buffer to
prepare each I/O element.

> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index 9fd81213b81b..6aef1a856a80 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c

[ ... ]

> +static int ublk_batch_prep_io(struct ublk_queue *ubq,
> +			      const struct ublk_batch_io_data *data,
> +			      const struct ublk_elem_header *elem)
> +{
> +	struct ublk_io *io = &ubq->ios[elem->tag];
> +	const struct ublk_batch_io *uc = &data->header;
> +	union ublk_io_buf buf = { 0 };
> +	int ret;
> +
> +	if (ublk_dev_support_auto_buf_reg(data->ub))
> +		buf.auto_reg = ublk_batch_auto_buf_reg(uc, elem);
> +	else if (ublk_dev_need_map_io(data->ub)) {
> +		buf.addr = ublk_batch_buf_addr(uc, elem);
> +
> +		ret = ublk_check_fetch_buf(data->ub, buf.addr);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ublk_io_lock(io);
> +	ret = __ublk_fetch(data->cmd, data->ub, io);
                ^^^^^^^^^^^^

Can this cause a sleeping-while-atomic bug? The call chain is:

  ublk_batch_prep_io()
    -> ublk_io_lock(io)         // acquires spinlock
    -> __ublk_fetch()
       -> ublk_mark_io_ready()
          -> ublk_reset_io_flags()
             -> mutex_lock(&ub->cancel_mutex)  // sleeps

When preparing the last I/O that makes the device ready (nr_io_ready ==
total), ublk_mark_io_ready() calls ublk_reset_io_flags() which tries to
acquire ub->cancel_mutex. This is a sleeping operation but io->lock
spinlock is held.

The existing ublk_fetch() avoids this by NOT holding the spinlock when
calling __ublk_fetch() - it only holds ub->mutex. Should ublk_batch_prep_io()
move the __ublk_fetch() call outside the spinlock-protected section?

-- 
Jens Axboe