[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: |
Thu, 19 Jul 2018 16:56:24 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Peter Xu <address@hidden> writes:
> On Thu, Jul 19, 2018 at 02:14:56PM +0200, Markus Armbruster wrote:
[...]
>> Here's my try:
>>
>> monitor: Fix unsafe sharing of @cur_mon among threads
>>
>> @cur_mon is null unless the main thread is running monitor code, either
>> HMP code within monitor_read(), or QMP code within
>> monitor_qmp_dispatch().
>>
>> Use of @cur_mon outside the main thread is therefore unsafe.
>>
>> Most of its uses are in monitor command handlers. These run in the main
>> thread.
>>
>> However, there are also uses hiding elsewhere, such as in
>> error_vprintf(), and thus error_report(), making these functions unsafe
>> outside the main thread. No such unsafe uses are known at this time.
>> Regardless, this is an unnecessary trap. It's an ancient trap, though.
>>
>> More recently, commit cf869d53172 "qmp: support out-of-band (oob)
>> execution" spiced things up: the monitor I/O thread assigns to @cur_mon
>> when executing commands out-of-band. Having two threads save, set and
>> restore @cur_mon without synchronization is definitely unsafe. We can
>> end up with @cur_mon null while the main thread runs monitor code, or
>> non-null while it runs non-monitor code.
>>
>> We could fix this by making the I/O thread not mess with @cur_mon, but
>> that would leave the trap armed and ready.
>>
>> Instead, make @cur_mon thread-local. It's now reliably null unless the
>> thread is running monitor code.
>
> Looks good to me.
With that commit message:
Reviewed-by: Markus Armbruster <address@hidden>
- Re: [Qemu-devel] [PATCH v4] monitor: let cur_mon be per-thread, Markus Armbruster, 2018/07/18
- 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 <=