On Sun, Mar 09, 2025 at 12:23:08AM +0800, Ming Lei wrote:
> Try to handle loop aio command via NOWAIT IO first, then we can avoid to
> queue the aio command into workqueue.
>
> Fallback to workqueue in case of -EAGAIN.
>
> BLK_MQ_F_BLOCKING has to be set for calling into .read_iter() or
> .write_iter() which might sleep even though it is NOWAIT.
This needs performance numbers (or other reasons) justifying the
change, especially as BLK_MQ_F_BLOCKING is a bit of an overhead.
> static DEFINE_IDR(loop_index_idr);
> static DEFINE_MUTEX(loop_ctl_mutex);
> static DEFINE_MUTEX(loop_validate_mutex);
> @@ -380,8 +382,17 @@ static void lo_rw_aio_do_completion(struct loop_cmd *cmd)
>
> if (!atomic_dec_and_test(&cmd->ref))
> return;
> +
> + if (cmd->ret == -EAGAIN) {
> + struct loop_device *lo = rq->q->queuedata;
> +
> + loop_queue_work(lo, cmd);
> + return;
> + }
This looks like the wrong place for the rety, as -EAGAIN can only come from
the submissions path. i.e. we should never make it to the full completion
path for that case.
> static int lo_rw_aio(struct loop_device *lo, struct loop_cmd *cmd, loff_t pos)
> +{
> + unsigned int nr_bvec = lo_cmd_nr_bvec(cmd);
> + int ret;
> +
> + cmd->iocb.ki_flags &= ~IOCB_NOWAIT;
> + ret = lo_submit_rw_aio(lo, cmd, pos, nr_bvec);
> + if (ret != -EIOCBQUEUED)
> + lo_rw_aio_complete(&cmd->iocb, ret);
> + return 0;
This needs an explanation that it is for the fallback path and thus
clears the nowait flag.
> +}
> +
> +static int lo_rw_aio_nowait(struct loop_device *lo, struct loop_cmd *cmd, loff_t pos)
Overly long line.
> @@ -1926,6 +1955,17 @@ static blk_status_t loop_queue_rq(struct blk_mq_hw_ctx *hctx,
> break;
> }
>
> + if (cmd->use_aio) {
> + loff_t pos = ((loff_t) blk_rq_pos(rq) << 9) + lo->lo_offset;
> + int ret = lo_rw_aio_nowait(lo, cmd, pos);
> +
> + if (!ret)
> + return BLK_STS_OK;
> + if (ret != -EAGAIN)
> + return BLK_STS_IOERR;
> + /* fallback to workqueue for handling aio */
> + }
Why isn't all the logic in this branch in lo_rw_aio_nowait?
