Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754578AbbEUAyZ (ORCPT ); Wed, 20 May 2015 20:54:25 -0400 Received: from www.linutronix.de ([62.245.132.108]:35899 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752819AbbEUAyX (ORCPT ); Wed, 20 May 2015 20:54:23 -0400 Date: Thu, 21 May 2015 02:54:26 +0200 (CEST) From: Thomas Gleixner To: Vikas Shivappa cc: Vikas Shivappa , x86@kernel.org, linux-kernel@vger.kernel.org, hpa@zytor.com, mingo@kernel.org, tj@kernel.org, peterz@infradead.org, Matt Fleming , "Auld, Will" , peter.zijlstra@intel.com, h.peter.anvin@intel.com, "Juvva, Kanaka D" , mtosatti@redhat.com Subject: Re: [PATCH 3/7] x86/intel_rdt: Add support for cache bit mask management In-Reply-To: Message-ID: References: <1431370976-31115-1-git-send-email-vikas.shivappa@linux.intel.com> <1431370976-31115-4-git-send-email-vikas.shivappa@linux.intel.com> User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4236 Lines: 120 On Wed, 20 May 2015, Thomas Gleixner wrote: > On Wed, 20 May 2015, Vikas Shivappa wrote: > > On Mon, 18 May 2015, Thomas Gleixner wrote: > > > > > On Mon, 18 May 2015, Vikas Shivappa wrote: > > > > On Fri, 15 May 2015, Thomas Gleixner wrote: > > > > > > +static inline bool intel_rdt_update_cpumask(int cpu) > > > > > > +{ > > > > > > + } > > > > > > > > > > You must be kidding. > > > > > > > > the rapl and cqm use similar code. You want me to keep a seperate package > > > > mask > > > > for this code which not would be that frequent at all ? > > > > > > You find for everything a place where you copied your stuff from > > > without thinking about it, right? > > > > > > Other people dessperately try to fix the cpu online times which are > > > more and more interesting the larger the systems become. So it might > > > be a good idea to come up with a proper fast implementation which can > > > be used everywhere instead of blindly copying code. > > > > Ok , i can try to do this as a seperate patch after the cache allocation to > > Hell no. We do preparatory patches first. I'm not believing in 'can > try' promises. > > > get a support for faster implementation for traversing package and cpus in the > > packages which can be used by everyone. we would need to start from scratch > > with having packagemask_t equivalent to cpumask_t. hope that is fair ? > > Yes, that's what I want to see. I was kidding. You need two functionalities: 1) A way to figure out whether you already have a CPU taking care of the package to which the newly online CPU belongs. That's something you need to track in your own code and I already gave you the 5 lines of code which can handle that. Remember? id = topology_physical_package_id(cpu); if (!__test_and_set_bit(id, &package_map)) { cpumask_set_cpu(cpu, &rdt_cpumask); cbm_update_msrs(cpu); } So you cannot add much infrastructure for that because you need to track your own state in the CAT relevant package bitmap. 2) A way to find out whether the to be offlined CPU is the last online CPU of a package. If it's not the last one, then you need a fast way to find the next online cpu which belongs to that package. If it's the last one you need to kill the cat. The information is already available, so it's not rocket science. And it takes maximal 7 lines of code to implement it. Q: Why 'maximal' 7? A: Because I'm a lazy bastard and cannot be bothered to figure out whether one of the lines is superfluous. Q: Why can't I be bothered? A: It's none of my problems and it actually does not matter much. Here is the pseudo code: do_magic_stuff(tmp, TCC, COM); clr(c, tmp); n = do_more_stuff(tmp); if (notavailable(n)) kill_the_cat(); else set(n, RCM); Hint: One of the NNN placeholders resolves to topology_core_cpumask() Now once you figured that out, you will notice that the above 5 lines of code to solve problem #1 can be simplified because you can avoid the package_map bitmap completely. Sorry, no pseudo code for this. But you get more than one hint this time: - Hint #1 still applies - The logic is very similar to the above #2 pseudo code - It takes maximal 6 lines of code to implement it There is one caveat: If Hint #1 cannot solve your problem, then you need to figure out why and then work with the people who are responsible for it to figure out how it can be resolved. A few general hints: - The line counts are neither including the conditionals which invoke that code nor the function body nor variable declarations. But they include braces, All I care about is the logic itself. See the pseudo code example above. - Please provide a solution for #2 and #1 before you bother me with another patch series. Thanks, tglx -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/