From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35931) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UmQl9-00078s-0O for qemu-devel@nongnu.org; Tue, 11 Jun 2013 11:45:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UmQl6-0000DX-ME for qemu-devel@nongnu.org; Tue, 11 Jun 2013 11:45:22 -0400 Received: from mail-ob0-x230.google.com ([2607:f8b0:4003:c01::230]:60760) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UmQl6-0000DP-Dz for qemu-devel@nongnu.org; Tue, 11 Jun 2013 11:45:20 -0400 Received: by mail-ob0-f176.google.com with SMTP id v19so11900308obq.35 for ; Tue, 11 Jun 2013 08:45:19 -0700 (PDT) Sender: fluxion Date: Tue, 11 Jun 2013 10:45:09 -0500 From: mdroth Message-ID: <20130611154509.GA1681@vm> References: <1370888600-25388-1-git-send-email-mdroth@linux.vnet.ibm.com> <51B6F660.5090004@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <51B6F660.5090004@suse.de> Subject: Re: [Qemu-devel] [PATCH] ide-test: fix failure for test_flush List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?iso-8859-1?Q?F=E4rber?= Cc: Kevin Wolf , Anthony Liguori , qemu-devel@nongnu.org, stefanha@redhat.com On Tue, Jun 11, 2013 at 12:05:20PM +0200, Andreas Färber wrote: > Am 10.06.2013 20:23, schrieb Michael Roth: > > bd07684aacfb61668ae2c25b7dd00b64f3d7c7f3 added a test to ensure BSY > > flag is set when a flush request is in flight. It does this by setting > > a blkdebug breakpoint on flush_to_os before issuing a CMD_FLUSH_CACHE. > > It then resumes CMD_FLUSH_CACHE operation and checks that BSY is unset. > > > > The actual unsetting of BSY does not occur until ide_flush_cb gets > > called in a bh, however, so in some cases this check will race with > > the actual completion. > > > > Fix this by polling the ide status register until BSY flag gets unset > > before we do our final sanity checks. According to > > f68ec8379e88502b4841a110c070e9b118d3151c this is in line with how a guest > > would determine whether or not the device is still busy. > > > > Signed-off-by: Michael Roth > > --- > > tests/ide-test.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/tests/ide-test.c b/tests/ide-test.c > > index 828e71a..7e2eb94 100644 > > --- a/tests/ide-test.c > > +++ b/tests/ide-test.c > > @@ -455,7 +455,10 @@ static void test_flush(void) > > data = inb(IDE_BASE + reg_device); > > g_assert_cmpint(data & DEV, ==, 0); > > > > - data = inb(IDE_BASE + reg_status); > > + do { > > + data = inb(IDE_BASE + reg_status); > > + } while (data & BSY); > > Is a busy loop really a good idea for a qtest? CC'ing Anthony. I'm not sure, my main justification was simply that we currently do it in ide-test for send_dma_request() > For the theoretical case that BSY is not cleared it might be better to > terminate the loop with some timeout to get an assertion failure or at > least use some form of sleep() to yield the thread while waiting? > I don't think yielding/sleeping is too big a deal since we do a blocking read() on each iteration which i assume will result in a yield while the inb is being processed by qemu. Timeouts might be nice thing to add later though. What about something like this? inb_wait(addr, testfn, opaque, timeout) { while (timeout == -1 || timeout-- > 0) { val = inb(addr); if (testfn(val, opaque)) { return val; } usleep(1000); } abort() } It's not particularly precise, but for imposing a rough limit it should work. > > + > > assert_bit_set(data, DRDY); > > assert_bit_clear(data, BSY | DF | ERR | DRQ); > > This BSY clear assertion will always be true now due to the above while > condition; it won't if we change it though. > > Regards, > Andreas > > > > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >