| Message ID | 20231123194931.171598-5-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | scsi: eliminate AioContext lock | expand |
On Thu, Nov 23, 2023 at 02:49:31PM -0500, Stefan Hajnoczi wrote: > Commit abfcd2760b3e ("dma-helpers: prevent dma_blk_cb() vs > dma_aio_cancel() race") acquired the AioContext lock inside dma_blk_cb() > to avoid a race with scsi_device_purge_requests() running in the main > loop thread. > > The SCSI code no longer calls dma_aio_cancel() from the main loop thread > while I/O is running in the IOThread AioContext. Therefore it is no > longer necessary to take this lock to protect DMAAIOCB fields. The > ->cb() function also does not require the lock because blk_aio_*() and > friends do not need the AioContext lock. > > Both hw/ide/core.c and hw/ide/macio.c also call dma_blk_io() but don't > rely on it taking the AioContext lock, so this change is safe. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > system/dma-helpers.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) Reviewed-by: Eric Blake <eblake@redhat.com>
Am 23.11.2023 um 20:49 hat Stefan Hajnoczi geschrieben: > Commit abfcd2760b3e ("dma-helpers: prevent dma_blk_cb() vs > dma_aio_cancel() race") acquired the AioContext lock inside dma_blk_cb() > to avoid a race with scsi_device_purge_requests() running in the main > loop thread. > > The SCSI code no longer calls dma_aio_cancel() from the main loop thread > while I/O is running in the IOThread AioContext. Therefore it is no > longer necessary to take this lock to protect DMAAIOCB fields. The > ->cb() function also does not require the lock because blk_aio_*() and > friends do not need the AioContext lock. > > Both hw/ide/core.c and hw/ide/macio.c also call dma_blk_io() but don't > rely on it taking the AioContext lock, so this change is safe. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> The commit message neglects to talk about dbs->io_func, which is what took the AioContext lock even before commit abfcd2760b3e. I think the reason is the same as for the previous patch. Reviewed-by: Kevin Wolf <kwolf@redhat.com>
diff --git a/system/dma-helpers.c b/system/dma-helpers.c index 36211acc7e..528117f256 100644 --- a/system/dma-helpers.c +++ b/system/dma-helpers.c @@ -119,13 +119,12 @@ static void dma_blk_cb(void *opaque, int ret) trace_dma_blk_cb(dbs, ret); - aio_context_acquire(ctx); dbs->acb = NULL; dbs->offset += dbs->iov.size; if (dbs->sg_cur_index == dbs->sg->nsg || ret < 0) { dma_complete(dbs, ret); - goto out; + return; } dma_blk_unmap(dbs); @@ -168,7 +167,7 @@ static void dma_blk_cb(void *opaque, int ret) trace_dma_map_wait(dbs); dbs->bh = aio_bh_new(ctx, reschedule_dma, dbs); cpu_register_map_client(dbs->bh); - goto out; + return; } if (!QEMU_IS_ALIGNED(dbs->iov.size, dbs->align)) { @@ -179,8 +178,6 @@ static void dma_blk_cb(void *opaque, int ret) dbs->acb = dbs->io_func(dbs->offset, &dbs->iov, dma_blk_cb, dbs, dbs->io_func_opaque); assert(dbs->acb); -out: - aio_context_release(ctx); } static void dma_aio_cancel(BlockAIOCB *acb)
Commit abfcd2760b3e ("dma-helpers: prevent dma_blk_cb() vs dma_aio_cancel() race") acquired the AioContext lock inside dma_blk_cb() to avoid a race with scsi_device_purge_requests() running in the main loop thread. The SCSI code no longer calls dma_aio_cancel() from the main loop thread while I/O is running in the IOThread AioContext. Therefore it is no longer necessary to take this lock to protect DMAAIOCB fields. The ->cb() function also does not require the lock because blk_aio_*() and friends do not need the AioContext lock. Both hw/ide/core.c and hw/ide/macio.c also call dma_blk_io() but don't rely on it taking the AioContext lock, so this change is safe. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- system/dma-helpers.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)