RE: [PATCH 08/32] Define CONFIG_INTEL_RDT

From: "Yu, Fenghua" <fenghua.yu@intel.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@elte.hu>,
	"Anvin, H Peter" <h.peter.anvin@intel.com>,
	"Luck, Tony" <tony.luck@intel.com>, Tejun Heo <tj@kernel.org>,
	"Borislav Petkov" <bp@suse.de>,
	Stephane Eranian <eranian@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	"David Carrillo-Cisneros" <davidcc@google.com>,
	"Shankar, Ravi V" <ravi.v.shankar@intel.com>,
	Vikas Shivappa <vikas.shivappa@linux.intel.com>,
	"Prakhya, Sai Praneeth" <sai.praneeth.prakhya@intel.com>,
	linux-kernel <linux-kernel@vger.kernel.org>, x86 <x86@kernel.org>
Subject: RE: [PATCH 08/32] Define CONFIG_INTEL_RDT
Date: Wed, 13 Jul 2016 18:05:54 +0000	[thread overview]
Message-ID: <3E5A0FA7E9CA944F9D5414FEC6C712205DFD4AB3@ORSMSX106.amr.corp.intel.com> (raw)
In-Reply-To: <alpine.DEB.2.11.1607131223020.4083@nanos>

> From: Thomas Gleixner [mailto:tglx@linutronix.de]
> Sent: Wednesday, July 13, 2016 3:26 AM
> On Tue, 12 Jul 2016, Fenghua Yu wrote:
> 
> Subject: Define CONFIG_INTEL_RDT
> 
> That does not qualify as a proper patch subject
> 
> > From: Vikas Shivappa <vikas.shivappa@linux.intel.com>
> >
> > CONFIG_INTEL_RDT is defined.
> 
> That tells us what?
> 
> > --- a/arch/x86/include/asm/intel_rdt.h
> > +++ b/arch/x86/include/asm/intel_rdt.h
> > @@ -24,8 +24,16 @@ struct clos_cbm_table {
> >   * on scheduler hot path:
> >   * - This will stay as no-op unless we are running on an Intel SKU
> >   * which supports L3 cache allocation.
> > + * - When support is present and enabled, does not do any
> > + * IA32_PQR_MSR writes until the user starts really using the feature
> > + * ie creates a rdtgroup directory and assigns a cache_mask thats
> > + * different from the root rdtgroup's cache_mask.
> >   * - Caches the per cpu CLOSid values and does the MSR write only
> > - * when a task with a different CLOSid is scheduled in.
> > + * when a task with a different CLOSid is scheduled in. That
> > + * means the task belongs to a different rdtgroup.
> > + * - Closids are allocated so that different rdtgroup directories
> > + * with same cache_mask gets the same CLOSid. This minimizes CLOSids
> > + * used and reduces MSR write frequency.
> 
> How is this and the following changes related to $subject ?

No, this piece of code is not related to $subject.

Here is why this patch behaves like this:

This patch and actually first 12 patches are directly from last year's cgroup base CAT patch
series. The last year's patch series had gone 16 versions already. Because
the first 12 patches have been reviewed many times, we keep them untouched
(except removing cgroup code in patch 8 and some unused cdp code in patch 11)
and release other patches on top of the first 12 patches.

I fully agree this patch should be split if we want to have a good overall
patch series.

Thanks.

-Fenghua