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

[Alexander Graf]

On 10.01.2014, at 15:13, Alexey Kardashevskiy <address@hidden> wrote:

> On 01/11/2014 01:00 AM, Alexander Graf wrote:
>> 
>> On 10.01.2014, at 14:42, Alexey Kardashevskiy <address@hidden> wrote:
>> 
>>> On 01/11/2014 12:28 AM, Alexander Graf wrote:
>>>> 
>>>> On 10.01.2014, at 14:03, Mike Day <address@hidden> wrote:
>>>> 
>>>>> 
>>>>> Alexey Kardashevskiy <address@hidden> writes:
>>>>> 
>>>>>> On 01/10/2014 10:40 AM, Alexander Graf wrote:
>>>>>>> 
>>>>>> 
>>>>>>> What if we make the max thread count a property of our cpu
>>>>>>> class? The we
>>>>>> can add a threads=max option which will be identical between kvm
>>>>>> and tcg.
>>>>>> 
>>>>>> 
>>>>>> You lost me here :) Right now the sequence is: 1. smp_parse 2. 
>>>>>> config_accelerator 3. machine_init
>>>>>> 
>>>>>> I proposed 1. config_accelerator - reads max threads from KVM
>>>>>> (and initializes "host" type) 2. smp_parse - does the parsing
>>>>>> using smp_threads tweaked in 1) 3. machine_init - creates CPUs
>>>>>> which may or may be not "host".
>>>>> 
>>>>> The patch as it its now is very simple and well-contained. I
>>>>> wonder how much it would expand if we added a max thread count to
>>>>> the cpu class. It seems like the need for a max thread count is
>>>>> idiomatic to powerpc.
>>>> 
>>>> It's only ever useful on IBM POWER. Any other PowerPC system can 
>>>> partition vcpus by host threads, it's only IBM POWER hardware that's
>>>> as broken as it is.
>>>> 
>>> 
>>>> But really, what problem are you trying to solve here? Do you have
>>>> users that don't understand the constraints they have? You will
>>>> still have this even with this patch, as if you do threads=max as
>>>> default for KVM (which is a bad idea, because it diverges from TCG)
>>>> -smp 5 would still allocate 2 host cores on p7 and you're not
>>>> effectively using your resources.
>>> 
>>> I am not changing the default here. I am adding an ability to choose
>>> the maximum.
>>> 
>>> 
>>>> With the patch you're also not taking compat thread count into
>>>> account. A POWER7 compat system would still need to manually specify
>>>> threads=4, no?
>>> 
>>> Nope. I will need to smp_threads=min(smp_threads, 4) (and I do this in
>>> my patches which I think I already posted but I need to repost them)
>>> so if it is 1 by default, it will be still 1.
>> 
>> Bleks. So suddenly we have one piece of code overriding magic that
>> happens in another piece of code. This sounds like in 5 years from now
>> nobody will have a clue what's going on here anymore :).
> 
> 
> git blame will know.
> 
> 
>> Can't we determine the number of "default threads" at a common place,
>> preferably derived from cpu type?
> 
> 
> We can do anything. I asked how exactly as I really (really) do not
> understand the details.

If we can override smp_threads with the compat= option we should be able to do 
so based on the cpu class a user selects as well, no?

> 
> 
>> I do remember that Anthony wanted to introduce a concept for "CPU
>> sockets" once where you just plug in a 6-core p7 into a slot and that
>> automatically gives you 6x4 threads of the specific cpu type. That's
>> probably overkill for this scenario though :). Be aware that you're not
>> the first person thinking along these lines.
> 
> 
> Sure I am not.
> 
> 
>>>> I do see that the user experience is slightly suboptimal, but by 
>>>> creating this special case there's a good chance you're doing more
>>>> harm than good.
>>> 
>>> Again. I do not change the default. I add an additional option (which
>>> is false by default) to use the maximum of the CPU. What harm can it
>>> possibly make?
>>> 
>>> I am definitely missing your point, sorry.
>> 
> 
>> In which case would this help anyone? I'm serious, please describe
>> exactly the reason for this patch. I'm sure you had people ask you to
>> write it or it fixes a nuisance you have yourself.
> 
> 
> Right now by default people use 1 (one) threads from each core. -smp 8
> takes 8 CPU cores instead of 8 threads of one CPU. How is it good?

It's not good, it's consistent. A user gets exactly what he specified. If I am 
on p7, I specify threads=4 (or not, depending on how fast I need single thread 
performance to be).

> And I'll ask again - how can the user know the maximum number of threads
> per core otherwise than guessing it from the CPU model? Does any user space
> tool print it?

How does the maximum number help the user at all? If he is on p8 and can do 
SMT2 granularity over cores (which IIRC was the granularity hardware allows you 
to do) why would he want to use threads=8 which is the maximum he can use?


Alex