diff mbox

[3/6] Make char muxer more robust wrt small FIFOs

Message ID 1270140161-17216-4-git-send-email-agraf@suse.de
State New
Headers show

Commit Message

Alexander Graf April 1, 2010, 4:42 p.m. UTC
Virtio-Console can only process one character at a time. Using it on S390
gave me strage "lags" where I got the character I pressed before when
pressing one. So I typed in "abc" and only received "a", then pressed "d"
but the guest received "b" and so on.

While the stdio driver calls a poll function that just processes on its
queue in case virtio-console can't take multiple characters at once, the
muxer does not have such callbacks, so it can't empty its queue.

To work around that limitation, I introduced a new timer that only gets
active when the guest can not receive any more characters. In that case
it polls again after a while to check if the guest is now receiving input.

This patch fixes input when using -nographic on s390 for me.

---

Please consider for stable.
---
 qemu-char.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

Comments

Amit Shah April 5, 2010, 3:40 a.m. UTC | #1
On (Thu) Apr 01 2010 [18:42:38], Alexander Graf wrote:
> Virtio-Console can only process one character at a time.

The host can process as many as you give it, depending on the buffer
size exposed by the guest.

On older guests (guest kernels w/o multiport support), the guest reads
input from host, processes it and only then opens up another buffer for
the host to write into.

On newer guests (guest kernels that support multiport), the guest
fills the entire vq so that host can send as many buffers as possible
without getting throttled. I guess you're getting hit by this.

> Using it on S390
> gave me strage "lags" where I got the character I pressed before when
> pressing one. So I typed in "abc" and only received "a", then pressed "d"
> but the guest received "b" and so on.

This might be because qemu-char would not be able to send out 'b' while
the guest still processes 'a' and has no free buffers to write out to.
On seeing 'd', it flushes its queue.

Can you try using a 2.6.34-rc3 kernel without this patch to see if
things work fine?

		Amit
Alexander Graf April 7, 2010, 2:32 p.m. UTC | #2
Amit Shah wrote:
> On (Thu) Apr 01 2010 [18:42:38], Alexander Graf wrote:
>   
>> Virtio-Console can only process one character at a time.
>>     
>
> The host can process as many as you give it, depending on the buffer
> size exposed by the guest.
>
> On older guests (guest kernels w/o multiport support), the guest reads
> input from host, processes it and only then opens up another buffer for
> the host to write into.
>
> On newer guests (guest kernels that support multiport), the guest
> fills the entire vq so that host can send as many buffers as possible
> without getting throttled. I guess you're getting hit by this.
>   

Probably, yes.

>> Using it on S390
>> gave me strage "lags" where I got the character I pressed before when
>> pressing one. So I typed in "abc" and only received "a", then pressed "d"
>> but the guest received "b" and so on.
>>     
>
> This might be because qemu-char would not be able to send out 'b' while
> the guest still processes 'a' and has no free buffers to write out to.
> On seeing 'd', it flushes its queue.
>
> Can you try using a 2.6.34-rc3 kernel without this patch to see if
> things work fine?
>   

Hrm - would it actually matter? We need to have older guest kernels
working anyways. And my S390 LPAR doesn't exactly have a lot of disk
space, so compiling a kernel is anything but fun :-).


Alex
Amit Shah April 7, 2010, 2:43 p.m. UTC | #3
On (Wed) Apr 07 2010 [16:32:04], Alexander Graf wrote:
> Amit Shah wrote:
> > On (Thu) Apr 01 2010 [18:42:38], Alexander Graf wrote:
> >   
> >> Virtio-Console can only process one character at a time.
> >>     
> >
> > The host can process as many as you give it, depending on the buffer
> > size exposed by the guest.
> >
> > On older guests (guest kernels w/o multiport support), the guest reads
> > input from host, processes it and only then opens up another buffer for
> > the host to write into.
> >
> > On newer guests (guest kernels that support multiport), the guest
> > fills the entire vq so that host can send as many buffers as possible
> > without getting throttled. I guess you're getting hit by this.
> >   
> 
> Probably, yes.
> 
> >> Using it on S390
> >> gave me strage "lags" where I got the character I pressed before when
> >> pressing one. So I typed in "abc" and only received "a", then pressed "d"
> >> but the guest received "b" and so on.
> >>     
> >
> > This might be because qemu-char would not be able to send out 'b' while
> > the guest still processes 'a' and has no free buffers to write out to.
> > On seeing 'd', it flushes its queue.
> >
> > Can you try using a 2.6.34-rc3 kernel without this patch to see if
> > things work fine?
> >   
> 
> Hrm - would it actually matter? We need to have older guest kernels
> working anyways. And my S390 LPAR doesn't exactly have a lot of disk
> space, so compiling a kernel is anything but fun :-).

Oh I just want to find out if it works for you -- you shouldn't get hit
by the 1-char limit anymore.

		Amit
diff mbox

Patch

diff --git a/qemu-char.c b/qemu-char.c
index 048da3f..6ad6609 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -219,6 +219,7 @@  typedef struct {
     IOEventHandler *chr_event[MAX_MUX];
     void *ext_opaque[MAX_MUX];
     CharDriverState *drv;
+    QEMUTimer *accept_timer;
     int focus;
     int mux_cnt;
     int term_got_escape;
@@ -380,6 +381,13 @@  static void mux_chr_accept_input(CharDriverState *chr)
         d->chr_read[m](d->ext_opaque[m],
                        &d->buffer[m][d->cons[m]++ & MUX_BUFFER_MASK], 1);
     }
+
+    /* We're still not able to sync producer and consumer, so let's wait a bit
+       and try again by then. */
+    if (d->prod[m] != d->cons[m]) {
+        qemu_mod_timer(d->accept_timer, qemu_get_clock(vm_clock)
+                                        + (int64_t)100000);
+    }
 }
 
 static int mux_chr_can_read(void *opaque)
@@ -462,6 +470,8 @@  static CharDriverState *qemu_chr_open_mux(CharDriverState *drv)
     chr->opaque = d;
     d->drv = drv;
     d->focus = -1;
+    d->accept_timer = qemu_new_timer(vm_clock,
+                                     (QEMUTimerCB*)mux_chr_accept_input, chr);
     chr->chr_write = mux_chr_write;
     chr->chr_update_read_handler = mux_chr_update_read_handler;
     chr->chr_accept_input = mux_chr_accept_input;