On 01/03/2017 17:08, Halil Pasic wrote:
> applied I do not see the problem any more. I will most likely
> turn this into a patch tomorrow. I would like to give it some more testing and
> thinking (see questions below) until tomorrow.
> 
> I should probably cc stable, or?

Yes, please do!

> 
> Q1. For this to work correctly, it seems to me, we need to be sure that
> virtio_blk_req_complete can not be happen between the newly added
> notify_guest_bh(s);
> and 
> vblk->dataplane_started = false; 
> becomes visible. How is this ensured?

blk_set_aio_context drains the block device, and the event notifiers are
not active anymore so draining the block device coincides with the last
call to virtio_blk_req_complete.

Please add a comment - it's a good observation.

> Q2. The virtio_blk_data_plane_stop should be from the thread/context
> associated with the main event loop, and with that
> vblk->dataplane_started = false too. But I think dataplane_started
> may end up being used form a different thread (e.g. req_complete).

1) virtio_queue_aio_set_host_notifier_handler stops the event notifiers

2) virtio_bus_set_host_notifier invokes them one last time before exiting

Note that this could call again virtio_queue_notify_vq and hence
virtio_device_start_ioeventfd, but dataplane won't be reactivated
because vblk->dataplane_started is still true.

> How does the sequencing work there and/or is it even important?

It is important and not really easy to get right---as shown by the bug
you found, in fact.

Thanks,

Paolo

Reply via email to