mirror of
https://github.com/Motorhead1991/qemu.git
synced 2025-08-04 08:13:54 -06:00
virtio-blk: simplify virtio_blk_dma_restart_cb()
virtio_blk_dma_restart_cb() is tricky because the BH must deal with
virtio_blk_data_plane_start()/virtio_blk_data_plane_stop() being called.
There are two issues with the code:
1. virtio_blk_realize() should use qdev_add_vm_change_state_handler()
instead of qemu_add_vm_change_state_handler(). This ensures the
ordering with virtio_init()'s vm change state handler that calls
virtio_blk_data_plane_start()/virtio_blk_data_plane_stop() is
well-defined. Then blk's AioContext is guaranteed to be up-to-date in
virtio_blk_dma_restart_cb() and it's no longer necessary to have a
special case for virtio_blk_data_plane_start().
2. Only blk_drain() waits for virtio_blk_dma_restart_cb()'s
blk_inc_in_flight() to be decremented. The bdrv_drain() family of
functions do not wait for BlockBackend's in_flight counter to reach
zero. virtio_blk_data_plane_stop() relies on blk_set_aio_context()'s
implicit drain, but that's a bdrv_drain() and not a blk_drain().
Note that virtio_blk_reset() already correctly relies on blk_drain().
If virtio_blk_data_plane_stop() switches to blk_drain() then we can
properly wait for pending virtio_blk_dma_restart_bh() calls.
Once these issues are taken care of the code becomes simpler. This
change is in preparation for multiple IOThreads in virtio-blk where we
need to clean up the multi-threading behavior.
I ran the reproducer from commit 49b44549ac
("virtio-blk: On restart,
process queued requests in the proper context") to check that there is
no regression.
Cc: Sergio Lopez <slp@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Message-id: 20221102182337.252202-1-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
This commit is contained in:
parent
816a430c51
commit
a937f8e857
3 changed files with 26 additions and 39 deletions
|
@ -806,8 +806,10 @@ static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
|
|||
virtio_blk_handle_vq(s, vq);
|
||||
}
|
||||
|
||||
void virtio_blk_process_queued_requests(VirtIOBlock *s, bool is_bh)
|
||||
static void virtio_blk_dma_restart_bh(void *opaque)
|
||||
{
|
||||
VirtIOBlock *s = opaque;
|
||||
|
||||
VirtIOBlockReq *req = s->rq;
|
||||
MultiReqBuffer mrb = {};
|
||||
|
||||
|
@ -834,43 +836,27 @@ void virtio_blk_process_queued_requests(VirtIOBlock *s, bool is_bh)
|
|||
if (mrb.num_reqs) {
|
||||
virtio_blk_submit_multireq(s, &mrb);
|
||||
}
|
||||
if (is_bh) {
|
||||
blk_dec_in_flight(s->conf.conf.blk);
|
||||
}
|
||||
|
||||
/* Paired with inc in virtio_blk_dma_restart_cb() */
|
||||
blk_dec_in_flight(s->conf.conf.blk);
|
||||
|
||||
aio_context_release(blk_get_aio_context(s->conf.conf.blk));
|
||||
}
|
||||
|
||||
static void virtio_blk_dma_restart_bh(void *opaque)
|
||||
{
|
||||
VirtIOBlock *s = opaque;
|
||||
|
||||
qemu_bh_delete(s->bh);
|
||||
s->bh = NULL;
|
||||
|
||||
virtio_blk_process_queued_requests(s, true);
|
||||
}
|
||||
|
||||
static void virtio_blk_dma_restart_cb(void *opaque, bool running,
|
||||
RunState state)
|
||||
{
|
||||
VirtIOBlock *s = opaque;
|
||||
BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(s)));
|
||||
VirtioBusState *bus = VIRTIO_BUS(qbus);
|
||||
|
||||
if (!running) {
|
||||
return;
|
||||
}
|
||||
|
||||
/*
|
||||
* If ioeventfd is enabled, don't schedule the BH here as queued
|
||||
* requests will be processed while starting the data plane.
|
||||
*/
|
||||
if (!s->bh && !virtio_bus_ioeventfd_enabled(bus)) {
|
||||
s->bh = aio_bh_new(blk_get_aio_context(s->conf.conf.blk),
|
||||
virtio_blk_dma_restart_bh, s);
|
||||
blk_inc_in_flight(s->conf.conf.blk);
|
||||
qemu_bh_schedule(s->bh);
|
||||
}
|
||||
/* Paired with dec in virtio_blk_dma_restart_bh() */
|
||||
blk_inc_in_flight(s->conf.conf.blk);
|
||||
|
||||
aio_bh_schedule_oneshot(blk_get_aio_context(s->conf.conf.blk),
|
||||
virtio_blk_dma_restart_bh, s);
|
||||
}
|
||||
|
||||
static void virtio_blk_reset(VirtIODevice *vdev)
|
||||
|
@ -1213,7 +1199,13 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
|
|||
return;
|
||||
}
|
||||
|
||||
s->change = qemu_add_vm_change_state_handler(virtio_blk_dma_restart_cb, s);
|
||||
/*
|
||||
* This must be after virtio_init() so virtio_blk_dma_restart_cb() gets
|
||||
* called after ->start_ioeventfd() has already set blk's AioContext.
|
||||
*/
|
||||
s->change =
|
||||
qdev_add_vm_change_state_handler(dev, virtio_blk_dma_restart_cb, s);
|
||||
|
||||
blk_ram_registrar_init(&s->blk_ram_registrar, s->blk);
|
||||
blk_set_dev_ops(s->blk, &virtio_block_ops, s);
|
||||
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue