[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: |
Avi Kivity |
Subject: |
Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem |
Date: |
Mon, 03 Sep 2012 13:16:28 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120717 Thunderbird/14.0 |
On 09/03/2012 01:06 PM, liu ping fan wrote:
> 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 cannot call container_of(mr) in the memory core, because the
parameters to container_of() are not known. But that is not the main issue.
Something we can do is make MemoryRegionOps::object() take a mr
parameter instead of opaque. MemoryRegionOps::object() then uses
container_of() to derive the object to ref.
(works with MemoryRegionOps::ref()/MemoryRegionOps::unref() too; Jan,
this decouples Object from MemoryRegion at the cost of extra boilerplate
in client code, but it may be worthwhile as a temporary measure while we
gain more experience with this)
>
>>>> 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.
Look at cirrus-vga.c, there are many calls to the memory API there.
While I doubt that we have one region disabling itself (or a subregion
of itself), that's all code that would be run under the same device
lock, and so would deadlock.
--
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
- Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem, Avi Kivity, 2012/09/05