Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem

[Avi Kivity <avi@redhat.com>]

On 08/27/2012 06:19 AM, Anthony Liguori wrote:
> Liu Ping Fan <qemulist@gmail.com> writes:
>
> > From: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
> >
> > Scene:
> >   obja lies in objA, when objA's ref->0, it will be freed,
> > but at that time obja can still be in use.
> >
> > The real example is:
> > typedef struct PCIIDEState {
> >     PCIDevice dev;
> >     IDEBus bus[2]; --> create in place
> >     .....
> > }
> >
> > When without big lock protection for mmio-dispatch, we will hold
> > obj's refcnt. So memory_region_init_io() will replace the third para
> > "void *opaque" with "Object *obj".
> > With this patch, we can protect PCIIDEState from disappearing during
> > mmio-dispatch hold the IDEBus->ref.
> >
> > And the ref circle has been broken when calling qdev_delete_subtree().
> >
> > Signed-off-by: Liu Ping Fan <pingfank@linux.vnet.ibm.com>
>
> I think this is solving the wrong problem.  There are many, many
> dependencies a device may have on other devices.  Memory allocation
> isn't the only one.
>
> The problem is that we want to make sure that a device doesn't "go away"
> while an MMIO dispatch is happening.  This is easy to solve without
> touching referencing counting.
>
> The device will hold a lock while the MMIO is being dispatched.  The
> delete path simply needs to acquire that same lock.  This will ensure
> that a delete operation cannot finish while MMIO is still in flight.

Where does the lock live?  If it lives in the device, then we can't
acquire it during mmio dispatch (the device may have gone away).  If it
lives outside the device, we need to to tell the memory core about it. 
I would really like to avoid adding additional state; I think the curent
opaque should serve both to provide state to the callbacks and for life
cycle management.

> Regarding deleting a device, not all devices are capable of being
> deleted and specifically, devices that are composed within the memory of
> another device cannot be directly deleted (they can only be deleted
> as part of their parent's destruction).

This doesn't help at all.  We have devices that can be deleted, and we
would like to make the devices use the unlocked path, so we have to
account for it.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.