[Qemu-devel] [PATCH] qemu: generate signals on tap I/O

[Qemu-devel] [PATCH] qemu: generate signals on tap I/O

[Aurelien Jarno]

The patch below from KVM improves network transfers in a huge way. wget
in a MIPS target now gives me a transfer speed of up to 120 Mbits/s with
an e1000 emulated card.


    kvm: qemu: generate signals on tap I/O

    currently tap does not generate signals on I/O; this causes
    network latency to be dependent on the timer tick (1ms without
    dyntick, guest dependent with dyntick).  by generating a signal
    on I/O, we can inform the guest immediately that a packet has
    arrived.

    Signed-off-by: Avi Kivity <avi@qumranet.com>

---
 vl.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/vl.c b/vl.c
index c87e8bc..fb01a46 100644
--- a/vl.c
+++ b/vl.c
@@ -3939,6 +3939,7 @@ static TAPState *net_tap_fd_init(VLANState *vlan, int fd)
     if (!s)
         return NULL;
     s->fd = fd;
+    enable_sigio_timer(fd);
     s->vc = qemu_new_vlan_client(vlan, tap_receive, NULL, s);
     qemu_set_fd_handler(s->fd, tap_send, NULL, s);
     snprintf(s->vc->info_str, sizeof(s->vc->info_str), "tap: fd=%d", fd);

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] qemu: generate signals on tap I/O

[Anders Melchiorsen]

Aurelien Jarno <aurelien@aurel32.net> writes:

> The patch below from KVM improves network transfers in a huge way. 
> wget in a MIPS target now gives me a transfer speed of up to 120
> Mbits/s with an e1000 emulated card.

Without I/O signals, qemu is relying on periodic timer events to poll
the I/O. That seems wrong, even though it works reasonably well
because timers are so frequent. In KVM, timers are less frequent, and
it does not work quite as well.

Here is a quick try at a more elaborate patch.

It attaches a signal to all[1] file descriptors that will be used in
select(). Also, it uses a dedicated SIGIO handler rather than
piggybacking on the alarm handler, so alarm I/O is changed to use
SIGALRM.

I copied the handler function from the alarm case, quite frankly I do
not quite understand what is going on. Also, I left _WIN32 out, since
I have no idea how signals work there.


[1] The slirp file descriptors are not included yet.


Anders.




diff --git a/vl.c b/vl.c
index c87e8bc..ff8ceef 100644
--- a/vl.c
+++ b/vl.c
@@ -1148,6 +1148,25 @@ static int timer_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
+#ifndef _WIN32
+static void host_io_handler(int host_signum)
+{
+    CPUState *env = next_cpu;
+
+    if (env) {
+        /* stop the currently executing cpu because io occured */
+        cpu_interrupt(env, CPU_INTERRUPT_EXIT);
+#ifdef USE_KQEMU
+        if (env->kqemu_enabled) {
+            kqemu_cpu_interrupt(env);
+        }
+#endif
+    }
+
+    event_pending = 1;
+}
+#endif
+
 #ifdef _WIN32
 void CALLBACK host_alarm_handler(UINT uTimerID, UINT uMsg,
                                  DWORD_PTR dwUser, DWORD_PTR dw1, DWORD_PTR dw2)
@@ -1240,7 +1259,20 @@ static uint64_t qemu_next_deadline(void)
 
 #define RTC_FREQ 1024
 
-static void enable_sigio_timer(int fd)
+static void enable_sigio(int fd)
+{
+    struct sigaction act;
+
+    sigfillset(&act.sa_mask);
+    act.sa_flags = 0;
+    act.sa_handler = host_io_handler;
+
+    sigaction(SIGIO, &act, NULL);
+    fcntl(fd, F_SETFL, O_ASYNC);
+    fcntl(fd, F_SETOWN, getpid());
+}
+
+static void enable_sigalrm(int fd)
 {
     struct sigaction act;
 
@@ -1249,8 +1281,9 @@ static void enable_sigio_timer(int fd)
     act.sa_flags = 0;
     act.sa_handler = host_alarm_handler;
 
-    sigaction(SIGIO, &act, NULL);
+    sigaction(SIGALRM, &act, NULL);
     fcntl(fd, F_SETFL, O_ASYNC);
+    fcntl(fd, F_SETSIG, SIGALRM);
     fcntl(fd, F_SETOWN, getpid());
 }
 
@@ -1287,7 +1320,7 @@ static int hpet_start_timer(struct qemu_alarm_timer *t)
     if (r < 0)
         goto fail;
 
-    enable_sigio_timer(fd);
+    enable_sigalrm(fd);
     t->priv = (void *)(long)fd;
 
     return 0;
@@ -1325,7 +1358,7 @@ static int rtc_start_timer(struct qemu_alarm_timer *t)
         return -1;
     }
 
-    enable_sigio_timer(rtc_fd);
+    enable_sigalrm(rtc_fd);
 
     t->priv = (void *)(long)rtc_fd;
 
