[Qemu-devel] Linux SIGIO handling changes

[Qemu-devel] Linux SIGIO handling changes

[Anders Melchiorsen]

I am resending this patch, split into two parts now.

This cleans up SIGIO handling to improve latency:
- SIGALRM for alarm timers
- enable SIGIO on qemu_set_fd_handler2()

The issue was found in KVM, where it is much more visible,
because there is no periodic timer. However, it has been
confirmed (by Aurelien Jarno) that even for qemu, this
approach "improves network transfers in a huge way".

Please apply, or give a firm rejection so I can stop resending.

Cheers,
Anders.

[Qemu-devel] [PATCH] Use SIGALRM even for timers delivered over a fd

[Anders Melchiorsen]

This ensures that all timers use the same signal, no
matter how the underlying timer is implemented. It also
frees up SIGIO for real I/O events.

The patch is only relevant for Linux, hpet and rtc timers.

Signed-off-by: Anders Melchiorsen <mail@flac.kalibalik.dk>

diff --git a/vl.c b/vl.c
index 78486cf..ad4f6ef 100644
--- a/vl.c
+++ b/vl.c
@@ -1241,7 +1241,7 @@ static uint64_t qemu_next_deadline(void)
 
 #define RTC_FREQ 1024
 
-static void enable_sigio_timer(int fd)
+static void enable_sigalrm(int fd)
 {
     struct sigaction act;
 
@@ -1250,8 +1250,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());
 }
 
@@ -1288,7 +1289,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;
@@ -1326,7 +1327,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;
 
-- 
1.5.2.5

[Qemu-devel] [PATCH] Use SIGIO in Linux host

[Anders Melchiorsen]

Network packets coming over TAP have a latency that is
dictated by the periodic timer. That can hurt performance.

This patch activates signals for fds that are being used
with select(), giving predictable latency.

Signed-off-by: Anders Melchiorsen <mail@flac.kalibalik.dk>

diff --git a/vl.c b/vl.c
index ad4f6ef..eb2a75e 100644
--- a/vl.c
+++ b/vl.c
@@ -1149,6 +1149,25 @@ static int timer_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
+#if defined(__linux__)
+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)
@@ -1241,6 +1260,19 @@ static uint64_t qemu_next_deadline(void)
 
 #define RTC_FREQ 1024
 
