diff mbox

Re: [PATCH 0/4] Improve -icount, fix it with iothread

Message ID 20110223162738.GD27880@edde.se.axis.com
State New
Headers show

Commit Message

Edgar E. Iglesias Feb. 23, 2011, 4:27 p.m. UTC 
On Wed, Feb 23, 2011 at 01:42:59PM +0100, Paolo Bonzini wrote:
> On 02/23/2011 12:08 PM, Edgar E. Iglesias wrote:
> >> >  No, this supersedes Marcelo's patch.  10-20% doesn't seem comparable to
> >> >  "looks like it deadlocked" anyway.  Also, Jan has ideas on how to remove
> >> >  the synchronization overhead in the main loop for TCG+iothread.
> > I see. I tried booting two of my MIPS and CRIS linux guests with iothread
> > and -icount 4. Without your patch, the boot crawls super slow. Your patch
> > gives a huge improvement. This was the "deadlock" scenario which I
> > mentioned in previous emails.
> >
> > Just to clarify the previous test where I saw slowdown with your patch:
> > A CRIS setup that has a CRIS and basically only two peripherals,
> > a timer block and a device (X) that computes stuff but delays the results
> > with a virtual timer. The guest CPU is 99% of the time just
> > busy-waiting for device X to get ready.
> >
> > This latter test runs in 3.7s with icount 4 and without iothread,
> > with or without your patch.

Sorry, typo here. I ran -icount 10, not 4.

> 
> Thanks for testing this.
> 
> > With icount 4 and iothread it runs in ~1m5s without your patch and
> > ~1m20s with your patch. That was the 20% slowdown I mentioned earlier.
> 
> Ok, so it is in both cases with iothread.  We go from 16x slowdown to 
> 19x on one testcase :) and "huge improvement" on another.  (Also, the 
> CRIS images on qemu.org simply hang for me without my patch and numeric 
> icount---and the watchdog triggers---so that's another factor in favor 
> of the patches).  I guess we can live with the slowdown for now, if 
> somebody else finds the patch okay.

I agree. It would be nice with someone else review aswell though. Jan?


> Do you have images for the slow test?

the 16x vs 19x slowdown testcase is unfortunately on a proprietary machine
which I cant release at the moment. I'll have to debug that case at
some point.

For the "almost deadlock" testcase, I think the CRIS image on the wiki is
OK. With the following patch, the watchdog can be disabled. Run with
icount & iothread and you'll see the crawling. With your patch you'll
see the the guest booting much faster than before. Still slower than
non iothread builds though. In particular if you compare icount 10, with
and without iothread.

Comments

Paolo Bonzini Feb. 23, 2011, 4:32 p.m. UTC | #1 
On 02/23/2011 05:27 PM, Edgar E. Iglesias wrote:
> For the "almost deadlock" testcase, I think the CRIS image on the
> wiki is OK.

Ah yes, I was understanding that the watchdog itself slowed down too and
didn't trigger.  So I was indeed seeing it (and not patient enough to 
see it boot).

> Still slower than non iothread builds though. In particular if you
> compare icount 10, with and without iothread.

Very good, thanks.  I'll look at it after this series is in, and Jan's 
optimization patch as well hopefully.

Paolo
diff mbox

Patch

diff --git a/hw/etraxfs_timer.c b/hw/etraxfs_timer.c
index 133741b..2223744 100644
--- a/hw/etraxfs_timer.c
+++ b/hw/etraxfs_timer.c
@@ -197,8 +197,8 @@  static void watchdog_hit(void *opaque)
         ptimer_run(t->ptimer_wd, 1);
         qemu_irq_raise(t->nmi);
     }
-    else
-        qemu_system_reset_request();
 
     t->wd_hits++;
 }