[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 11/33] scsi: only access SCSIDevice->requests from one thread
From: |
Kevin Wolf |
Subject: |
Re: [PULL 11/33] scsi: only access SCSIDevice->requests from one thread |
Date: |
Wed, 31 Jan 2024 11:17:08 +0100 |
Am 29.01.2024 um 17:30 hat Hanna Czenczek geschrieben:
> I don’t like using drain as a form of lock specifically against AioContext
> changes, but maybe Stefan is right, and we should use it in this specific
> case to get just the single problem fixed. (Though it’s not quite trivial
> either. We’d probably still want to remove the assertion from
> blk_get_aio_context(), so we don’t have to require all of its callers to
> hold a count in the in-flight counter.)
Okay, fair, maybe fixing the specific problem is more important that
solving the more generic blk_get_aio_context() race.
In this case, wouldn't it be enough to increase the in-flight counter so
that the drain before switching AioContexts would run the BH before
anything bad can happen? Does the following work?
Kevin
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 0a2eb11c56..dc09eb8024 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -120,17 +120,11 @@ static void scsi_device_for_each_req_async_bh(void
*opaque)
SCSIRequest *next;
/*
- * If the AioContext changed before this BH was called then reschedule into
- * the new AioContext before accessing ->requests. This can happen when
- * scsi_device_for_each_req_async() is called and then the AioContext is
- * changed before BHs are run.
+ * The AioContext can't have changed because we increased the in-flight
+ * counter for s->conf.blk.
*/
ctx = blk_get_aio_context(s->conf.blk);
- if (ctx != qemu_get_current_aio_context()) {
- aio_bh_schedule_oneshot(ctx, scsi_device_for_each_req_async_bh,
- g_steal_pointer(&data));
- return;
- }
+ assert(ctx == qemu_get_current_aio_context());
QTAILQ_FOREACH_SAFE(req, &s->requests, next, next) {
data->fn(req, data->fn_opaque);
@@ -138,6 +132,7 @@ static void scsi_device_for_each_req_async_bh(void *opaque)
/* Drop the reference taken by scsi_device_for_each_req_async() */
object_unref(OBJECT(s));
+ blk_dec_in_flight(s->conf.blk);
}
/*
@@ -163,6 +158,7 @@ static void scsi_device_for_each_req_async(SCSIDevice *s,
*/
object_ref(OBJECT(s));
+ blk_inc_in_flight(s->conf.blk);
aio_bh_schedule_oneshot(blk_get_aio_context(s->conf.blk),
scsi_device_for_each_req_async_bh,
data);
- Re: [PULL 11/33] scsi: only access SCSIDevice->requests from one thread, (continued)
Re: [PULL 11/33] scsi: only access SCSIDevice->requests from one thread, Hanna Czenczek, 2024/01/25
Re: [PULL 11/33] scsi: only access SCSIDevice->requests from one thread, Hanna Czenczek, 2024/01/29
- Re: [PULL 11/33] scsi: only access SCSIDevice->requests from one thread,
Kevin Wolf <=
Re: [PULL 11/33] scsi: only access SCSIDevice->requests from one thread, Hanna Czenczek, 2024/01/23