@@ -5481,6 +5514,10 @@ int qemu_set_fd_handler2(int fd,
             return -1;
         ioh->next = first_io_handler;
         first_io_handler = ioh;
+#ifndef _WIN32
+        enable_sigio(fd);
+#endif
+
     found:
         ioh->fd = fd;
         ioh->fd_read_poll = fd_read_poll;

Re: [Qemu-devel] [PATCH] qemu: generate signals on tap I/O

[Aurelien Jarno]

Anders Melchiorsen a écrit :
> Aurelien Jarno <aurelien@aurel32.net> writes:
> 
>> The patch below from KVM improves network transfers in a huge way. 
>> wget in a MIPS target now gives me a transfer speed of up to 120
>> Mbits/s with an e1000 emulated card.
> 
> Without I/O signals, qemu is relying on periodic timer events to poll
> the I/O. That seems wrong, even though it works reasonably well
> because timers are so frequent. In KVM, timers are less frequent, and
> it does not work quite as well.
> 
> Here is a quick try at a more elaborate patch.
> 
> It attaches a signal to all[1] file descriptors that will be used in
> select(). Also, it uses a dedicated SIGIO handler rather than
> piggybacking on the alarm handler, so alarm I/O is changed to use
> SIGALRM.
> 
> I copied the handler function from the alarm case, quite frankly I do
> not quite understand what is going on. Also, I left _WIN32 out, since
> I have no idea how signals work there.
> 

I have just given a tried to your patch, which looks indeed more
complete. I have made some performance measurement
(qemu-system-mips64el, 2.6.24 5kc kernel), I am getting the same
performances as with my patches on the network side, and a very small
improvement (< 3%) on the hard disk performance, but it can also be
noise (though the measurements looked quite stable).

Anybody to commit this patch?

> 
> diff --git a/vl.c b/vl.c
> index c87e8bc..ff8ceef 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1148,6 +1148,25 @@ static int timer_load(QEMUFile *f, void *opaque, int version_id)
>      return 0;
>  }
>  
> +#ifndef _WIN32
> +static void host_io_handler(int host_signum)
> +{
> +    CPUState *env = next_cpu;
> +
> +    if (env) {
> +        /* stop the currently executing cpu because io occured */
> +        cpu_interrupt(env, CPU_INTERRUPT_EXIT);
> +#ifdef USE_KQEMU
> +        if (env->kqemu_enabled) {
> +            kqemu_cpu_interrupt(env);
> +        }
> +#endif
> +    }
> +
> +    event_pending = 1;
> +}
> +#endif
> +
>  #ifdef _WIN32
>  void CALLBACK host_alarm_handler(UINT uTimerID, UINT uMsg,
>                                   DWORD_PTR dwUser, DWORD_PTR dw1, DWORD_PTR dw2)
> @@ -1240,7 +1259,20 @@ static uint64_t qemu_next_deadline(void)
>  
>  #define RTC_FREQ 1024
>  
> -static void enable_sigio_timer(int fd)
> +static void enable_sigio(int fd)
> +{
> +    struct sigaction act;
> +
> +    sigfillset(&act.sa_mask);
> +    act.sa_flags = 0;
> +    act.sa_handler = host_io_handler;
> +
> +    sigaction(SIGIO, &act, NULL);
> +    fcntl(fd, F_SETFL, O_ASYNC);
> +    fcntl(fd, F_SETOWN, getpid());
> +}
> +
> +static void enable_sigalrm(int fd)
>  {
>      struct sigaction act;
>  
> @@ -1249,8 +1281,9 @@ static void enable_sigio_timer(int fd)
>      act.sa_flags = 0;
>      act.sa_handler = host_alarm_handler;
>  
> -    sigaction(SIGIO, &act, NULL);
> +    sigaction(SIGALRM, &act, NULL);
>      fcntl(fd, F_SETFL, O_ASYNC);
> +    fcntl(fd, F_SETSIG, SIGALRM);
>      fcntl(fd, F_SETOWN, getpid());
>  }
>  
> @@ -1287,7 +1320,7 @@ static int hpet_start_timer(struct qemu_alarm_timer *t)
>      if (r < 0)
>          goto fail;
>  
> -    enable_sigio_timer(fd);
> +    enable_sigalrm(fd);
>      t->priv = (void *)(long)fd;
>  
>      return 0;
> @@ -1325,7 +1358,7 @@ static int rtc_start_timer(struct qemu_alarm_timer *t)
>          return -1;
>      }
>  
> -    enable_sigio_timer(rtc_fd);
> +    enable_sigalrm(rtc_fd);
>  
>      t->priv = (void *)(long)rtc_fd;
>  
> @@ -5481,6 +5514,10 @@ int qemu_set_fd_handler2(int fd,
>              return -1;
>          ioh->next = first_io_handler;
>          first_io_handler = ioh;
> +#ifndef _WIN32
> +        enable_sigio(fd);
> +#endif
> +
>      found:
>          ioh->fd = fd;
>          ioh->fd_read_poll = fd_read_poll;
> 


-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net