Message ID | 20230815160521.3357063-1-stefanha@redhat.com (mailing list archive) |
---|---|
Headers | show |
Series | block-backend: process I/O in the current AioContext | expand |
Am 15.08.2023 um 18:05 hat Stefan Hajnoczi geschrieben: > Switch blk_aio_*() APIs over to multi-queue by using > qemu_get_current_aio_context() instead of blk_get_aio_context(). This change > will allow devices to process I/O in multiple IOThreads in the future. Both code paths still use blk_aio_em_aiocb_info, which is: static AioContext *blk_aio_em_aiocb_get_aio_context(BlockAIOCB *acb_) { BlkAioEmAIOCB *acb = container_of(acb_, BlkAioEmAIOCB, common); return blk_get_aio_context(acb->rwco.blk); } static const AIOCBInfo blk_aio_em_aiocb_info = { .aiocb_size = sizeof(BlkAioEmAIOCB), .get_aio_context = blk_aio_em_aiocb_get_aio_context, }; .get_aio_context() is called by bdrv_aio_cancel(), which already looks wrong before this patch because in theory it can end up polling the AioContext of a different thread. After this patch, .get_aio_context() doesn't even necessarily return the AioContext that runs the request any more. The only thing that might save us is that I can't find any device that both supports iothreads and calls bdrv_aio_cancel(). But we shouldn't rely on that. Maybe the solution is to just remove .get_aio_context altogether and use AIO_WAIT_WHILE(NULL, ...) in bdrv_aio_cancel(). Kevin
On Fri, Aug 18, 2023 at 05:24:22PM +0200, Kevin Wolf wrote: > Am 15.08.2023 um 18:05 hat Stefan Hajnoczi geschrieben: > > Switch blk_aio_*() APIs over to multi-queue by using > > qemu_get_current_aio_context() instead of blk_get_aio_context(). This change > > will allow devices to process I/O in multiple IOThreads in the future. > > Both code paths still use blk_aio_em_aiocb_info, which is: > > static AioContext *blk_aio_em_aiocb_get_aio_context(BlockAIOCB *acb_) > { > BlkAioEmAIOCB *acb = container_of(acb_, BlkAioEmAIOCB, common); > > return blk_get_aio_context(acb->rwco.blk); > } > > static const AIOCBInfo blk_aio_em_aiocb_info = { > .aiocb_size = sizeof(BlkAioEmAIOCB), > .get_aio_context = blk_aio_em_aiocb_get_aio_context, > }; > > .get_aio_context() is called by bdrv_aio_cancel(), which already looks > wrong before this patch because in theory it can end up polling the > AioContext of a different thread. After this patch, .get_aio_context() > doesn't even necessarily return the AioContext that runs the request any > more. > > The only thing that might save us is that I can't find any device that > both supports iothreads and calls bdrv_aio_cancel(). But we shouldn't > rely on that. > > Maybe the solution is to just remove .get_aio_context altogether and use > AIO_WAIT_WHILE(NULL, ...) in bdrv_aio_cancel(). I will remove AIOCBInfo.get_aio_context in v2. Stefan