From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:36308) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RKw3C-0001ZD-7G for qemu-devel@nongnu.org; Mon, 31 Oct 2011 13:53:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RKw3A-00025S-Fx for qemu-devel@nongnu.org; Mon, 31 Oct 2011 13:53:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52827) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RKw3A-000242-6G for qemu-devel@nongnu.org; Mon, 31 Oct 2011 13:53:32 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p9VHrDhs019773 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 31 Oct 2011 13:53:31 -0400 Message-ID: <4EAECF85.2010303@redhat.com> Date: Mon, 31 Oct 2011 17:40:37 +0100 From: Kevin Wolf MIME-Version: 1.0 References: <1319811501-6823-1-git-send-email-kwolf@redhat.com> <4EAAD9B4.7040206@redhat.com> <4EAEB4A9.2000203@redhat.com> <4EAEC62C.9000600@redhat.com> In-Reply-To: <4EAEC62C.9000600@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] dma: Avoid reentrancy in DMA transfer handlers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org Am 31.10.2011 17:00, schrieb Kevin Wolf: > Am 31.10.2011 16:34, schrieb Paolo Bonzini: >> On 10/31/2011 03:46 PM, Kevin Wolf wrote: >>>> Hmm, I think you should set rearm = 1 to ensure the BH is run when >>>> ultimately you leave the sync read. Sorry for not spotting this before. >>> >>> I was about to agree, but in fact adding a rearm = 1; line leads to >>> crashes, whereas in the version I posted it just works. So it looks like >>> something is wrong with doing it, even though it seemed to make perfect >>> sense at the first sight. >> >> But what will restart the DMA at the end of the synchronous I/O, then? > > bdrv_read/write are called inside fdctrl_read_data(), so the outer > DMA_run() already has rearm = 1. > > I think the more interesting question is why rescheduling can break > anything. Where would we schedule the BH additionally when it isn't > already scheduled anyway? I think I found the problem: > @@ -374,6 +381,8 @@ static void DMA_run (void) > } > } > > +out: > + running = 0; > if (rearm) > qemu_bh_schedule_idle(dma_bh); > } We should only reset running to 0 in the outermost instance. Moving the out: label a line down seems to fix the crashes. Kevin