Re: [PATCH v2 2/4] cpus: Make {start,end}_exclusive() recursive

[Richard Henderson]

On 2/13/23 02:52, Ilya Leoshkevich wrote:
> Currently dying to one of the core_dump_signal()s deadlocks, because
> dump_core_and_abort() calls start_exclusive() two times: first via
> stop_all_tasks(), and then via preexit_cleanup() ->
> qemu_plugin_user_exit().
>
> There are a number of ways to solve this: resume after dumping core;
> check cpu_in_exclusive_context() in qemu_plugin_user_exit(); or make
> {start,end}_exclusive() recursive. Pick the last option, since it's
> the most straightforward one.
>
> Fixes: da91c1920242 ("linux-user: Clean up when exiting due to a signal")
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   cpus-common.c         | 12 ++++++++++--
>   include/hw/core/cpu.h |  4 ++--
>   2 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/cpus-common.c b/cpus-common.c
> index 793364dc0ed..a0c52cd187f 100644
> --- a/cpus-common.c
> +++ b/cpus-common.c
> @@ -192,6 +192,11 @@ void start_exclusive(void)
>       CPUState *other_cpu;
>       int running_cpus;
>   
> +    if (current_cpu->exclusive_context_count) {
> +        current_cpu->exclusive_context_count++;
> +        return;
> +    }
> +
>       qemu_mutex_lock(&qemu_cpu_list_lock);
>       exclusive_idle();
>   
> @@ -219,13 +224,16 @@ void start_exclusive(void)
>        */
>       qemu_mutex_unlock(&qemu_cpu_list_lock);
>   
> -    current_cpu->in_exclusive_context = true;
> +    current_cpu->exclusive_context_count++;

I think this line would be clearer as "= 1".

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~