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

[Avi Kivity <avi@redhat.com>]

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.

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

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.