[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycl
From: |
liu ping fan |
Subject: |
Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem |
Date: |
Mon, 3 Sep 2012 18:06:37 +0800 |
On Mon, Sep 3, 2012 at 4:52 PM, Avi Kivity <address@hidden> wrote:
> On 09/03/2012 10:44 AM, liu ping fan wrote:
>>>>
>>>
>>> If we make the refcount/lock internal to the region, we must remove the
>>> opaque, since the region won't protect it.
>>>
>>> Replacing the opaque with container_of(mr) doesn't help, since we can't
>>> refcount mr, only mr->impl.
>>>
>> I think you mean if using MemoryRegionImpl, then at this level, we had
>> better not touch the refcnt of container_of(mr) and should face the
>> mr->impl->refcnt. Right?
>
> I did not understand the second part, sorry.
>
My understanding of "Replacing the opaque with container_of(mr)
doesn't help, since we can't refcount mr, only
mr->impl." is that although Object_ref(container_of(mr)) can help us
to protect it from disappearing. But apparently it is not right place
to do it it in memory core. Do I catch you meaning?
>>> We could externalize the refcounting and push it into device code. This
>>> means:
>>>
>>> memory_region_init_io(&s->mem, dev)
>>>
>>> ...
>>>
>>> object_ref(dev)
>>> memory_region_add_subregion(..., &dev->mr)
>>>
>>> ...
>>>
>>> memory_region_del_subregion(..., &dev->mr) // implied flush
>>> object_unref(dev)
>>>
>> I think "object_ref(dev)" just another style to push
>> MemoryRegionOps::object() to device level. And I think we can bypass
>> it. The caller (unplug, pci-reconfig ) of
>> memory_region_del_subregion() ensure the @dev is valid.
>> If the implied flush is implemented in synchronize, _del_subregion()
>> will guarantee no reader for @dev->mr and @dev exist any longer.
>
> The above code has a deadlock. memory_region_del_subregion() may be
> called under the device lock (since it may be the result of mmio to the
> device), and if the flush uses synchronized_rcu(), it will wait forever
> for the read-side critical section to complete.
>
But if _del_subregion() just wait for mr-X quiescent period, while
calling in mr-Y's read side critical section, then we can avoid
deadlock. I saw in pci-mapping, we delete mr-X in mr-Y read side.
>> So I
>> think we can save both object_ref/unref(dev) for memory system.
>> The only problem is that whether we can implement it as synchronous or
>> not, is it possible that we can launch a _del_subregion(mr-X) in
>> mr-X's dispatcher?
>
> Yes. Real cases exist.
Oh, I find the sample code, then, the deadlock is unavoidable in this method.
>
> What alternatives remain?
>
I think a way out may be async+refcnt
Regards,
pingfan
> --
> error compiling committee.c: too many arguments to function
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, Avi Kivity, 2012/09/01
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, Avi Kivity, 2012/09/01
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, liu ping fan, 2012/09/03
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, liu ping fan, 2012/09/03
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, liu ping fan, 2012/09/05
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, Avi Kivity, 2012/09/05
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, Jan Kiszka, 2012/09/05
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, Avi Kivity, 2012/09/05
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, Jan Kiszka, 2012/09/05
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, Avi Kivity, 2012/09/05
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, Jan Kiszka, 2012/09/05