From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chao Peng Subject: Re: [PATCH v7 03/14] x86: detect and initialize Intel CAT feature Date: Tue, 19 May 2015 17:33:49 +0800 Message-ID: <20150519093349.GF8239@pengc-linux.bj.intel.com> References: <1431075415-21917-1-git-send-email-chao.p.peng@linux.intel.com> <1431075415-21917-4-git-send-email-chao.p.peng@linux.intel.com> <555A065F020000780007B2DE@mail.emea.novell.com> <20150519074019.GD8239@pengc-linux.bj.intel.com> <555B137F020000780007B7AC@mail.emea.novell.com> Reply-To: Chao Peng Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <555B137F020000780007B7AC@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: wei.liu2@citrix.com, Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, Ian.Jackson@eu.citrix.com, xen-devel@lists.xen.org, will.auld@intel.com, keir@xen.org, dgdegra@tycho.nsa.gov List-Id: xen-devel@lists.xenproject.org On Tue, May 19, 2015 at 09:42:07AM +0100, Jan Beulich wrote: > >>> On 19.05.15 at 09:40, wrote: > > On Mon, May 18, 2015 at 02:33:51PM +0100, Jan Beulich wrote: > >> >>> On 08.05.15 at 10:56, wrote: > >> > +static unsigned long *__read_mostly cat_socket_init_bitmap; > >> > +static unsigned long *__read_mostly cat_socket_enable_bitmap; > >> > >> Didn't we agree to fold these two into one? Apart from that the > >> _bitmap name tag doesn't seem very useful... > > > > Looks like this? > > > > static unsigned long *__read_mostly cat_socket_init, > > *__read_mostly cat_socket_enable; > > Yes, except that you will want to drop one (or make clear why you > need both). As said in the previous version, cat_socket_enable is the actual CAT enabled status. And cat_socket_init is used for performance optimization purpose only. It indicate if the socket has been initialized, so that initialization happens only once for each socket, but not __cpus_in_socket__ times. There are three possibilities: 1) Not initialized; 2) Initialized, CAT disabled; 3) Initialized, CAT enabled; So it's not possible to use only one bit to represent three values. > > >> > static int cpu_callback( > >> > struct notifier_block *nfb, unsigned long action, void *hcpu) > >> > { > >> > if ( action == CPU_STARTING ) > >> > psr_cpu_init(); > >> > + else if ( action == CPU_DYING ) > >> > + psr_cpu_fini(); > >> > >> Are these the right notifiers for doing things involving memory > >> allocation / freeing? > > > > psr_cpu_fini() does not really have memory allocation/freeing but > > psr_cpu_init() does have. > > Hmm, wait - where did I see this allocation? Looking again, I don't see > it now. But if there was one, then surely it would be wrong for _fini > to not free it. > > > While one thing to clarify is: psr_cpu_init() will not propagate > > the error even when the memory allocation fails(instead it disables > > the CAT on the socket). > > > > If there is still problem then perhaps I need change CPU_STARTING back > > to CPU_ONLINE and make on_selected_cpus() call on target cpu. > > No, if there was any allocation needed, you should do the allocation > in CPU_UP_PREPARE and the initialization in CPU_STARTING. But it > looks like this is moot now anyway. As I will add allocation in patch 4, so I will move psr_cpu_init() to CPU_UP_PREPARE(As the allocation length is based on the cpuid result, so the initialization will also be done here), but still need to move psr_cpu_fini(which will add the freeing code) to CPU_DOWN_PREPARE? Chao