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

[Jan Kiszka]

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
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