On 5/24/22 10:08, Stefan Hajnoczi wrote:
On Tue, May 24, 2022 at 09:55:39AM +0200, Paolo Bonzini wrote:
On 5/22/22 17:06, Stefan Hajnoczi wrote:
However, I hit on a problem that I think Emanuele and Paolo have already
pointed out: draining is GS & IO. This might have worked under the 1 IOThread
model but it does not make sense for multi-queue. It is possible to submit I/O
requests in drained sections. How can multiple threads be in drained sections
simultaneously and possibly submit further I/O requests in their drained
sections? Those sections wouldn't be "drained" in any useful sense of the word.
Yeah, that works only if the drained sections are well-behaved.
"External" sources of I/O are fine; they are disabled using is_external, and
don't drain themselves I think.
I/O requests for a given BDS may be executing in multiple AioContexts,
so how do you call aio_disable_external() on all relevant AioContexts?
With multiqueue yeah, we have to replace aio_disable_external() with
drained_begin/end() callbacks; but I'm not talking about that yet.
In parallel to the block layer discussions, it's possible to work on
introducing a request queue lock in virtio-blk and virtio-scsi. That's the
only thing that relies on the AioContext lock outside the block layer.
I'm not sure what the request queue lock protects in virtio-blk? In
virtio-scsi I guess a lock is needed to protect SCSI target emulation
state?
Yes, but even in virtio-blk there is this code that runs in the main
thread and is currently protected by aio_context_acquire/release:
blk_drain(s->blk);
/* We drop queued requests after blk_drain() because blk_drain()
* itself can produce them. */
while (s->rq) {
req = s->rq;
s->rq = req->next;
virtqueue_detach_element(req->vq, &req->elem, 0);
virtio_blk_free_request(req);
}
Maybe it's safe to run it without a lock because it runs after
virtio_set_status(vdev, 0) but I'd rather play it safe and protect s->rq
with a lock.
Paolo
