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

[liu ping fan]

On Thu, Aug 30, 2012 at 3:08 PM, Jan Kiszka <address@hidden> wrote:
> On 2012-08-30 07:54, liu ping fan wrote:
>> On Thu, Aug 30, 2012 at 1:40 AM, Avi Kivity <address@hidden> wrote:
>>> On 08/29/2012 10:30 AM, Jan Kiszka wrote:
>>>> On 2012-08-29 19:23, Avi Kivity wrote:
>>>>> On 08/28/2012 02:42 AM, Jan Kiszka wrote:
>>>>>>
>>>>>> Let's not talk about devices or MMIO dispatching. I think the problem is
>>>>>> way more generic, and we will face it multiple times in QEMU.
>>>>>
>>>>> The problem exists outside qemu as well.  It is one of the reasons for
>>>>> the popularity of garbage collection IMO, and the use of reference
>>>>> counting when GC is not available.
>>>>>
>>>>> This pattern is even documented in
>>>>> Documentation/DocBook/kernel-locking.tmpl:
>>>>>
>>>>> @@ -104,12 +114,11 @@
>>>>>  struct object *cache_find(int id)
>>>>>  {
>>>>>          struct object *obj;
>>>>> -        unsigned long flags;
>>>>>
>>>>> -        spin_lock_irqsave(&amp;cache_lock, flags);
>>>>> +        rcu_read_lock();
>>>>>          obj = __cache_find(id);
>>>>>          if (obj)
>>>>>                  object_get(obj);
>>>>> -        spin_unlock_irqrestore(&amp;cache_lock, flags);
>>>>> +        rcu_read_unlock();
>>>>>
>>>>>
>>>>> Of course that doesn't mean we should use it, but at least it indicates
>>>>> that it is a standard pattern.  With MemoryRegion the pattern is
>>>>> changed, since MemoryRegion is a thunk, not the object we're really
>>>>> dispatching to.
>>>>
>>>> We are dispatching according to the memory region (parameters, op
>>>> handlers, opaques). If we end up in device object is not decided at this
>>>> level. A memory region describes a dispatchable area - not to confuse
>>>> with a device that may only partially be able to receive such requests.
>>>
>> But I think the meaning of the memory region is for dispatching. If no
>> dispatching associated with mr, why need it exist in the system?
>
> Where did I say that memory regions should no longer be used for
> dispatching? The point is to keep the clean layer separation between
> memory regions and device objects instead of merging them together.
>
> Given
>
>        Object
>        ^    ^
>        |    |
> Region 1    Region 2
>
> you protect the object during dispatch, implicitly (and that is bad)
> requiring that no region must change in that period. I say what rather
> needs protection are the regions so that Region 2 can pass away and
> maybe reappear independent of Region 1. And: I won't need to know the

OK, I see, this is a strong reason.

Regards,
pingfan

> type of that object the regions are referring to in this model. That's
> the difference.
>
>>  And
>> could you elaborate that who will be the ref holder of mr?
>
> The memory subsystem while running a memory region handler. If that will
> be a reference counter or a per-region lock like Avi suggested, we still
> need to find out.
>
> Jan
>
> --
> Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
> Corporate Competence Center Embedded Linux