Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH] smp: autodetect numbers of threads per core

[Alexey Kardashevskiy]

On 12/17/2013 05:37 PM, Alexey Kardashevskiy wrote:
> On 12/04/2013 04:34 PM, Alexey Kardashevskiy wrote:
>> On 11/16/2013 03:58 AM, Alexey Kardashevskiy wrote:
>>> On 16.11.2013 0:15, Alexander Graf wrote:
>>>>
>>>>
>>>> Am 15.11.2013 um 00:12 schrieb Alexey Kardashevskiy <address@hidden>:
>>>>
>>>>> At the moment only a whole CPU core can be assigned to a KVM. Since
>>>>> POWER7/8 support several threads per core, we want all threads of a core
>>>>> to go to the same KVM so every time we run QEMU with -enable-kvm on
>>>>> POWER, we have to add -smp X,threads=(4|8)" (4 for POWER7 and
>>>>> 8 for POWER8).
>>>>>
>>>>> This patch tries to read smp_threads number from an accelerator and
>>>>> falls back to the default value (1) if the accelerator did not care
>>>>> to change it.
>>>>>
>>>>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
>>>>> ---
>>>>>
>>>>> (!!!)
>>>>>
>>>>> The usual question - what would be the normal way of doing this?
>>>>> What does this patch break? I cannot think of anything right now.
>>>>
>>>> Is this really what the user wants? On p7 you can run in no-smt, smt2
>>>> and smt4 mode. Today we simply default to no-smt. Changing defaults
>>>> is usually a bad thing.
>>>
>>>
>>> Defaulting to 1 thread on P7 is a bad thing (other threads stay unused -
>>> what is good about this?) and the only reason which I know why it is
>>> still threads=1 is that it is hard to make a patch for upstream to
>>> change this default.
>>>
>>>
>>> Paul, please, help.
>>
>>
>> Anyone, ping? Thanks.
> 
> 
> Everyone ignores me :(


Ping?


> 
> 
>>
>>
>>>> Alex
>>>>
>>>>>
>>>>>
>>>>> ---
>>>>> target-ppc/kvm.c |  6 ++++++
>>>>> vl.c             | 29 ++++++++++++++++-------------
>>>>> 2 files changed, 22 insertions(+), 13 deletions(-)
>>>>>
>>>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>>>>> index 10d0cd9..80c0386 100644
>>>>> --- a/target-ppc/kvm.c
>>>>> +++ b/target-ppc/kvm.c
>>>>> @@ -109,6 +109,12 @@ int kvm_arch_init(KVMState *s)
>>>>>                         "VM to stall at times!\n");
>>>>>     }
>>>>>
>>>>> +    if (!smp_threads) {
>>>>> +        smp_threads = cap_ppc_smt;
>>>>> +    } else {
>>>>> +        smp_threads = MIN(smp_threads, cap_ppc_smt);
>>>>> +    }
>>>>> +
>>>>>     kvm_ppc_register_host_cpu_type();
>>>>>
>>>>>     return 0;
>>>>> diff --git a/vl.c b/vl.c
>>>>> index 4ad15b8..97fa203 100644
>>>>> --- a/vl.c
>>>>> +++ b/vl.c
>>>>> @@ -210,7 +210,7 @@ int singlestep = 0;
>>>>> int smp_cpus = 1;
>>>>> int max_cpus = 0;
>>>>> int smp_cores = 1;
>>>>> -int smp_threads = 1;
>>>>> +int smp_threads = 0;
>>>>> #ifdef CONFIG_VNC
>>>>> const char *vnc_display;
>>>>> #endif
>>>>> @@ -1395,7 +1395,9 @@ static void smp_parse(QemuOpts *opts)
>>>>>         if (cpus == 0 || sockets == 0) {
>>>>>             sockets = sockets > 0 ? sockets : 1;
>>>>>             cores = cores > 0 ? cores : 1;
>>>>> -            threads = threads > 0 ? threads : 1;
>>>>> +            if (!threads) {
>>>>> +                threads = smp_threads > 0 ? smp_threads : 1;
>>>>> +            }
>>>>>             if (cpus == 0) {
>>>>>                 cpus = cores * threads * sockets;
>>>>>             }
>>>>> @@ -1413,7 +1415,8 @@ static void smp_parse(QemuOpts *opts)
>>>>>         smp_cpus = cpus;
>>>>>         smp_cores = cores > 0 ? cores : 1;
>>>>>         smp_threads = threads > 0 ? threads : 1;
>>>>> -
>>>>> +    } else if (!smp_threads) {
>>>>> +        smp_threads = 1;
>>>>>     }
>>>>>
>>>>>     if (max_cpus == 0) {
>>>>> @@ -3880,16 +3883,6 @@ int main(int argc, char **argv, char **envp)
>>>>>         data_dir[data_dir_idx++] = CONFIG_QEMU_DATADIR;
>>>>>     }
>>>>>
>>>>> -    smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
>>>>> -
>>>>> -    machine->max_cpus = machine->max_cpus ?: 1; /* Default to UP */
>>>>> -    if (smp_cpus > machine->max_cpus) {
>>>>> -        fprintf(stderr, "Number of SMP cpus requested (%d), exceeds max 
>>>>> cpus "
>>>>> -                "supported by machine `%s' (%d)\n", smp_cpus,  
>>>>> machine->name,
>>>>> -                machine->max_cpus);
>>>>> -        exit(1);
>>>>> -    }
>>>>> -
>>>>>     /*
>>>>>      * Get the default machine options from the machine if it is not 
>>>>> already
>>>>>      * specified either by the configuration file or by the command line.
>>>>> @@ -4039,6 +4032,16 @@ int main(int argc, char **argv, char **envp)
>>>>>
>>>>>     configure_accelerator();
>>>>>
>>>>> +    smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL));
>>>>> +
>>>>> +    machine->max_cpus = machine->max_cpus ?: 1; /* Default to UP */
>>>>> +    if (smp_cpus > machine->max_cpus) {
>>>>> +        fprintf(stderr, "Number of SMP cpus requested (%d), exceeds max 
>>>>> cpus "
>>>>> +                "supported by machine `%s' (%d)\n", smp_cpus,  
>>>>> machine->name,
>>>>> +                machine->max_cpus);
>>>>> +        exit(1);
>>>>> +    }
>>>>> +
>>>>>     if (!qtest_enabled() && qtest_chrdev) {
>>>>>         qtest_init();
>>>>>     }
>>>
>>>
>>
>>
> 
> 


-- 
Alexey