Re: [Qemu-devel] [PATCH] monitor: increase amount of data for monitor to read

[Markus Armbruster]

Did this fall through the cracks?

Denis Plotnikov <address@hidden> writes:

> Right now QMP and HMP monitors read 1 byte at a time from the socket, which
> is very inefficient. With 100+ VMs on the host this easily reasults in
> a lot of unnecessary system calls and CPU usage in the system.

Yes, reading one byte at a time is awful.  But QMP is control plane; I
didn't expect it to impact system performance.  How are you using QMP?
Just curious, not actually opposed to improving QMP efficiency.

> This patch changes the amount of data to read to 4096 bytes, which matches
> buffer size on the channel level. Fortunately, monitor protocol is
> synchronous right now thus we should not face side effects in reality.
>
> Signed-off-by: Denis V. Lunev <address@hidden>
> Signed-off-by: Denis Plotnikov <address@hidden>
> ---
>  include/monitor/monitor.h | 2 +-
>  monitor.c                 | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index c1b40a9cac..afa1ed34a4 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -14,7 +14,7 @@ extern __thread Monitor *cur_mon;
>  #define MONITOR_USE_CONTROL   0x04
>  #define MONITOR_USE_PRETTY    0x08
>  
> -#define QMP_REQ_QUEUE_LEN_MAX 8
> +#define QMP_REQ_QUEUE_LEN_MAX 4096

This looks suspicious.  It's a request count, not a byte count.  Can you
explain what led you to change it this way?

>  
>  bool monitor_cur_is_qmp(void);
>  
> diff --git a/monitor.c b/monitor.c
> index 4807bbe811..a08e020b61 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4097,7 +4097,7 @@ static int monitor_can_read(void *opaque)
>  {
>      Monitor *mon = opaque;
>  
> -    return !atomic_mb_read(&mon->suspend_cnt);
> +    return !atomic_mb_read(&mon->suspend_cnt) ? 4096 : 0;
>  }
>  
>  /*

The ramifications are not obvious to me.  I think I need to (re-)examine
how QMP reads input, with special consideration to its OOB feature.