+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|O_NONBLOCK);
+    fcntl(fd, F_SETOWN, getpid());
+}
+
 static void enable_sigalrm(int fd)
 {
     struct sigaction act;
@@ -5530,6 +5562,10 @@ int qemu_set_fd_handler2(int fd,
             return -1;
         ioh->next = first_io_handler;
         first_io_handler = ioh;
+#if defined(__linux__)
+        enable_sigio(fd);
+#endif
+
     found:
         ioh->fd = fd;
         ioh->fd_read_poll = fd_read_poll;
-- 
1.5.2.5

Re: [Qemu-devel] Linux SIGIO handling changes

[Anthony Liguori]

Anders Melchiorsen wrote:
> I am resending this patch, split into two parts now.
>
> This cleans up SIGIO handling to improve latency:
> - SIGALRM for alarm timers
> - enable SIGIO on qemu_set_fd_handler2()
>
> The issue was found in KVM, where it is much more visible,
> because there is no periodic timer. However, it has been
> confirmed (by Aurelien Jarno) that even for qemu, this
> approach "improves network transfers in a huge way".
>
> Please apply, or give a firm rejection so I can stop resending.
>   

Probably the right thing to do is the direction KVM is moving toward, 
i.e. have a separate IO thread.

Setting SIGIO on every file descriptor is really just a hack to break 
out of the cpu exec loop.  It's unclear to me whether it's really always 
the right thing to do for every file descriptor.

Regards,

Anthony Liguori

> Cheers,
> Anders.
>
>
>
>   

Re: [Qemu-devel] Linux SIGIO handling changes

[Avi Kivity]

Anthony Liguori wrote:
> Anders Melchiorsen wrote:
>> I am resending this patch, split into two parts now.
>>
>> This cleans up SIGIO handling to improve latency:
>> - SIGALRM for alarm timers
>> - enable SIGIO on qemu_set_fd_handler2()
>>
>> The issue was found in KVM, where it is much more visible,
>> because there is no periodic timer. However, it has been
>> confirmed (by Aurelien Jarno) that even for qemu, this
>> approach "improves network transfers in a huge way".
>>
>> Please apply, or give a firm rejection so I can stop resending.
>>   
>
> Probably the right thing to do is the direction KVM is moving toward, 
> i.e. have a separate IO thread.
>
> Setting SIGIO on every file descriptor is really just a hack to break 
> out of the cpu exec loop.  It's unclear to me whether it's really 
> always the right thing to do for every file descriptor.
>

Even with a separate iothread one needs the signals, as there is no 
other race-free way to poll for both aio completions and fd readiness.

[well, pselect works, but I dislike it and it isn't present on all kernels]

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Linux SIGIO handling changes

[Anthony Liguori]

Avi Kivity wrote:
> Anthony Liguori wrote:
>> Anders Melchiorsen wrote:
>>> I am resending this patch, split into two parts now.
>>>
>>> This cleans up SIGIO handling to improve latency:
>>> - SIGALRM for alarm timers
>>> - enable SIGIO on qemu_set_fd_handler2()
>>>
>>> The issue was found in KVM, where it is much more visible,
>>> because there is no periodic timer. However, it has been
>>> confirmed (by Aurelien Jarno) that even for qemu, this
>>> approach "improves network transfers in a huge way".
>>>
>>> Please apply, or give a firm rejection so I can stop resending.
>>>   
>>
>> Probably the right thing to do is the direction KVM is moving toward, 
>> i.e. have a separate IO thread.
>>
>> Setting SIGIO on every file descriptor is really just a hack to break 
>> out of the cpu exec loop.  It's unclear to me whether it's really 
>> always the right thing to do for every file descriptor.
>>
>
> Even with a separate iothread one needs the signals, as there is no 
> other race-free way to poll for both aio completions and fd readiness.

Unless you emulate signalfd() using a thread.  FWIW, I've been thinking 
of implementing something similar to posix-aio (using a thread-pool for 
AIO) for QEMU to get a bit more control for this sort of thing.

posix-aio is a pretty unfortunate interface as it doesn't provide a 
mechanism to do asynchronous fdatasync() nor individual vector requests.

Regards,

Anthony Liguori

Regards,

Anthony Liguori

> [well, pselect works, but I dislike it and it isn't present on all 
> kernels]
>

Re: [Qemu-devel] Linux SIGIO handling changes

[Paul Brook]

> posix-aio is a pretty unfortunate interface as it doesn't provide a
> mechanism to do asynchronous fdatasync() 

Isn't that what aio_fsync(O_DSYNC) does?

Paul

Re: [Qemu-devel] Linux SIGIO handling changes

[Anthony Liguori]

Paul Brook wrote:
>> posix-aio is a pretty unfortunate interface as it doesn't provide a
>> mechanism to do asynchronous fdatasync() 
>>     
>
> Isn't that what aio_fsync(O_DSYNC) does?
>   

Indeed.  So it's really just the lack of vectored operations and the 
fact that we limit it to a single thread.

Regards,

Anthony Liguori

> Paul
>   

Re: [Qemu-devel] Linux SIGIO handling changes

[Avi Kivity]

Anthony Liguori wrote:
>>
>> Even with a separate iothread one needs the signals, as there is no 
>> other race-free way to poll for both aio completions and fd readiness.
>
> Unless you emulate signalfd() using a thread.  FWIW, I've been 
> thinking of implementing something similar to posix-aio (using a 
> thread-pool for AIO) for QEMU to get a bit more control for this sort 
> of thing.
>

Yes, of course.  It's still a separate thread, but as it's hidden from 
the main logic, that's no problem.

-- 
error compiling committee.c: too many arguments to function