[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: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [Qemu-block] [PATCH 4/4] virtio-blk: Clean up start/stop with mutex and BH |
Date: |
Thu, 17 Mar 2016 16:07:22 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 |
On 17/03/2016 16:00, Stefan Hajnoczi wrote:
>> > + data = g_new(VirtIOBlockStartData, 1);
>> > + data->vblk = vblk;
>> > + data->bh = aio_bh_new(s->ctx, virtio_blk_data_plane_start_bh_cb,
>> > data);
>> > + qemu_bh_schedule(data->bh);
>> > + qemu_mutex_unlock(&s->start_stop_lock);
>> > return;
> This BH usage pattern is dangerous:
>
> 1. The BH is created and scheduled.
> 2. Before the BH executes the device is unrealized.
> 3. The data->bh pointer is inaccessible so we have a dangling BH that
> will access vblk after vblk has been freed.
>
> In some cases it can be safe but I don't see why the pattern is safe in
> this case. Either the BH needs to hold some sort of reference to keep
> vblk alive, or vblk needs to know about pending BHs so they can be
> deleted.
You're right. After unrealizing virtio_blk_data_plane_stop has set of
vblk->dataplane_started = false, so that's covered. However, you still
need an object_ref/object_object_unref pair.
That said, Christian wasn't testing hotplug/hot-unplug so this shouldn't
matter in his case. Let's see if we can catch the reentrancy with an
assertion...
Paolo
- [Qemu-devel] [PATCH 0/4] Tweaks around virtio-blk start/stop, Fam Zheng, 2016/03/16
- [Qemu-devel] [PATCH 1/4] block: Use drained section in bdrv_set_aio_context, Fam Zheng, 2016/03/16
- [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 <=
- 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, 2016/03/23
- 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