From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34111) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQfcz-0006PV-43 for qemu-devel@nongnu.org; Tue, 23 Oct 2012 10:38:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TQfcm-0003tE-0K for qemu-devel@nongnu.org; Tue, 23 Oct 2012 10:38:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:65060) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQfcl-0003sv-OJ for qemu-devel@nongnu.org; Tue, 23 Oct 2012 10:38:31 -0400 Message-ID: <5086ABC3.1070505@redhat.com> Date: Tue, 23 Oct 2012 16:37:55 +0200 From: Avi Kivity MIME-Version: 1.0 References: <1350897839-29593-1-git-send-email-pingfank@linux.vnet.ibm.com> <1350897839-29593-6-git-send-email-pingfank@linux.vnet.ibm.com> <5085140F.2070603@redhat.com> <508684BE.1080103@redhat.com> <508685C0.60700@redhat.com> <508687C4.4030109@siemens.com> <5086899A.5020702@redhat.com> <50868ABA.2090600@siemens.com> <50868D60.4050009@redhat.com> <50869041.7030800@siemens.com> In-Reply-To: <50869041.7030800@siemens.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [patch v4 05/16] memory: introduce ref, unref interface for MemoryRegionOps List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Liu Ping Fan , Stefan Hajnoczi , Marcelo Tosatti , "qemu-devel@nongnu.org" , Anthony Liguori , Paolo Bonzini On 10/23/2012 02:40 PM, Jan Kiszka wrote: > On 2012-10-23 14:28, Avi Kivity wrote: >> On 10/23/2012 02:16 PM, Jan Kiszka wrote: >>> On 2012-10-23 14:12, Paolo Bonzini wrote: >>>> Il 23/10/2012 14:04, Jan Kiszka ha scritto: >>>>>>>>> >>>>>>>>> So the stop_machine idea is thrown away? >>>>>>> >>>>>>> IIRC I convinced myself that it's just as bad. >>>>> One tricky part with stop machine is that legacy code may trigger it >>>>> while holding the BQL, does not expect to lose that lock even for a >>>>> brief while, but synchronizing on other threads does require dropping >>>>> the lock right now. Maybe an implementation detail, but at least a nasty >>>>> one. >>>> >>>> But it would only be triggered by hot-unplug, no? >>> >>> Once all code that adds/removes memory regions from within access >>> handlers is converted. >> >> add/del is fine. memory_region_destroy() is the problem. I have >> patches queued that fix those problems and add an assert() to make sure >> we don't add more. >> >> It's not just memory regions, it's practically anything that can be >> removed and that has callbacks. The two proposals are: >> >> - qomify >> - split unplug into isolate+destroy >> - let the issuer of the callbacks manage the reference counts > > What do you mean with the last one? Call ref/unref as needed (this patchset). Here, "the issuer of the callbacks" is the memory core. For timer callbacks, it is the timer subsystem. > >> >> vs >> >> - split unplug into isolate+destroy >> - let unplug defer destruction to a bottom half, and stop_machine there >> - if we depend on the results [1], add a continuation >> >> [1] Say a monitor command wants to return only after the block device >> has been detached from qemu > > The monitor is likely harmless (as it's pretty confined). But is that > all? Hunting down all (corner) cases will make switching to this model > tricky. That is my feeling as well. The first model requires more work, but is complete. The second model is easier, but we may run into a wall if we find a case it doesn't cover. -- error compiling committee.c: too many arguments to function