All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] Main loop
@ 2009-09-26 23:55 malc
  2009-09-27  0:49 ` Anthony Liguori
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: malc @ 2009-09-26 23:55 UTC (permalink / raw)
  To: qemu-devel


At http://repo.or.cz/w/qemu/malc.git?a=shortlog;h=refs/heads/mtloop you
can find the branch which refactors main execution loop somewhat, effects
include:

a. Host alarm timers are gone
b. IO thread is replaced (now Windows is supported too)

I have no means of testing the KVM/Xen bits (both are likely to be broken
by this), and since testing was only done on Linux/X86-64[1],PPC and 
Windows/i386 chances are good that something might be not so great in
BSD/Solairs/MacOS X lands.

Apart from obvious KVM bits, other things were not implemented (yet) 
either: proper VM stop/resume, GDB, etc.

Diffstat relative to c2b023b62707f5dc73497dc03f3764f145a29785 (plus
one somewhat unrelated local commit) is bellow.

 Makefile        |    1 -
 configure       |    8 -
 cpu-defs.h      |    2 -
 cpu-exec.c      |    3 +
 hw/xenfb.c      |    2 +-
 qemu-options.hx |    9 -
 qemu-thread.c   |  163 ------
 qemu-thread.h   |   40 --
 sysemu.h        |    2 +-
 vl.c            | 1650 ++++++++++++++++++++-----------------------------------
 10 files changed, 593 insertions(+), 1287 deletions(-)

Comments?

[1] Linux/X86-64 was tested yesterday in the meantime the code was changed
    and might have become less stable, not sure.

-- 
mailto:av1474@comtv.ru

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] Main loop
  2009-09-26 23:55 [Qemu-devel] Main loop malc
@ 2009-09-27  0:49 ` Anthony Liguori
  2009-09-27 10:55   ` malc
  2009-09-27 14:23 ` Blue Swirl
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 16+ messages in thread
From: Anthony Liguori @ 2009-09-27  0:49 UTC (permalink / raw)
  To: malc; +Cc: qemu-devel

malc wrote:
> At http://repo.or.cz/w/qemu/malc.git?a=shortlog;h=refs/heads/mtloop you
> can find the branch which refactors main execution loop somewhat, effects
> include:
>
> a. Host alarm timers are gone
> b. IO thread is replaced (now Windows is supported too)
>   

What was the motivation?

Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] Main loop
  2009-09-27  0:49 ` Anthony Liguori
@ 2009-09-27 10:55   ` malc
  2009-09-27 14:05     ` Anthony Liguori
  2009-09-27 14:31     ` malc
  0 siblings, 2 replies; 16+ messages in thread
From: malc @ 2009-09-27 10:55 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: qemu-devel

On Sat, 26 Sep 2009, Anthony Liguori wrote:

> malc wrote:
> > At http://repo.or.cz/w/qemu/malc.git?a=shortlog;h=refs/heads/mtloop you
> > can find the branch which refactors main execution loop somewhat, effects
> > include:
> > 
> > a. Host alarm timers are gone
> > b. IO thread is replaced (now Windows is supported too)
> >   
> 
> What was the motivation?

To have working audio on my 250 Hz system, where audio polling mode
ensures that wakeups happen at ~1Khz (with sufficiently small fragment
size) thus allowing Fast Tracker 2 to run properly. IOW this approach
allows to react to FD activity indpendantly of any timers.

Things i forgot to mention, slirp crawls (i.e. when sending some file
with nc to the guest the (hidden[1]) cpu utilization is only at ~50%
and subsequently it takes (relative to alarm mode) forever for transfer
to finnish, FWIW same problem plagues IO Thread)

This is the 5 or 6th internal iteration of the thing, at one point i had
a system working where tcg only held the locks during the IO/helpers, but
in the end complexity wasn't worth it.

-- 
mailto:av1474@comtv.ru

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] Main loop
  2009-09-27 10:55   ` malc
@ 2009-09-27 14:05     ` Anthony Liguori
  2009-09-27 14:39       ` malc
  2009-09-27 14:31     ` malc
  1 sibling, 1 reply; 16+ messages in thread
From: Anthony Liguori @ 2009-09-27 14:05 UTC (permalink / raw)
  To: malc; +Cc: Glauber Costa, qemu-devel

malc wrote:
> On Sat, 26 Sep 2009, Anthony Liguori wrote:
>
>   
>> malc wrote:
>>     
>>> At http://repo.or.cz/w/qemu/malc.git?a=shortlog;h=refs/heads/mtloop you
>>> can find the branch which refactors main execution loop somewhat, effects
>>> include:
>>>
>>> a. Host alarm timers are gone
>>> b. IO thread is replaced (now Windows is supported too)
>>>   
>>>       
>> What was the motivation?
>>     
>
> To have working audio on my 250 Hz system, where audio polling mode
> ensures that wakeups happen at ~1Khz (with sufficiently small fragment
> size) thus allowing Fast Tracker 2 to run properly. IOW this approach
> allows to react to FD activity indpendantly of any timers.
>   

Ah, yes.  This is also addressed by the IO thread FWIW.  The IO thread 
can run select without holding a mutex and can send a signal to the TCG 
thread in order to break it's execution out before attempting to acquire 
the global mutex.

> Things i forgot to mention, slirp crawls (i.e. when sending some file
> with nc to the guest the (hidden[1]) cpu utilization is only at ~50%
> and subsequently it takes (relative to alarm mode) forever for transfer
> to finnish, FWIW same problem plagues IO Thread)
>   

The slirp issue is that it has a hook in the polling loop.  The IO 
thread doesn't suffer from this problem because slirp is driven in the 
IO thread itself which still receives regular signals.

FWIW, I consider the fact that slirp (or anything else) requires 
periodic polling to be a bug.

> This is the 5 or 6th internal iteration of the thing, at one point i had
> a system working where tcg only held the locks during the IO/helpers, but
> in the end complexity wasn't worth it.
>   

Glauber spent some time with the IO thread recently.  Any reason you 
didn't start with the existing IO thread (besides the fact that it 
doesn't work with TCG)?

Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] Main loop
  2009-09-26 23:55 [Qemu-devel] Main loop malc
  2009-09-27  0:49 ` Anthony Liguori
@ 2009-09-27 14:23 ` Blue Swirl
  2009-09-27 14:35   ` malc
  2009-09-27 17:43 ` malc
       [not found] ` <m3fxa7jug0.fsf@neno.mitica>
  3 siblings, 1 reply; 16+ messages in thread
From: Blue Swirl @ 2009-09-27 14:23 UTC (permalink / raw)
  To: malc; +Cc: qemu-devel

On Sun, Sep 27, 2009 at 2:55 AM, malc <av1474@comtv.ru> wrote:
>
> At http://repo.or.cz/w/qemu/malc.git?a=shortlog;h=refs/heads/mtloop you
> can find the branch which refactors main execution loop somewhat, effects
> include:
>
> a. Host alarm timers are gone
> b. IO thread is replaced (now Windows is supported too)
>
> I have no means of testing the KVM/Xen bits (both are likely to be broken
> by this), and since testing was only done on Linux/X86-64[1],PPC and
> Windows/i386 chances are good that something might be not so great in
> BSD/Solairs/MacOS X lands.
>
> Apart from obvious KVM bits, other things were not implemented (yet)
> either: proper VM stop/resume, GDB, etc.
>
> Diffstat relative to c2b023b62707f5dc73497dc03f3764f145a29785 (plus
> one somewhat unrelated local commit) is bellow.
>
>  Makefile        |    1 -
>  configure       |    8 -
>  cpu-defs.h      |    2 -
>  cpu-exec.c      |    3 +
>  hw/xenfb.c      |    2 +-
>  qemu-options.hx |    9 -
>  qemu-thread.c   |  163 ------
>  qemu-thread.h   |   40 --
>  sysemu.h        |    2 +-
>  vl.c            | 1650 ++++++++++++++++++++-----------------------------------
>  10 files changed, 593 insertions(+), 1287 deletions(-)
>
> Comments?

+extern void ontb (CPUState *env);
+

Please try to avoid prototypes outside header files. The name does not
tell what the function does.

     if (delta < MIN_TIMER_REARM_US)
         delta = MIN_TIMER_REARM_US;
+    if (delta > MIN_TIMER_REARM_US)
+        delta = MIN_TIMER_REARM_US;

The above four lines are equal to
     delta = MIN_TIMER_REARM_US;

Maybe the latter two should use MAX_TIMER_REARM_US?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] Main loop
  2009-09-27 10:55   ` malc
  2009-09-27 14:05     ` Anthony Liguori
@ 2009-09-27 14:31     ` malc
  1 sibling, 0 replies; 16+ messages in thread
From: malc @ 2009-09-27 14:31 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: qemu-devel

On Sun, 27 Sep 2009, malc wrote:

> On Sat, 26 Sep 2009, Anthony Liguori wrote:
> 
> > malc wrote:
> > > At http://repo.or.cz/w/qemu/malc.git?a=shortlog;h=refs/heads/mtloop you
> > > can find the branch which refactors main execution loop somewhat, effects
> > > include:
> > > 
> > > a. Host alarm timers are gone
> > > b. IO thread is replaced (now Windows is supported too)
> > >   
> > 
> > What was the motivation?
> 
> To have working audio on my 250 Hz system, where audio polling mode
> ensures that wakeups happen at ~1Khz (with sufficiently small fragment
> size) thus allowing Fast Tracker 2 to run properly. IOW this approach
> allows to react to FD activity indpendantly of any timers.
> 
> Things i forgot to mention, slirp crawls (i.e. when sending some file
> with nc to the guest the (hidden[1]) cpu utilization is only at ~50%
> and subsequently it takes (relative to alarm mode) forever for transfer
> to finnish, FWIW same problem plagues IO Thread)
> 
> This is the 5 or 6th internal iteration of the thing, at one point i had
> a system working where tcg only held the locks during the IO/helpers, but
> in the end complexity wasn't worth it.

Forgot shameless plug

[1] http://repo.or.cz/w/apc.git?a=blob;f=README;h=8c27c13e6b047e3d4a29a61fccdbc2fc43205714;hb=d8cd505b304b368d5f68584193ff2088c10abf55

-- 
mailto:av1474@comtv.ru

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] Main loop
  2009-09-27 14:23 ` Blue Swirl
@ 2009-09-27 14:35   ` malc
  0 siblings, 0 replies; 16+ messages in thread
From: malc @ 2009-09-27 14:35 UTC (permalink / raw)
  To: Blue Swirl; +Cc: qemu-devel

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2106 bytes --]

On Sun, 27 Sep 2009, Blue Swirl wrote:

> On Sun, Sep 27, 2009 at 2:55 AM, malc <av1474@comtv.ru> wrote:
> >
> > At http://repo.or.cz/w/qemu/malc.git?a=shortlog;h=refs/heads/mtloop you
> > can find the branch which refactors main execution loop somewhat, effects
> > include:
> >
> > a. Host alarm timers are gone
> > b. IO thread is replaced (now Windows is supported too)
> >
> > I have no means of testing the KVM/Xen bits (both are likely to be broken
> > by this), and since testing was only done on Linux/X86-64[1],PPC and
> > Windows/i386 chances are good that something might be not so great in
> > BSD/Solairs/MacOS X lands.
> >
> > Apart from obvious KVM bits, other things were not implemented (yet)
> > either: proper VM stop/resume, GDB, etc.
> >
> > Diffstat relative to c2b023b62707f5dc73497dc03f3764f145a29785 (plus
> > one somewhat unrelated local commit) is bellow.
> >
> >  Makefile        |    1 -
> >  configure       |    8 -
> >  cpu-defs.h      |    2 -
> >  cpu-exec.c      |    3 +
> >  hw/xenfb.c      |    2 +-
> >  qemu-options.hx |    9 -
> >  qemu-thread.c   |  163 ------
> >  qemu-thread.h   |   40 --
> >  sysemu.h        |    2 +-
> >  vl.c            | 1650 ++++++++++++++++++++-----------------------------------
> >  10 files changed, 593 insertions(+), 1287 deletions(-)
> >
> > Comments?
> 
> +extern void ontb (CPUState *env);
> +

It's for local debugging only.

> 
> Please try to avoid prototypes outside header files. The name does not
> tell what the function does.
> 
>      if (delta < MIN_TIMER_REARM_US)
>          delta = MIN_TIMER_REARM_US;
> +    if (delta > MIN_TIMER_REARM_US)
> +        delta = MIN_TIMER_REARM_US;
> 
> The above four lines are equal to
>      delta = MIN_TIMER_REARM_US;
> 
> Maybe the latter two should use MAX_TIMER_REARM_US?
> 

And this is a leftover from local debugging which is already gone:
http://repo.or.cz/w/qemu/malc.git?a=commit;h=f48b6952e1f61ba2fffc1a5bbe06db942b1c0c72
Was trying to find out why -smp N boot times were so slow.

