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

[Jan Kiszka]

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(&cache_lock, flags);
> +        rcu_read_lock();
>          obj = __cache_find(id);
>          if (obj)
>                  object_get(obj);
> -        spin_unlock_irqrestore(&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.

> 
>> Besides
>> MMIO and PIO dispatching, it will haunt us for file or event handlers,
>> any kind of callbacks etc.
>>
>> Context A                               Context B
>> ---------                               ---------
>>                                         object = lookup()
>> deregister(object)
>> modify(object) -> invalid state
>> ...                                     use(object)
>> modify(object) -> valid state
>> register(object)
>>
>> And with "object" I'm not talking about QOM but any data structure.
>>
> 
> 
> Context B
> ---------
> rcu_read_lock()
> object = lookup()
> if (object) {
>     ref(object)
> }
> rcu_read_unlock()
> 
> use(object)
> 
> unref(object)
> 
> (substitute locking scheme to conform to taste and/or dietary
> restrictions)   
> 

+ wait for refcount(object) == 0 in deregister(object). That's what I'm
proposing.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux