From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:49909) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T8SZe-0008PX-53 for qemu-devel@nongnu.org; Mon, 03 Sep 2012 05:04:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T8SZY-0001fd-Aj for qemu-devel@nongnu.org; Mon, 03 Sep 2012 05:04:02 -0400 Received: from mx1.redhat.com ([209.132.183.28]:11723) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T8SPU-0007V1-ND for qemu-devel@nongnu.org; Mon, 03 Sep 2012 04:53:32 -0400 Message-ID: <50446FD9.3040805@redhat.com> Date: Mon, 03 Sep 2012 11:52:41 +0300 From: Avi Kivity 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> <503E51BB.9070308@siemens.com> <503E5422.5050805@redhat.com> <503F1172.70103@siemens.com> <5041CB73.4050800@redhat.com> In-Reply-To: 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: liu ping fan Cc: Jan Kiszka , Liu Ping Fan , "qemu-devel@nongnu.org" , Anthony Liguori , Paolo Bonzini On 09/03/2012 10:44 AM, liu ping fan wrote: >>> >> >> If we make the refcount/lock internal to the region, we must remove the >> opaque, since the region won't protect it. >> >> Replacing the opaque with container_of(mr) doesn't help, since we can't >> refcount mr, only mr->impl. >> > I think you mean if using MemoryRegionImpl, then at this level, we had > better not touch the refcnt of container_of(mr) and should face the > mr->impl->refcnt. Right? I did not understand the second part, sorry. >> We could externalize the refcounting and push it into device code. This >> means: >> >> memory_region_init_io(&s->mem, dev) >> >> ... >> >> object_ref(dev) >> memory_region_add_subregion(..., &dev->mr) >> >> ... >> >> memory_region_del_subregion(..., &dev->mr) // implied flush >> object_unref(dev) >> > I think "object_ref(dev)" just another style to push > MemoryRegionOps::object() to device level. And I think we can bypass > it. The caller (unplug, pci-reconfig ) of > memory_region_del_subregion() ensure the @dev is valid. > If the implied flush is implemented in synchronize, _del_subregion() > will guarantee no reader for @dev->mr and @dev exist any longer. The above code has a deadlock. memory_region_del_subregion() may be called under the device lock (since it may be the result of mmio to the device), and if the flush uses synchronized_rcu(), it will wait forever for the read-side critical section to complete. > So I > think we can save both object_ref/unref(dev) for memory system. > The only problem is that whether we can implement it as synchronous or > not, is it possible that we can launch a _del_subregion(mr-X) in > mr-X's dispatcher? Yes. Real cases exist. What alternatives remain? -- error compiling committee.c: too many arguments to function