-- 
mailto:av1474@comtv.ru

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] Main loop
  2009-09-27 14:05     ` Anthony Liguori
@ 2009-09-27 14:39       ` malc
  2009-09-28 13:57         ` Glauber Costa
  0 siblings, 1 reply; 16+ messages in thread
From: malc @ 2009-09-27 14:39 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: Glauber Costa, qemu-devel

On Sun, 27 Sep 2009, Anthony Liguori wrote:

> malc wrote:
> > On Sat, 26 Sep 2009, Anthony Liguori wrote:
> > 
> >   
> > > malc wrote:
> > >     
> > > > At http://repo.or.cz/w/qemu/malc.git?a=shortlog;h=refs/heads/mtloop you
> > > > can find the branch which refactors main execution loop somewhat,
> > > > effects
> > > > include:
> > > > 
> > > > a. Host alarm timers are gone
> > > > b. IO thread is replaced (now Windows is supported too)
> > > >         
> > > What was the motivation?
> > >     
> > 
> > To have working audio on my 250 Hz system, where audio polling mode
> > ensures that wakeups happen at ~1Khz (with sufficiently small fragment
> > size) thus allowing Fast Tracker 2 to run properly. IOW this approach
> > allows to react to FD activity indpendantly of any timers.
> >   
> 
> Ah, yes.  This is also addressed by the IO thread FWIW.  The IO thread can run
> select without holding a mutex and can send a signal to the TCG thread in
> order to break it's execution out before attempting to acquire the global
> mutex.

Huh? I said the same issue _applies_ to IO Thread, you are contradicting
it, how come?

> 
> > Things i forgot to mention, slirp crawls (i.e. when sending some file
> > with nc to the guest the (hidden[1]) cpu utilization is only at ~50%
> > and subsequently it takes (relative to alarm mode) forever for transfer
> > to finnish, FWIW same problem plagues IO Thread)
> >   
> 
> The slirp issue is that it has a hook in the polling loop.  The IO thread
> doesn't suffer from this problem because slirp is driven in the IO thread
> itself which still receives regular signals.
> 
> FWIW, I consider the fact that slirp (or anything else) requires periodic
> polling to be a bug.
> 
> > This is the 5 or 6th internal iteration of the thing, at one point i had
> > a system working where tcg only held the locks during the IO/helpers, but
> > in the end complexity wasn't worth it.
> >   
> 
> Glauber spent some time with the IO thread recently.  Any reason you didn't
> start with the existing IO thread (besides the fact that it doesn't work with
> TCG)?
> 

Because i wasn't writing a replacement for IO Thread to begin with (btw it
does work with TCG), what it doesn't do is play nicely with icount[1], 
work on Windows, and for mysterious reasons still requires alarm timers, 
it also deadlocks for me when killing QEMU windows while running smp 
guest, but that's easily corrected mistake somewhere i guess.

[1] Current mtloop has an issue with icount also, halt related.

-- 
mailto:av1474@comtv.ru

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] Main loop
  2009-09-26 23:55 [Qemu-devel] Main loop malc
  2009-09-27  0:49 ` Anthony Liguori
  2009-09-27 14:23 ` Blue Swirl
@ 2009-09-27 17:43 ` malc
       [not found] ` <m3fxa7jug0.fsf@neno.mitica>
  3 siblings, 0 replies; 16+ messages in thread
From: malc @ 2009-09-27 17:43 UTC (permalink / raw)
  To: qemu-devel

On Sun, 27 Sep 2009, malc wrote:

> 
> At http://repo.or.cz/w/qemu/malc.git?a=shortlog;h=refs/heads/mtloop you
> can find the branch which refactors main execution loop somewhat, effects
> include:
> 
> a. Host alarm timers are gone
> b. IO thread is replaced (now Windows is supported too)
> 

[..snip..]

And just to make things more interesting:

$ git show

commit 309c5a3c4c2d234c1de8e0fb9441c8754590b1ce
Author: malc <av1474@comtv.ru>
Date:   Sun Sep 27 20:53:52 2009 +0400

    Add trace points for debian-503-i386-netinst.iso
    
    tracing begins _after_ selecting Install menu entry,
    results:
    
    c2b023b62707f5dc73497    this one (not sure what the has would be)
    ----------------------   -----------------------------------------
    took 9.810747 seconds    took 9.352371 seconds
    took 17.470818 seconds   took 15.257145 seconds
    took 36.917233 seconds   took 31.720386 seconds
    took 39.437925 seconds   took 33.894850 seconds
    took 56.677207 seconds   took 48.580790 seconds
    took 71.053026 seconds   took 59.385500 seconds
    took 89.334869 seconds   took 71.016816 seconds
    took 102.079938 seconds  took 84.707160 seconds
    took 108.837361 seconds  took 93.934140 seconds
    took 114.551840 seconds  took 98.579744 seconds
    took 120.375490 seconds  took 103.829356 seconds
    took 126.530942 seconds  took 109.003527 seconds

diff --git a/vl.c b/vl.c
index 34b8e72..c507fec 100644
--- a/vl.c
+++ b/vl.c
@@ -3142,12 +3142,17 @@ void onpc (target_ulong pc)
     static int done;
     static double a;
 
+    if (pc == 0xc036405a) {
+        a = now ();
+        done = 1;
+        return;
+    }
     if (!done) {
         done = 1;
         a = now ();
         return;
     }
-    if (pc == 0x08048054) {
+    if (pc == 0x08048054 || pc == 0x0804c00c) {
         printf ("took %f seconds\n", now () - a);
     }
 }
@@ -3516,9 +3521,11 @@ static void *exec_thread_loop (void __attribute__ ((unused)) *unused)
                 break;
 
             if (mt.wait_for_cont) {
+#if 0
                 static struct stats s = { .name = "cont", .limit = 1000 };
 
                 smark (&s);
+#endif
                 if (sem_wait (&mt.cont_sem)) {
                     die (errno, "exec_thread_loop: sem_wait/cont");
                 }

[end]

IO Thread results are:

took 9.955913 seconds
took 17.407633 seconds
took 38.311728 seconds
took 40.854557 seconds
took 58.371600 seconds
took 70.021890 seconds
took 85.377891 seconds
took 101.429223 seconds
took 111.663570 seconds
took 116.943185 seconds
took 122.880269 seconds
took 129.600638 seconds

IOW slightly worse than non IO Thread.

Caveat emptor:
  a. those are benchmarks
  b. i tend to fuck up things on occasion (frequently)

So take with a grain of salt.

P.S. (not sure what the has would be) should read (not sure what the
     _hash_ would be)

-- 
mailto:av1474@comtv.ru

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] Re: Main loop
       [not found] ` <m3fxa7jug0.fsf@neno.mitica>
@ 2009-09-28  9:42   ` malc
       [not found]     ` <m3pr9bidy9.fsf@neno.mitica>
  0 siblings, 1 reply; 16+ messages in thread
From: malc @ 2009-09-28  9:42 UTC (permalink / raw)
  To: Juan Quintela; +Cc: qemu-devel

On Mon, 28 Sep 2009, Juan Quintela wrote:

> malc <av1474@comtv.ru> wrote:
> > At http://repo.or.cz/w/qemu/malc.git?a=shortlog;h=refs/heads/mtloop you
> > can find the branch which refactors main execution loop somewhat, effects
> > include:
> >
> > a. Host alarm timers are gone
> > b. IO thread is replaced (now Windows is supported too)
> >
> > I have no means of testing the KVM/Xen bits (both are likely to be broken
> > by this), and since testing was only done on Linux/X86-64[1],PPC and 
> > Windows/i386 chances are good that something might be not so great in
> > BSD/Solairs/MacOS X lands.
> 
> No cookie:
> It don't work in kvm mode.

Didn't i say that i have no means of testing it? Even x86 box yields:

~$ uname -a
Linux laptop 2.6.31 #7 SMP PREEMPT Sun Sep 13 10:59:44 MSD 2009 x86_64 AMD 
Athlon(tm)64 X2 Dual Core Processor  3800+ AuthenticAMD GNU/Linux
~$ egrep '^flags.*(vmx|svm)' /proc/cpuinfo 
~$

Help is needed with that.

> root@neno ~]# /scratch/qemu/x86_64-softmmu/qemu-system-x86_64 -M pc -m 512 -smp 1 -name test -drive file=/scratch/tmp/f11b.img -net nic,macaddr=54:52:00:53:7e:5b,vlan=0,model=virtio -net tap,script=/etc/kvm-ifup,vlan=0,ifname=vnet1,downscript=no --monitor stdio --serial telnet:0:5553,server --virtioconsole telnet:0:5554,server --enable-kvm -usbdevice tablet -vnc :1
> QEMU waiting for connection on: telnet:0.0.0.0:5553,server
> QEMU waiting for connection on: telnet:0.0.0.0:5554,server
> QEMU 0.11.50 monitor - type 'help' for more information
> (qemu) exec_thread_loop: sem_wait/halt: Interrupted system call
> Aborted
> 
> It is not able to finish boot.

And yet you still enable kvm.. sigh..

> 
> Doing a loadvm of a guest, just load the data, it don't run at all.
> 
> > Apart from obvious KVM bits, other things were not implemented (yet) 
> > either: proper VM stop/resume, GDB, etc.

And somehow the above failed to impress and suggest that loadvm wouldn't work?

[..snip..]

-- 
mailto:av1474@comtv.ru

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] Re: Main loop
       [not found]     ` <m3pr9bidy9.fsf@neno.mitica>
@ 2009-09-28 10:19       ` malc
  0 siblings, 0 replies; 16+ messages in thread
From: malc @ 2009-09-28 10:19 UTC (permalink / raw)
  To: Juan Quintela; +Cc: qemu-devel

On Mon, 28 Sep 2009, Juan Quintela wrote:

> malc <av1474@comtv.ru> wrote:
> > On Mon, 28 Sep 2009, Juan Quintela wrote:
> >
> >> malc <av1474@comtv.ru> wrote:
> >> > At http://repo.or.cz/w/qemu/malc.git?a=shortlog;h=refs/heads/mtloop you
> >> > can find the branch which refactors main execution loop somewhat, effects
> >> > include:
> >> >
> >> > a. Host alarm timers are gone
> >> > b. IO thread is replaced (now Windows is supported too)
> >> >
> >> > I have no means of testing the KVM/Xen bits (both are likely to be broken
> >> > by this), and since testing was only done on Linux/X86-64[1],PPC and 
> >> > Windows/i386 chances are good that something might be not so great in
> >> > BSD/Solairs/MacOS X lands.
> >> 
> >> No cookie:
> >> It don't work in kvm mode.
> 
> > Didn't i say that i have no means of testing it? Even x86 box yields:
> 
> I read it as that you wanted people to test it :p

I must have expressed myself poorly, what was meant:
KVM/Xen are quite lilely broken (truth)
I have no means of testing _and_ fixing it
Testing _without_ those is nice, fixing those is better still

> 
> > ~$ uname -a
> > Linux laptop 2.6.31 #7 SMP PREEMPT Sun Sep 13 10:59:44 MSD 2009 x86_64 AMD 
> > Athlon(tm)64 X2 Dual Core Processor  3800+ AuthenticAMD GNU/Linux
> > ~$ egrep '^flags.*(vmx|svm)' /proc/cpuinfo 
> > ~$
> >
> > Help is needed with that.
> 
> on the "I like side" of the patch:
> - timers (clocks or whatever) are gone!!!!!
> - differences beteween io_thread/no io_thread also gone
> - pthread wrappers are gone
> on the "I don't like"
> - calling "minor refactoring" to a single patch that remove all the
>   clocks, remove io thread, remove pthread wrappers, and some other
>   things in one go

grumble, grubmble

-- 
mailto:av1474@comtv.ru

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] Main loop
  2009-09-27 14:39       ` malc
@ 2009-09-28 13:57         ` Glauber Costa
  2009-09-28 18:50           ` malc
  2009-09-28 23:57           ` malc
  0 siblings, 2 replies; 16+ messages in thread
From: Glauber Costa @ 2009-09-28 13:57 UTC (permalink / raw)
  To: malc; +Cc: qemu-devel

> > Glauber spent some time with the IO thread recently.  Any reason you didn't
> > start with the existing IO thread (besides the fact that it doesn't work with
> > TCG)?
> > 
> 
> Because i wasn't writing a replacement for IO Thread to begin with (btw it
> does work with TCG), what it doesn't do is play nicely with icount[1], 
> work on Windows, and for mysterious reasons still requires alarm timers, 
> it also deadlocks for me when killing QEMU windows while running smp 
> guest, but that's easily corrected mistake somewhere i guess.

You wasn't, but you ended up doing so, since it is unlikely that they will both
live in the end. I am still reading your patch, and I still have to struggle a little
bit to understand it. Can you please tell us why do you think your approach is better?

The current I/O thread currently works with both kvm and tcg. For KVM, I am quite close
(expecting to post patches today) that enables the in-kernel irqchip for it, and pretty
close to do smp.

Note that I don't think it requires alarm timers at all, by design. So, again, why should
we drop what we have in favour of your implementation?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] Main loop
  2009-09-28 13:57         ` Glauber Costa
@ 2009-09-28 18:50           ` malc
  2009-09-28 19:35             ` Anthony Liguori
  2009-09-28 23:57           ` malc
  1 sibling, 1 reply; 16+ messages in thread
From: malc @ 2009-09-28 18:50 UTC (permalink / raw)
  To: Glauber Costa; +Cc: qemu-devel

On Mon, 28 Sep 2009, Glauber Costa wrote:

> > > Glauber spent some time with the IO thread recently.  Any reason you didn't
> > > start with the existing IO thread (besides the fact that it doesn't work with
> > > TCG)?
> > > 
> > 
> > Because i wasn't writing a replacement for IO Thread to begin with (btw it
> > does work with TCG), what it doesn't do is play nicely with icount[1], 
> > work on Windows, and for mysterious reasons still requires alarm timers, 
> > it also deadlocks for me when killing QEMU windows while running smp 
> > guest, but that's easily corrected mistake somewhere i guess.
> 
> You wasn't, but you ended up doing so, since it is unlikely that they 
> will both live in the end. I am still reading your patch, and I still 
> have to struggle a little bit to understand it. Can you please tell us 
> why do you think your approach is better?
> 
> The current I/O thread currently works with both kvm and tcg. For KVM, I 
> am quite close (expecting to post patches today) that enables the 
> in-kernel irqchip for it, and pretty close to do smp.
> 
> Note that I don't think it requires alarm timers at all, by design. So, 
> again, why should we drop what we have in favour of your implementation?

Now that we have talked i see the problem and it basically boils down
to this: kvm can(and does) run multiple vcpus in multiple threads,
qemu always uses one, on top of this you are mainly interested in KVM
and i'm _only_ interested in TCG. The way i see it the best approach
would be to factor out main loop into separate file and let QEMU and
KVM go their own separate ways w.r.t. this new entity.

-- 
mailto:av1474@comtv.ru

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] Main loop
  2009-09-28 18:50           ` malc
@ 2009-09-28 19:35             ` Anthony Liguori
  2009-09-28 21:21               ` Glauber Costa
  0 siblings, 1 reply; 16+ messages in thread
From: Anthony Liguori @ 2009-09-28 19:35 UTC (permalink / raw)
  To: malc; +Cc: Glauber Costa, qemu-devel

malc wrote:
> Now that we have talked i see the problem and it basically boils down
> to this: kvm can(and does) run multiple vcpus in multiple threads,
> qemu always uses one, on top of this you are mainly interested in KVM
> and i'm _only_ interested in TCG.

The multiple vcpus in multiple threads model is the right one for TCG 
too.  Both Paul and Fabrice have expressed interest in this in the 
past.  For certain architecture combinations, it may not be that bad to 
implement either.

>  The way i see it the best approach
> would be to factor out main loop into separate file and let QEMU and
> KVM go their own separate ways w.r.t. this new entity.
>   
OTOH, if most of the heavy lifting (like IO dispatch) can be refactored 
to shared functions, two main loops may not be so bad.


Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] Main loop
  2009-09-28 19:35             ` Anthony Liguori
@ 2009-09-28 21:21               ` Glauber Costa
  0 siblings, 0 replies; 16+ messages in thread
From: Glauber Costa @ 2009-09-28 21:21 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: qemu-devel

On Mon, Sep 28, 2009 at 02:35:24PM -0500, Anthony Liguori wrote:
> malc wrote:
>> Now that we have talked i see the problem and it basically boils down
>> to this: kvm can(and does) run multiple vcpus in multiple threads,
>> qemu always uses one, on top of this you are mainly interested in KVM
>> and i'm _only_ interested in TCG.
>
> The multiple vcpus in multiple threads model is the right one for TCG  
> too.  Both Paul and Fabrice have expressed interest in this in the past.  
> For certain architecture combinations, it may not be that bad to  
> implement either.
>
>>  The way i see it the best approach
>> would be to factor out main loop into separate file and let QEMU and
>> KVM go their own separate ways w.r.t. this new entity.
>>   
> OTOH, if most of the heavy lifting (like IO dispatch) can be refactored  
> to shared functions, two main loops may not be so bad.
I am not opposed to it either.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] Main loop
  2009-09-28 13:57         ` Glauber Costa
  2009-09-28 18:50           ` malc
@ 2009-09-28 23:57           ` malc
  1 sibling, 0 replies; 16+ messages in thread
