Re: [Qemu-devel] [PATCH 13/15] hotplug: introduce qdev_unplug_complete() to remove device from views

[Paolo Bonzini]

Il 08/08/2012 08:25, Liu Ping Fan ha scritto:
> +void qdev_unplug_complete(DeviceState *dev, Error **errp)
> +{
> +    /* isolate from mem view */
> +    qdev_unmap(dev);
> +    qemu_lock_devtree();
> +    /* isolate from device tree */
> +    qdev_unset_parent(dev);
> +    qemu_unlock_devtree();
> +    object_unref(OBJECT(dev));

Rather than deferring the free, you should defer the unref.  Otherwise
the following can happen when you have "real" RCU access to the memory
map on the read-side:

    VCPU thread                    I/O thread
=====================================================================
    get MMIO request
    rcu_read_lock()
    walk memory map
                                   qdev_unmap()
                                   lock_devtree()
                                   ...
                                   unlock_devtree
                                   unref dev -> refcnt=0, free enqueued
    ref()
    rcu_read_unlock()
                                   free()
    <dangling pointer!>

If you defer the unref, you have instead

    VCPU thread                    I/O thread
=====================================================================
    get MMIO request
    rcu_read_lock()
    walk memory map
                                   qdev_unmap()
                                   lock_devtree()
                                   ...
                                   unlock_devtree
                                   unref is enqueued
    ref() -> refcnt = 2
    rcu_read_unlock()
                                   unref() -> refcnt=1
    unref() -> refcnt = 1

So this also makes patch 14 unnecessary.

Paolo

> +}