On 01/11/2014 01:00 AM, Alexander Graf wrote: > > On 10.01.2014, at 14:42, Alexey Kardashevskiy <a...@ozlabs.ru> wrote: > >> On 01/11/2014 12:28 AM, Alexander Graf wrote: >>> >>> On 10.01.2014, at 14:03, Mike Day <ncm...@ncultra.org> wrote: >>> >>>> >>>> Alexey Kardashevskiy <a...@ozlabs.ru> 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. > 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? 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? -- Alexey
