Patchwork [RESEND] char: Flush read buffer in mux_chr_can_read

login
register
mail settings
Submitter Jan Kiszka
Date Oct. 19, 2010, 3:04 p.m.
Message ID <4CBDB361.1060606@siemens.com>
Download mbox | patch
Permalink /patch/68344/
State New
Headers show

Comments

Jan Kiszka - Oct. 19, 2010, 3:04 p.m.
Move the buffer flush from mux_chr_read to mux_chr_can_read. While the
latter is called periodically, the former will only be invoked when new
characters arrive at the back-end. This caused problems to front-end
drivers whenever they were unable to read data immediately, e.g.
virtio-console attached to stdio.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 qemu-char.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Alexander Graf - Oct. 19, 2010, 3:22 p.m.
Am 19.10.2010 um 17:04 schrieb Jan Kiszka <jan.kiszka@siemens.com>:

> Move the buffer flush from mux_chr_read to mux_chr_can_read. While the
> latter is called periodically, the former will only be invoked when new
> characters arrive at the back-end. This caused problems to front-end
> drivers whenever they were unable to read data immediately, e.g.
> virtio-console attached to stdio.

This should help ny s390 virtio console issue, right? :) I'll try it out asap, worst case that might take until the weekend.

Alex

>
Jan Kiszka - Oct. 19, 2010, 3:26 p.m.
Am 19.10.2010 17:22, Alexander Graf wrote:
> 
> Am 19.10.2010 um 17:04 schrieb Jan Kiszka <jan.kiszka@siemens.com>:
> 
>> Move the buffer flush from mux_chr_read to mux_chr_can_read. While the
>> latter is called periodically, the former will only be invoked when new
>> characters arrive at the back-end. This caused problems to front-end
>> drivers whenever they were unable to read data immediately, e.g.
>> virtio-console attached to stdio.
> 
> This should help ny s390 virtio console issue, right? :) I'll try it out asap, worst case that might take until the weekend.

I think you already tried and still found some scenarios broken (we
still need to understand them IIRC). However, this one fixes at least
those virtio-console scenarios I tested (with x86 guests).

Jan
Jan Kiszka - Nov. 17, 2010, 10:18 a.m.
Am 19.10.2010 17:04, Jan Kiszka wrote:
> Move the buffer flush from mux_chr_read to mux_chr_can_read. While the
> latter is called periodically, the former will only be invoked when new
> characters arrive at the back-end. This caused problems to front-end
> drivers whenever they were unable to read data immediately, e.g.
> virtio-console attached to stdio.
> 

Ping.

> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  qemu-char.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 6d2dce7..f4c3876 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -398,6 +398,8 @@ static int mux_chr_can_read(void *opaque)
>      MuxDriver *d = chr->opaque;
>      int m = d->focus;
>  
> +    mux_chr_accept_input(opaque);
> +
>      if ((d->prod[m] - d->cons[m]) < MUX_BUFFER_SIZE)
>          return 1;
>      if (d->chr_can_read[m])
> @@ -412,8 +414,6 @@ static void mux_chr_read(void *opaque, const uint8_t *buf, int size)
>      int m = d->focus;
>      int i;
>  
> -    mux_chr_accept_input (opaque);
> -
>      for(i = 0; i < size; i++)
>          if (mux_proc_byte(chr, d, buf[i])) {
>              if (d->prod[m] == d->cons[m] &&
Alexander Graf - Nov. 17, 2010, 11:55 a.m.
On 17.11.2010, at 11:18, Jan Kiszka wrote:

> Am 19.10.2010 17:04, Jan Kiszka wrote:
>> Move the buffer flush from mux_chr_read to mux_chr_can_read. While the
>> latter is called periodically, the former will only be invoked when new
>> characters arrive at the back-end. This caused problems to front-end
>> drivers whenever they were unable to read data immediately, e.g.
>> virtio-console attached to stdio.
>> 
> 
> Ping.

It's still broken, even with this patch applied.


Alex
Jan Kiszka - Nov. 17, 2010, 11:58 a.m.
Am 17.11.2010 12:55, Alexander Graf wrote:
> 
> On 17.11.2010, at 11:18, Jan Kiszka wrote:
> 
>> Am 19.10.2010 17:04, Jan Kiszka wrote:
>>> Move the buffer flush from mux_chr_read to mux_chr_can_read. While the
>>> latter is called periodically, the former will only be invoked when new
>>> characters arrive at the back-end. This caused problems to front-end
>>> drivers whenever they were unable to read data immediately, e.g.
>>> virtio-console attached to stdio.
>>>
>>
>> Ping.
> 
> It's still broken, even with this patch applied.
> 

No surprise. Still, the patch fixes other archs (namely x86).

Jan
Alexander Graf - Nov. 17, 2010, noon
On 17.11.2010, at 12:58, Jan Kiszka wrote:

> Am 17.11.2010 12:55, Alexander Graf wrote:
>> 
>> On 17.11.2010, at 11:18, Jan Kiszka wrote:
>> 
>>> Am 19.10.2010 17:04, Jan Kiszka wrote:
>>>> Move the buffer flush from mux_chr_read to mux_chr_can_read. While the
>>>> latter is called periodically, the former will only be invoked when new
>>>> characters arrive at the back-end. This caused problems to front-end
>>>> drivers whenever they were unable to read data immediately, e.g.
>>>> virtio-console attached to stdio.
>>>> 
>>> 
>>> Ping.
>> 
>> It's still broken, even with this patch applied.
>> 
> 
> No surprise. Still, the patch fixes other archs (namely x86).

Sure, no complaints on the patch from my side. The framework is still broken though :).


Alex

Patch

diff --git a/qemu-char.c b/qemu-char.c
index 6d2dce7..f4c3876 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -398,6 +398,8 @@  static int mux_chr_can_read(void *opaque)
     MuxDriver *d = chr->opaque;
     int m = d->focus;
 
+    mux_chr_accept_input(opaque);
+
     if ((d->prod[m] - d->cons[m]) < MUX_BUFFER_SIZE)
         return 1;
     if (d->chr_can_read[m])
@@ -412,8 +414,6 @@  static void mux_chr_read(void *opaque, const uint8_t *buf, int size)
     int m = d->focus;
     int i;
 
-    mux_chr_accept_input (opaque);
-
     for(i = 0; i < size; i++)
         if (mux_proc_byte(chr, d, buf[i])) {
             if (d->prod[m] == d->cons[m] &&