From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40781) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W1cqF-0000eA-G0 for qemu-devel@nongnu.org; Fri, 10 Jan 2014 09:13:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W1cq4-0002Ad-6G for qemu-devel@nongnu.org; Fri, 10 Jan 2014 09:13:43 -0500 Received: from mail-pd0-f176.google.com ([209.85.192.176]:48277) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W1cq3-0002AR-Tu for qemu-devel@nongnu.org; Fri, 10 Jan 2014 09:13:32 -0500 Received: by mail-pd0-f176.google.com with SMTP id r10so657967pdi.7 for ; Fri, 10 Jan 2014 06:13:31 -0800 (PST) Message-ID: <52D00002.7020208@ozlabs.ru> Date: Sat, 11 Jan 2014 01:13:22 +1100 From: Alexey Kardashevskiy MIME-Version: 1.0 References: <1389245648-10300-1-git-send-email-aik@ozlabs.ru> <87ppo0na45.fsf@pixel.localdomain> <52CF1ECC.3040208@ozlabs.ru> <52CF4F37.4050306@ozlabs.ru> <878uuof0pu.fsf@pixel.localdomain> <84B5F34E-6072-4E56-8493-AAC69B9F7DAE@suse.de> <52CFF8B8.7010605@ozlabs.ru> <03847015-B363-4234-BF34-BB034D0BBE08@suse.de> In-Reply-To: <03847015-B363-4234-BF34-BB034D0BBE08@suse.de> Content-Type: text/plain; charset=KOI8-R Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH v2] PPC: smp: autodetect numbers of threads per core List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Mike Day , Paul Mackerras , "qemu-ppc@nongnu.org" , "qemu-devel@nongnu.org" On 01/11/2014 01:00 AM, Alexander Graf wrote: > > On 10.01.2014, at 14:42, Alexey Kardashevskiy wrote: > >> On 01/11/2014 12:28 AM, Alexander Graf wrote: >>> >>> On 10.01.2014, at 14:03, Mike Day wrote: >>> >>>> >>>> Alexey Kardashevskiy 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