From: malc @ 2009-09-28 23:57 UTC (permalink / raw)
  To: Glauber Costa; +Cc: qemu-devel

On Mon, 28 Sep 2009, Glauber Costa wrote:

> > > Glauber spent some time with the IO thread recently.  Any reason you didn't
> > > start with the existing IO thread (besides the fact that it doesn't work with
> > > TCG)?
> > > 
> > 
> > Because i wasn't writing a replacement for IO Thread to begin with (btw it
> > does work with TCG), what it doesn't do is play nicely with icount[1], 
> > work on Windows, and for mysterious reasons still requires alarm timers, 
> > it also deadlocks for me when killing QEMU windows while running smp 
> > guest, but that's easily corrected mistake somewhere i guess.
> 

[..snip..]

> 
> Note that I don't think it requires alarm timers at all, by design. So, 
> again, why should we drop what we have in favour of your implementation?

It doesn't require them, yet no one has come up even with the
following crude patch which brings the time of booting debian netinst
closer to mtloop. That said the real reason why iothread should be
dropped is that it's basically a big pile of shit, not only alarm
timers weren't dropped the code uses pthread_cond_timedwait all over
the place for NO good reason without ever checking the result thus
wasting time, 2 seconds of the boot time were chopped of just by
replacing it with palin wait, with such an attention to detail i
wonder what other monsters are lurking there.

diff --git a/vl.c b/vl.c
index c6c6a6b..fb04b25 100644
--- a/vl.c
+++ b/vl.c
@@ -718,6 +718,14 @@ static void qemu_rearm_alarm_timer(struct 
qemu_alarm_timer *t)
 
 static struct qemu_alarm_timer *alarm_timer;
 
+static int notimer_start_timer(struct qemu_alarm_timer *t)
+{
+    return 0;
+}
+static void notimer_stop_timer(struct qemu_alarm_timer *t)
+{
+}
+
 #ifdef _WIN32
 
 struct qemu_alarm_win32 {
@@ -816,6 +824,9 @@ static void init_icount_adjust(void)
 }
 
 static struct qemu_alarm_timer alarm_timers[] = {
+#ifdef CONFIG_IOTHREAD
+    {"notimer", 0, notimer_start_timer, notimer_stop_timer, NULL, NULL},
+#endif
 #ifndef _WIN32
 #ifdef __linux__
     {"dynticks", ALARM_FLAG_DYNTICKS, dynticks_start_timer,
@@ -1167,7 +1178,7 @@ static int64_t qemu_next_deadline(void)
     return delta;
 }
 
-#if defined(__linux__) || defined(_WIN32)
+#if defined(__linux__) || defined(_WIN32) || defined(CONFIG_IOTHREAD)
 static uint64_t qemu_next_deadline_dyntick(void)
 {
     int64_t delta;
@@ -4199,7 +4210,11 @@ static int qemu_calculate_timeout(void)
 
     return timeout;
 #else /* CONFIG_IOTHREAD */
-    return 1000;
+    if (alarm_timer->start == notimer_start_timer) {
+        return qemu_next_deadline_dyntick() / 1000;
+    } else {
+        return 1000;
+    }
 #endif
 }
 

-- 
mailto:av1474@comtv.ru

^ permalink raw reply related	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2009-09-28 23:57 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-26 23:55 [Qemu-devel] Main loop malc
2009-09-27  0:49 ` Anthony Liguori
2009-09-27 10:55   ` malc
2009-09-27 14:05     ` Anthony Liguori
2009-09-27 14:39       ` malc
2009-09-28 13:57         ` Glauber Costa
2009-09-28 18:50           ` malc
2009-09-28 19:35             ` Anthony Liguori
2009-09-28 21:21               ` Glauber Costa
2009-09-28 23:57           ` malc
2009-09-27 14:31     ` malc
2009-09-27 14:23 ` Blue Swirl
2009-09-27 14:35   ` malc
2009-09-27 17:43 ` malc
     [not found] ` <m3fxa7jug0.fsf@neno.mitica>
2009-09-28  9:42   ` [Qemu-devel] " malc
     [not found]     ` <m3pr9bidy9.fsf@neno.mitica>
2009-09-28 10:19       ` malc

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.