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

[Alexander Graf]

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 :).

Can't we determine the number of "default threads" at a common place, 
preferably derived from cpu type?

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.

> 
>> 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.


Alex