Re: [RESEND PATCH 4/5] loop: try to handle loop aio command via NOWAIT IO first

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

On Tue, Mar 11, 2025 at 12:58:46AM -0700, Christoph Hellwig wrote:
> On Tue, Mar 11, 2025 at 09:33:01AM +0800, Ming Lei wrote:
> > On Mon, Mar 10, 2025 at 12:14:44PM +0100, Christoph Hellwig wrote:
> > > 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.
> > 
> > The difference is just rcu_read_lock() vs. srcu_read_lock(), and not
> 
> Not, it also includes offloading to kblockd in more cases.

But loop doesn't run into these cases:

blk_execute_rq_nowait():
	blk_mq_run_hw_queue(hctx, hctx->flags & BLK_MQ_F_BLOCKING);

blk_mq_start_hw_queue(struct blk_mq_hw_ctx *hctx)
	blk_mq_run_hw_queue(hctx, hctx->flags & BLK_MQ_F_BLOCKING);

blk_mq_start_stopped_hw_queues
	...

> 
> >
> >
> > see any difference in typical fio workload on loop device, and the gain
> > is pretty obvious, bandwidth is increased by > 4X in aio workloads:
> > 
> > https://lore.kernel.org/linux-block/f7c9d956-2b9b-8bb4-aa49-d57323fc8eb0@redhat.com/T/#md3a6154218cb6619d8af5432cf2dd3a4a7a3dcc6
> 
> Please document that in the commit log.
> 
> > > > +	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.
> > 
> > That is not true, at least for XFS:
> 
> Your trace sees lo_rw_aio_complete called from the block layer
> splitting called from loop, I see nothing about XFS there.  But yes,
> this shows the issue discussed last week in the iomap IOCB_NOWAIT
> thread.

Looks I mis-parse the stack, but it is still returned from blkdev's ->ki_complete(),
and need to be handled.



Thanks,
Ming