Re: [PATCH 5/7] x86/intel_rdt: Software Cache for IA32_PQR_MSR

From: Thomas Gleixner <tglx@linutronix.de>
To: Vikas Shivappa <vikas.shivappa@intel.com>
Cc: Vikas Shivappa <vikas.shivappa@linux.intel.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org, hpa@zytor.com,
	mingo@kernel.org, tj@kernel.org, peterz@infradead.org,
	Matt Fleming <matt.fleming@intel.com>,
	"Auld, Will" <will.auld@intel.com>,
	peter.zijlstra@intel.com, h.peter.anvin@intel.com, "Juvva,
	Kanaka D" <kanaka.d.juvva@intel.com>,
	mtosatti@redhat.com
Subject: Re: [PATCH 5/7] x86/intel_rdt: Software Cache for IA32_PQR_MSR
Date: Wed, 20 May 2015 23:14:26 +0200 (CEST)	[thread overview]
Message-ID: <alpine.DEB.2.11.1505202306070.4225@nanos> (raw)
In-Reply-To: <alpine.DEB.2.10.1505201218520.6658@vshiva-Udesk>

On Wed, 20 May 2015, Vikas Shivappa wrote:
> On Wed, 20 May 2015, Thomas Gleixner wrote:
> 
> > On Wed, 20 May 2015, Vikas Shivappa wrote:
> > > On Fri, 15 May 2015, Thomas Gleixner wrote:
> > > > > -static DEFINE_PER_CPU(struct intel_cqm_state, cqm_state);
> > > > > +DEFINE_PER_CPU(struct intel_pqr_state, pqr_state);
> > > > 
> > > > With CONFIG_PERF=n and CONFIG_CGROUP_RDT=y the linker will fail.
> > > 
> > > copy from Makefile below -
> > > obj-$(CONFIG_CPU_SUP_INTEL)             += perf_event_intel_rapl.o
> > > perf_event_intel_cqm.o
> > > 
> > > should work with CONFIG_PERF_EVENTS=n and CGROUP_RDT=y ?
> > 
> > Groan. Did you try to compile it? Certainly not.
> 
> of course I did compile successfully after I changed PERF_EVENTS=n and RDT=y
> in the .config.
> sorry, my reason was wrong though.
> 
> What I had not noticed was the .config is simply added to enabling
> CONFIG_PERF_EVENTS=y even though i disable it by editing .config.
> Thats because x86 is selecting it.
> So PERF_EVENTS cant be disabled on x86 and RDT is only in x86 && SUP_INTEL.
> 
> How did you configure CONFIG_PERF_EVENTS=n and CGROUP_RDT=y and see the error

I did not compile it. I looked at the config switches. And my two
observations still stand:

1) It will fail with CONFIG_PERF_EVENTS=n and CGROUP_RDT=y

2) perf_event_intel_rapl.o depends on CONFIG_PERF_EVENTS=y

It's up to you to provide me a proper reason why it will be always
built. I'm reviewing code and its not my job to figure out why
something magically works. 

It's your job to provide proper answers to my review observations. And
your answer to my observation #1 definitely does not fall into that
category:

> > > copy from Makefile below -
> > > obj-$(CONFIG_CPU_SUP_INTEL)             += perf_event_intel_rapl.o
> > > perf_event_intel_cqm.o
> > > 
> > > should work with CONFIG_PERF_EVENTS=n and CGROUP_RDT=y ?

Then I told you that this is crap, because of #2

So now you gave me an explanation WHY it works magically:

> So PERF_EVENTS cant be disabled on x86 and RDT is only in x86 && SUP_INTEL.

That should have been your answer to #1, right?

Thanks,

	tglx