[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 1/4] monitor: rename out_lock to mon_lock
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v5 1/4] monitor: rename out_lock to mon_lock |
Date: |
Thu, 17 May 2018 14:32:34 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) |
Peter Xu <address@hidden> writes:
> The out_lock was only protecting a few Monitor fields. In the future
"was protecting"? When? Or do you mean "is protecting"?
> the monitor code will start to run in multiple threads. We turn it into
> a bigger lock to protect not only the out buffer but also all the rest.
"We turn it into a bigger lock"? If this patch does what its title
claims, it can't "turn" anything. Do you perhaps mean "We're going to
turn it into a bigger lock"?
> For now we share the lock. We can split the lock when needed.
What exactly do you mean by "we share the lock"?
> Since at it, arrange the Monitor struct a bit.
"Since we're at it, rearrange". Can touch up on commit.
>
> Reviewed-by: Stefan Hajnoczi <address@hidden>
> Signed-off-by: Peter Xu <address@hidden>
> ---
> monitor.c | 53 +++++++++++++++++++++++++++++------------------------
> 1 file changed, 29 insertions(+), 24 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 46814af533..14c681dc8a 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -207,15 +207,6 @@ struct Monitor {
> int suspend_cnt; /* Needs to be accessed atomically */
> bool skip_flush;
> bool use_io_thr;
> -
> - /* We can't access guest memory when holding the lock */
> - QemuMutex out_lock;
> - QString *outbuf;
> - guint out_watch;
> -
> - /* Read under either BQL or out_lock, written with BQL+out_lock. */
> - int mux_out;
> -
> ReadLineState *rs;
> MonitorQMP qmp;
> gchar *mon_cpu_path;
> @@ -224,6 +215,20 @@ struct Monitor {
> mon_cmd_t *cmd_table;
> QLIST_HEAD(,mon_fd_t) fds;
> QTAILQ_ENTRY(Monitor) entry;
> +
> + /*
> + * The per-monitor lock. We can't access guest memory when holding
> + * the lock.
> + */
> + QemuMutex mon_lock;
> +
> + /*
> + * Fields that are protected by the per-monitor lock.
> + */
> + QString *outbuf;
> + guint out_watch;
> + /* Read under either BQL or mon_lock, written with BQL+mon_lock. */
> + int mux_out;
This is an improvement.
> };
>
> /* Let's add monitor global variables to this struct. */
[Remainder of patch snipped; it looks completely mechanical]