From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54172) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T6m6D-0002pn-Vd for qemu-devel@nongnu.org; Wed, 29 Aug 2012 13:30:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T6m6C-0004Eg-U8 for qemu-devel@nongnu.org; Wed, 29 Aug 2012 13:30:41 -0400 Received: from thoth.sbs.de ([192.35.17.2]:29728) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T6m6C-0004ER-KV for qemu-devel@nongnu.org; Wed, 29 Aug 2012 13:30:40 -0400 Message-ID: <503E51BB.9070308@siemens.com> Date: Wed, 29 Aug 2012 19:30:35 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1345801763-24227-1-git-send-email-qemulist@gmail.com> <1345801763-24227-11-git-send-email-qemulist@gmail.com> <503792F1.4090109@redhat.com> <503B1B4B.6050108@redhat.com> <503B260E.70607@web.de> <503BA9BC.5010207@redhat.com> <503BAAF0.2020103@siemens.com> <503BB7E7.4050709@redhat.com> <503BB9C5.3030605@siemens.com> <503BBA77.4090006@redhat.com> <503BBED4.9050705@siemens.com> <503BC1EE.4060608@redhat.com> <503BCCA1.10403@siemens.com> <503C9275.5040105@siemens.com> <503E4FF4.7060506@redhat.com> In-Reply-To: <503E4FF4.7060506@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 10/10] qdev: fix create in place obj's life cycle problem List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: Paolo Bonzini , Liu Ping Fan , liu ping fan , Anthony Liguori , "qemu-devel@nongnu.org" 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