Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934685AbdC3TDV (ORCPT ); Thu, 30 Mar 2017 15:03:21 -0400 Received: from mga06.intel.com ([134.134.136.31]:62065 "EHLO mga06.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934372AbdC3TDU (ORCPT ); Thu, 30 Mar 2017 15:03:20 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,248,1486454400"; d="scan'208";a="72001567" Date: Thu, 30 Mar 2017 12:03:44 -0700 (PDT) From: Shivappa Vikas X-X-Sender: vikas@vshiva-Udesk To: Thomas Gleixner cc: Vikas Shivappa , vikas.shivappa@intel.com, linux-kernel@vger.kernel.org, x86@kernel.org, hpa@zytor.com, mingo@kernel.org, peterz@infradead.org, ravi.v.shankar@intel.com, tony.luck@intel.com, fenghua.yu@intel.com, andi.kleen@intel.com Subject: Re: [PATCH 5/5] x86/intel_rdt: hotcpu updates for RDT In-Reply-To: Message-ID: References: <1487360328-6768-1-git-send-email-vikas.shivappa@linux.intel.com> <1487360328-6768-6-git-send-email-vikas.shivappa@linux.intel.com> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1592 Lines: 51 On Wed, 1 Mar 2017, Thomas Gleixner wrote: >> WARN_ON(c->x86_cache_occ_scale != cqm_l3_scale); >> >> @@ -1585,12 +1580,17 @@ static int intel_cqm_cpu_starting(unsigned int cpu) >> >> static int intel_cqm_cpu_exit(unsigned int cpu) >> { >> + struct intel_pqr_state *state = &per_cpu(pqr_state, cpu); > > Can be this_cpu_ptr() because the callback is guaranteed to run on the > outgoing CPU. Will fix this. Assumed the calls are setup cache alloc way - cpuhp_setup_state(CPUHP_AP_ONLINE_DYN .. > >> int target; >> >> /* Is @cpu the current cqm reader for this package ? */ >> if (!cpumask_test_and_clear_cpu(cpu, &cqm_cpumask)) >> return 0; > > So if the CPU is not the current cqm reader then the per cpu state of this > CPU is left stale. Great improvement. > >> + state->rmid = 0; >> + state->rmid_usecnt = 0; >> + wrmsr(MSR_IA32_PQR_ASSOC, 0, state->closid); > > What clears state->closid? And what guarantees that state->rmid is not > updated before the CPU has really gone away? - The rdt code takes care of clearing closid state now. Will update the comment. - The cqm however was never writing a zero to PQR_ASSOC. So the update needs to be - to remove the state->closid = 0 from cqm code as the rdt code takes care of closid state in clear_closid() called from both offline and online cpu. And also write a rmid = 0 to PQR_ASSOC. We can integrate the two of these hot cpu calls(from cat and cqm) to write PQR only once. guess I can skip all of these and send it as part of cqm changes we planned anyways, because this is really a cqm change. Thanks, Vikas