Patchwork Kick io-thread on qemu_chr_accept_input

login
register
mail settings
Submitter Jan Kiszka
Date March 16, 2012, 12:25 p.m.
Message ID <4F633153.9030004@siemens.com>
Download mbox | patch
Permalink /patch/147195/
State New
Headers show

Comments

Jan Kiszka - March 16, 2012, 12:25 p.m.
Once a chr frontend is able to receive input again, we need to inform
the io-thread about this fact. Otherwise, main_loop_wait may continue to
select without the related backend file descriptor in its set. This can
cause high input latencies if only low-rate events arrive otherwise.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

/me wonders if a similar issue explains the slirp slowness under KVM
with in-kernel irqchip enabled. Need to check...

 qemu-char.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Anthony Liguori - March 16, 2012, 1:16 p.m.
On 03/16/2012 07:25 AM, Jan Kiszka wrote:
> Once a chr frontend is able to receive input again, we need to inform
> the io-thread about this fact. Otherwise, main_loop_wait may continue to
> select without the related backend file descriptor in its set. This can
> cause high input latencies if only low-rate events arrive otherwise.
>
> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>

I'm not nacking this patch, but please note that this is a band-aid as not all 
char devices actually use qemu_chr_accept_input().

Regards,

Anthony Liguori

> ---
>
> /me wonders if a similar issue explains the slirp slowness under KVM
> with in-kernel irqchip enabled. Need to check...
>
>   qemu-char.c |    1 +
>   1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index 9a5be75..a589a84 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -177,6 +177,7 @@ void qemu_chr_accept_input(CharDriverState *s)
>   {
>       if (s->chr_accept_input)
>           s->chr_accept_input(s);
> +    qemu_notify_event();
>   }
>
>   void qemu_chr_fe_printf(CharDriverState *s, const char *fmt, ...)
Jan Kiszka - March 16, 2012, 1:18 p.m.
On 2012-03-16 14:16, Anthony Liguori wrote:
> On 03/16/2012 07:25 AM, Jan Kiszka wrote:
>> Once a chr frontend is able to receive input again, we need to inform
>> the io-thread about this fact. Otherwise, main_loop_wait may continue to
>> select without the related backend file descriptor in its set. This can
>> cause high input latencies if only low-rate events arrive otherwise.
>>
>> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>
> 
> I'm not nacking this patch, but please note that this is a band-aid as not all 
> char devices actually use qemu_chr_accept_input().

Then they have to be fixed. :)

Jan
Jan Kiszka - March 29, 2012, 12:19 p.m.
On 2012-03-16 14:18, Jan Kiszka wrote:
> On 2012-03-16 14:16, Anthony Liguori wrote:
>> On 03/16/2012 07:25 AM, Jan Kiszka wrote:
>>> Once a chr frontend is able to receive input again, we need to inform
>>> the io-thread about this fact. Otherwise, main_loop_wait may continue to
>>> select without the related backend file descriptor in its set. This can
>>> cause high input latencies if only low-rate events arrive otherwise.
>>>
>>> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>
>>
>> I'm not nacking this patch, but please note that this is a band-aid as not all 
>> char devices actually use qemu_chr_accept_input().
> 
> Then they have to be fixed. :)

Could you apply the patch in the meantime? It is not band-aid, but a
required base to fix such issues.

Jan

Patch

diff --git a/qemu-char.c b/qemu-char.c
index 9a5be75..a589a84 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -177,6 +177,7 @@  void qemu_chr_accept_input(CharDriverState *s)
 {
     if (s->chr_accept_input)
         s->chr_accept_input(s);
+    qemu_notify_event();
 }
 
 void qemu_chr_fe_printf(CharDriverState *s, const char *fmt, ...)