[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PULL 3/5] softmmu/cpus: Only set parallel_cpus for SMP
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PULL 3/5] softmmu/cpus: Only set parallel_cpus for SMP |
Date: |
Thu, 10 Sep 2020 12:32:18 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0 |
On 9/10/20 10:28 AM, Claudio Fontana wrote:
> Hi Philippe,
>
> On 9/8/20 9:56 AM, Philippe Mathieu-Daudé wrote:
>> +Laurent
>>
>> On 9/8/20 9:09 AM, Claudio Fontana wrote:
>>> On 9/7/20 6:49 PM, Philippe Mathieu-Daudé wrote:
>>>> On 9/7/20 6:30 PM, Claudio Fontana wrote:
>>>>> On 9/7/20 12:20 PM, Philippe Mathieu-Daudé wrote:
>>>>>> On 9/7/20 12:05 PM, Claudio Fontana wrote:
>>>>>>> Hi Richard,
>>>>>>>
>>>>>>> currently rebasing on top of this one,
>>>>>>> just a question, why is the patch not directly using "current_machine"?
>>>>>>
>>>>>> For user mode?
>>>>>
>>>>> In user mode I'd not expect softmmu/cpus.c to be built at all...
>>>>
>>>> Which is why :) current_machine is NULL in user-mode.
>>>
>>> Ciao Philippe,
>>>
>>> then why does the patch change softmmu/cpus.c in a way that accounts for
>>> user mode?
>>>
>>> The function that the patch changes is never called in user mode.
>>>
>>> The patch could instead use current_machine without any concern of it being
>>> NULL, it will always be set in vl.c .
>>
>> Better send a patch to prove your point :)
>
> Yes, I am already testing without problems the cpus-refactoring series with
> this applied:
>
> commit 53f6db772f1522025650441102b16be17773bdc9
> Author: Claudio Fontana <cfontana@suse.de>
> Date: Tue Sep 8 10:59:07 2020 +0200
>
> accel/tcg: use current_machine as it is always set for softmmu
>
> current_machine is always set before accelerators are initialized,
> so use that instead of MACHINE(qdev_get_machine()).
>
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>
> diff --git a/accel/tcg/tcg-cpus.c b/accel/tcg/tcg-cpus.c
> index ec7158b55e..05af1168a2 100644
> --- a/accel/tcg/tcg-cpus.c
> +++ b/accel/tcg/tcg-cpus.c
> @@ -484,7 +484,7 @@ static void tcg_start_vcpu_thread(CPUState *cpu)
> * then we will have cpus running in parallel.
> */
> if (qemu_tcg_mttcg_enabled()) {
> - MachineState *ms = MACHINE(qdev_get_machine());
> + MachineState *ms = current_machine;
> if (ms->smp.max_cpus > 1) {
> parallel_cpus = true;
> }
>
>
>
>>
>> I have bad remember about the last time I tried to understand/modify
>> that part, because IIRC the user-mode creates some fake machine to
>> satisfy various QEMU generic code assumptions. Sincerely I now prefer
>> stay away from this; too many unmerged patches there.
>
>
> linux-user/main.c and bsd-user/main.c seem to use cpu_create() to create the
> cpus,
> then they have their own cpu_loop(), they do not use any of the cpus.c code.
>
> By contrast, softmmu/vl.c initializes current_machine in qemu_init(),
> even before accelerator is chosen and configured.
>
> So there is no chance currently that current_machine is not set correctly when
> the accelerator vcpu startup code in is called.
Thanks for checking and correcting me!
We are good then :)
>
> Ciao,
>
> CLaudio
- [PULL 0/5] tcg patch queue, Richard Henderson, 2020/09/03
- [PULL 1/5] cputlb: Make store_helper less fragile to compiler optimizations, Richard Henderson, 2020/09/03
- [PULL 3/5] softmmu/cpus: Only set parallel_cpus for SMP, Richard Henderson, 2020/09/03
- Re: [PULL 3/5] softmmu/cpus: Only set parallel_cpus for SMP, Claudio Fontana, 2020/09/07
- Re: [PULL 3/5] softmmu/cpus: Only set parallel_cpus for SMP, Philippe Mathieu-Daudé, 2020/09/07
- Re: [PULL 3/5] softmmu/cpus: Only set parallel_cpus for SMP, Claudio Fontana, 2020/09/07
- Re: [PULL 3/5] softmmu/cpus: Only set parallel_cpus for SMP, Philippe Mathieu-Daudé, 2020/09/07
- Re: [PULL 3/5] softmmu/cpus: Only set parallel_cpus for SMP, Claudio Fontana, 2020/09/08
- Re: [PULL 3/5] softmmu/cpus: Only set parallel_cpus for SMP, Philippe Mathieu-Daudé, 2020/09/08
- Re: [PULL 3/5] softmmu/cpus: Only set parallel_cpus for SMP, Claudio Fontana, 2020/09/10
- Re: [PULL 3/5] softmmu/cpus: Only set parallel_cpus for SMP,
Philippe Mathieu-Daudé <=
[PULL 2/5] tcg: Fix tcg gen for vectorized absolute value, Richard Henderson, 2020/09/03
[PULL 4/5] tcg: Eliminate one store for in-place 128-bit dup_mem, Richard Henderson, 2020/09/03
[PULL 5/5] tcg: Implement 256-bit dup for tcg_gen_gvec_dup_mem, Richard Henderson, 2020/09/03
Re: [PULL 0/5] tcg patch queue, Peter Maydell, 2020/09/06