Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1158445 > unrolled thread

Re: [RFC PATCH] perf: honoring cpuid for number of fixed counters

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH] perf: honoring cpuid for number of fixed counters Imre Palik <imrep.amz@gmail.com> - 2015-06-04 12:40 +0200
    Re: [RFC PATCH] perf: honoring cpuid for number of fixed counters Peter Zijlstra <peterz@infradead.org> - 2015-06-04 13:50 +0200

#1158445 — Re: [RFC PATCH] perf: honoring cpuid for number of fixed counters

On 06/03/15 10:36, Peter Zijlstra wrote:
> On Wed, Jun 03, 2015 at 10:03:48AM +0200, Imre Palik wrote:
>> From: "Palik, Imre" <imrep@amazon.de>
>>
>> perf doesn't seem to honor the number of fixed counters specified by cpuid
>> leaf 0xa.  It always assume that intel CPUs have at least 3 fixed counters.
>>
>> So if some of the fixed counters are masked out by the hypervisor, it still
>> tries to check/set them.  This is good for testing the masking code in the
>> hypervisor, but not so nice otherwise.
>>
>> This patch makes perf pehave somewhat nicer when the number of fixed
>> counters is less than three.
> 
>> @@ -3042,13 +3042,6 @@ __init int intel_pmu_init(void)
>>  
>>  	x86_pmu.max_pebs_events		= min_t(unsigned, MAX_PEBS_EVENTS, x86_pmu.num_counters);
>>  
>> -	/*
>> -	 * Quirk: v2 perfmon does not report fixed-purpose events, so
>> -	 * assume at least 3 events:
>> -	 */
>> -	if (version > 1)
>> -		x86_pmu.num_counters_fixed = max((int)edx.split.num_counters_fixed, 3);
>> -
>>  	if (boot_cpu_has(X86_FEATURE_PDCM)) {
>>  		u64 capabilities;
> 
> So the problem is that there is real hardware out there that gets the
> CPUID stuff wrong, and this patch penalizes that by then not using the
> fixed counters.

I haven't thought about this.  Thanks.

> Further, the Intel Arch PerfMon v2 spec actually specifies there to be 3
> fixed function counters.
> 
> So anything that says it is v2+ and does not have the 3, is non
> compliant.
>
> I would suggest you go fix your hypervisor.

If I set up the hypervisor to advertise Arch PerfMon v1 (0 fixed counters), then  without my patch, perf still tries to use fixed counters.  So something is clearly broken here.

> Lacking that option; you could probe the MSRs to see if they're really
> there using wrmsr_safe() or something like that -- see
> check_hw_exists().

I'll send something along these lines soon.
--
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/

[toc] | [next] | [standalone]

#1158489

On Thu, Jun 04, 2015 at 12:35:08PM +0200, Imre Palik wrote:
> On 06/03/15 10:36, Peter Zijlstra wrote:
> > Further, the Intel Arch PerfMon v2 spec actually specifies there to be 3
> > fixed function counters.
> > 
> > So anything that says it is v2+ and does not have the 3, is non
> > compliant.
> >
> > I would suggest you go fix your hypervisor.
> 
> If I set up the hypervisor to advertise Arch PerfMon v1 (0 fixed
> counters), then  without my patch, perf still tries to use fixed
> counters.  So something is clearly broken here.

So the code you deleted does if (version > 1), and last I checked that
should return false if version == 1.

So please check what's happening there first.
--
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/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web