[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread |
Date: |
Wed, 18 Jul 2018 17:38:11 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Peter Xu <address@hidden> writes:
> After the Out-Of-Band work, the monitor iothread may be accessing the
> cur_mon as well (via monitor_qmp_dispatch_one()). Let's convert the
> cur_mon variable to be a per-thread variable to make sure there won't be
> a race between threads when accessing the variable.
Hmm... why hasn't the OOB work created such a race already?
A monitor reads, parses, dispatches and executes commands, formats and
sends replies.
Before OOB, all of that ran in the main thread. Any access of cur_mon
should therefore be from the main thread. No races.
OOB moves read, parse, format and send to an I/O thread. Dispatch and
execute remain in the main thread. *Except* for commands executed OOB,
dispatch and execute move to the I/O thread, too.
Why is this not racy? I guess it relies on careful non-use of cur_mon
in any part that may now execute in the I/O thread. Scary...
Should this go into 3.0 to reduce the risk of bugs?
> Note that thread variables are not initialized to a valid value when new
> thread is created. However for our case we don't need to set it up,
> since the cur_mon variable is only used in such a pattern:
>
> old_mon = cur_mon;
> cur_mon = xxx;
> (do something, read cur_mon if necessary in the stack)
> cur_mon = old_mon;
>
> It plays a role as stack variable, so no need to be initialized at all.
> We only need to make sure the variable won't be changed unexpectedly by
> other threads.
>
> Reviewed-by: Eric Blake <address@hidden>
> Reviewed-by: Marc-André Lureau <address@hidden>
> Reviewed-by: Stefan Hajnoczi <address@hidden>
> [peterx: touch up commit message a bit]
> Signed-off-by: Peter Xu <address@hidden>
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread,
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread, Peter Xu, 2018/07/19
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread, Markus Armbruster, 2018/07/19
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread, Peter Xu, 2018/07/19
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread, Markus Armbruster, 2018/07/19
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread, Peter Xu, 2018/07/19
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread, Markus Armbruster, 2018/07/19
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread, Peter Xu, 2018/07/19
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread, Markus Armbruster, 2018/07/19