[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] virtio-blk: Clean up start/sto
From: |
Christian Borntraeger |
Subject: |
Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] virtio-blk: Clean up start/stop with mutex and BH |
Date: |
Wed, 23 Mar 2016 10:12:43 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 |
On 03/23/2016 10:08 AM, Paolo Bonzini wrote:
>
>
> On 23/03/2016 09:10, Cornelia Huck wrote:
>>> - /* Kick right away to begin processing requests already in vring */
>>> - event_notifier_set(virtio_queue_get_host_notifier(s->vq));
>>> + vblk->dataplane_started = true;
>>>
>>> - /* Get this show started by hooking up our callbacks */
>>> + /* Get this show started by hooking up our callbacks. */
>>> aio_context_acquire(s->ctx);
>>> virtio_queue_aio_set_host_notifier_handler(s->vq, s->ctx, true, true);
>>> aio_context_release(s->ctx);
>>> + atomic_dec(&s->starting);
>>> +
>>> + /* Kick right away to begin processing requests already in vring */
>>> + event_notifier_set(virtio_queue_get_host_notifier(s->vq));
>>
>> I'm wondering whether moving this event_notifier_set() masks something?
>> IOW, may we run into trouble if the event notifier is set from some
>> other path before the callbacks are set up properly?
>
> The reentrancy check should catch that... But:
>
> 1) the patch really makes no difference, your fix is enough for me
Tu Bo, can you test with master + Cornelias 6 refactoring patches and nothing
on top?
> 2) vblk->dataplane_started becomes true before the callbacks are set;
> that should be enough.
>
> 3) this matches what I tested, but it would of course be better if the
> assertions on s->starting suffice
>
>>> - if (!vblk->dataplane_started || s->stopping) {
>>> + if (!vblk->dataplane_started) {
>>
>> No fear of reentrancy here?
>
> No, because this is only invoked from reset, hence only from the CPU
> thread and only under the BQL.
>
> On start, reentrancy happens between iothread (running outside BQL) and
> a CPU thread (running within BQL).
>
> Paolo
>
>>> return;
>>> }
>>>
>>> @@ -255,7 +251,7 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
>>> vblk->dataplane_started = false;
>>> return;
>>> }
>>> - s->stopping = true;
>>> +
>>> trace_virtio_blk_data_plane_stop(s);
>>>
>>> aio_context_acquire(s->ctx);
>>> @@ -274,5 +270,4 @@ void virtio_blk_data_plane_stop(VirtIOBlockDataPlane *s)
>>> k->set_guest_notifiers(qbus->parent, 1, false);
>>>
>>> vblk->dataplane_started = false;
>>> - s->stopping = false;
>>> }
>>>
>>
>
- Re: [Qemu-devel] [PATCH 1/4] block: Use drained section in bdrv_set_aio_context, (continued)
- [Qemu-devel] [PATCH 2/4] block-backend: Introduce blk_drained_begin/end, Fam Zheng, 2016/03/16
- [Qemu-devel] [PATCH 3/4] virtio-blk: Use blk_drained_begin/end around dataplane stop, Fam Zheng, 2016/03/16
- [Qemu-devel] [PATCH 4/4] virtio-blk: Clean up start/stop with mutex and BH, Fam Zheng, 2016/03/16
- Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] virtio-blk: Clean up start/stop with mutex and BH, Stefan Hajnoczi, 2016/03/17
- Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] virtio-blk: Clean up start/stop with mutex and BH, Paolo Bonzini, 2016/03/17
- Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] virtio-blk: Clean up start/stop with mutex and BH, Fam Zheng, 2016/03/22
- Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] virtio-blk: Clean up start/stop with mutex and BH, Paolo Bonzini, 2016/03/22
- Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] virtio-blk: Clean up start/stop with mutex and BH, Cornelia Huck, 2016/03/23
- Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] virtio-blk: Clean up start/stop with mutex and BH, Paolo Bonzini, 2016/03/23
- Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] virtio-blk: Clean up start/stop with mutex and BH,
Christian Borntraeger <=
- Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] virtio-blk: Clean up start/stop with mutex and BH, tu bo, 2016/03/24
- Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] virtio-blk: Clean up start/stop with mutex and BH, Cornelia Huck, 2016/03/24
- Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] virtio-blk: Clean up start/stop with mutex and BH, Cornelia Huck, 2016/03/24
- Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] virtio-blk: Clean up start/stop with mutex and BH, Cornelia Huck, 2016/03/24
Re: [Qemu-devel] [PATCH 0/4] Tweaks around virtio-blk start/stop, Paolo Bonzini, 2016/03/16