From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51124) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TRQBk-0007tc-Dp for qemu-devel@nongnu.org; Thu, 25 Oct 2012 12:21:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TRQBe-0001yO-GJ for qemu-devel@nongnu.org; Thu, 25 Oct 2012 12:21:44 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36220) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TRQBe-0001yF-6h for qemu-devel@nongnu.org; Thu, 25 Oct 2012 12:21:38 -0400 Message-ID: <50896707.9060405@redhat.com> Date: Thu, 25 Oct 2012 18:21:27 +0200 From: Avi Kivity MIME-Version: 1.0 References: <1350897839-29593-1-git-send-email-pingfank@linux.vnet.ibm.com> <1350897839-29593-13-git-send-email-pingfank@linux.vnet.ibm.com> <50865DB9.1060106@siemens.com> <50879606.3070004@web.de> <50890002.90701@redhat.com> <508906DC.8090502@web.de> In-Reply-To: <508906DC.8090502@web.de> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000 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" , liu ping fan , Anthony Liguori , Paolo Bonzini On 10/25/2012 11:31 AM, Jan Kiszka wrote: > On 2012-10-25 11:01, Avi Kivity wrote: >> On 10/24/2012 09:17 AM, Jan Kiszka wrote: >>>>> >>>>> This is ugly for many reasons. First of all, it is racy as the register >>>>> content may change while dropping the device lock, no? Then you would >>>>> raise or clear an IRQ spuriously. >>>>> >>>> Device state's intact is protected by busy flag, and will not broken >>> >>> Except that the busy flag concept is broken in itself. >> >> How do we fix an mmio that ends up mmio'ing back to itself, perhaps >> indirectly? Note this is broken in mainline too, but in a different way. >> >> Do we introduce clever locks that can detect deadlocks? > > That problem is already addressed (to my understanding) by blocking > nested MMIO in general. That doesn't work cross-thread. vcpu A: write to device X, dma-ing to device Y vcpu B: write to device Y, dma-ing to device X My suggestion was to drop the locks around DMA, then re-acquire the lock and re-validate data. > The brokenness of the busy flag is that it > prevents concurrent MMIO by dropping requests. Right. > >> >>> I see that we have a all-or-nothing problem here: to address this >>> properly, we need to convert the IRQ path to lock-less (or at least >>> compatible with holding per-device locks) as well. >> >> There is a transitional path where writing to a register that can cause >> IRQ changes takes both the big lock and the local lock. >> >> Eventually, though, of course all inner subsystems must be threaded for >> this work to have value. >> > > But that transitional path must not introduce regressions. Opening a > race window between IRQ cause update and event injection is such a > thing, just like dropping concurrent requests on the floor. Can you explain the race? -- error compiling committee.c: too many arguments to function