Re: [Qemu-devel] [RFC v6 10/27] monitor: allow to use IO thread for parsing

[Fam Zheng]

On Tue, 12/19 16:45, Peter Xu wrote:
> For each Monitor, add one field "use_io_thr" to show whether it will be
> using the dedicated monitor IO thread to handle input/output.  When set,
> monitor IO parsing work will be offloaded to dedicated monitor IO
> thread, rather than the original main loop thread.
> 
> This only works for QMP.  HMP will always be run on main loop thread.
> 
> Currently we're still keeping use_io_thr to off always.  Will turn it on
> later at some point.
> 
> One thing to mention is that we cannot set use_io_thr for every QMP
> monitors.  The problem is that MUXed typed chardevs may not work well
> with it now. When MUX is used, frontend of chardev can be the monitor
> plus something else.  The only thing we know would be safe to be run
> outside main thread so far is the monitor frontend. All the rest of the
> frontends should still be run in main thread only.
> 
> Signed-off-by: Peter Xu <address@hidden>
> ---
>  monitor.c | 41 ++++++++++++++++++++++++++++++++---------
>  1 file changed, 32 insertions(+), 9 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index 4ebd83fd94..4b2bee773f 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -190,6 +190,7 @@ struct Monitor {
>      int flags;
>      int suspend_cnt;
>      bool skip_flush;
> +    bool use_io_thr;
>  
>      QemuMutex out_lock;
>      QString *outbuf;
> @@ -573,7 +574,8 @@ static void monitor_qapi_event_init(void)
>  
>  static void handle_hmp_command(Monitor *mon, const char *cmdline);
>  
> -static void monitor_data_init(Monitor *mon, bool skip_flush)
> +static void monitor_data_init(Monitor *mon, bool skip_flush,
> +                              bool use_io_thr)
>  {
>      memset(mon, 0, sizeof(Monitor));
>      qemu_mutex_init(&mon->out_lock);
> @@ -581,6 +583,7 @@ static void monitor_data_init(Monitor *mon, bool 
> skip_flush)
>      /* Use *mon_cmds by default. */
>      mon->cmd_table = mon_cmds;
>      mon->skip_flush = skip_flush;
> +    mon->use_io_thr = use_io_thr;
>  }
>  
>  static void monitor_data_destroy(Monitor *mon)
> @@ -601,7 +604,7 @@ char *qmp_human_monitor_command(const char *command_line, 
> bool has_cpu_index,
>      char *output = NULL;
>      Monitor *old_mon, hmp;
>  
> -    monitor_data_init(&hmp, true);
> +    monitor_data_init(&hmp, true, false);
>  
>      old_mon = cur_mon;
>      cur_mon = &hmp;
> @@ -4103,8 +4106,9 @@ void error_vprintf_unless_qmp(const char *fmt, va_list 
> ap)
>  void monitor_init(Chardev *chr, int flags)
>  {
>      Monitor *mon = g_malloc(sizeof(*mon));
> +    GMainContext *context;
>  
> -    monitor_data_init(mon, false);
> +    monitor_data_init(mon, false, false);
>  
>      qemu_chr_fe_init(&mon->chr, chr, &error_abort);
>      mon->flags = flags;
> @@ -4116,19 +4120,38 @@ void monitor_init(Chardev *chr, int flags)
>          monitor_read_command(mon, 0);
>      }
>  
> +    if (mon->use_io_thr) {
> +        /*
> +         * When use_io_thr is set, we use the global shared dedicated
> +         * IO thread for this monitor to handle input/output.
> +         */
> +        context = monitor_io_context_get();
> +        /* We should have inited globals before reaching here. */
> +        assert(context);
> +    } else {
> +        /* The default main loop, which is the main thread */
> +        context = NULL;
> +    }
> +
> +    /*
> +     * Add the monitor before running it (which is triggered by
> +     * qemu_chr_fe_set_handlers), otherwise one monitor may find
> +     * itself not on the mon_list when running.
> +     */
> +    qemu_mutex_lock(&monitor_lock);
> +    QTAILQ_INSERT_HEAD(&mon_list, mon, entry);
> +    qemu_mutex_unlock(&monitor_lock);
> +
>      if (monitor_is_qmp(mon)) {
> -        qemu_chr_fe_set_handlers(&mon->chr, monitor_can_read, 
> monitor_qmp_read,
> -                                 monitor_qmp_event, NULL, mon, NULL, true);
>          qemu_chr_fe_set_echo(&mon->chr, true);
>          json_message_parser_init(&mon->qmp.parser, handle_qmp_command);
> +        qemu_chr_fe_set_handlers(&mon->chr, monitor_can_read, 
> monitor_qmp_read,
> +                                 monitor_qmp_event, NULL, mon, context, 
> true);
>      } else {
> +        assert(!context);       /* HMP does not support IOThreads */
>          qemu_chr_fe_set_handlers(&mon->chr, monitor_can_read, monitor_read,
>                                   monitor_event, NULL, mon, NULL, true);
>      }
> -
> -    qemu_mutex_lock(&monitor_lock);
> -    QLIST_INSERT_HEAD(&mon_list, mon, entry);
> -    qemu_mutex_unlock(&monitor_lock);
>  }
>  
>  void monitor_cleanup(void)
> -- 
> 2.14.3
> 

Reviewed-by: Fam Zheng <address@hidden>