Re: [PATCH V2 1/2] powerpc, perf: Ignore separate BHRB privilege state filter request

[Anshuman Khandual <khandual@linux.vnet.ibm.com>]

On 06/06/2013 10:26 AM, Michael Neuling wrote:
> Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote:
> 
>> Completely ignore BHRB privilege state filter request as we are
>> already configuring that with privilege state filtering attribute
>> for the accompanying PMU event. This would help achieve cleaner
>> user space interaction for BHRB.
>>
>> This patch fixes a situation like this
>>
>> Before patch:-
>> ------------
>> ./perf record -j any -e branch-misses:k ls
>> Error:
>> The sys_perf_event_open() syscall returned with 95 (Operation not supported) for event (branch-misses:k).
>> /bin/dmesg may provide additional information.
>> No CONFIG_PERF_EVENTS=y kernel support configured?
>>
>> Here 'perf record' actually copies over ':k' filter request into BHRB
>> privilege state filter config and our previous check in kernel would
>> fail that.
>>
>> After patch:-
>> -------------
>> ./perf record -j any -e branch-misses:k ls
>> perf  perf.data  perf.data.old  test-mmap-ring
>> [ perf record: Woken up 1 times to write data ]
>> [ perf record: Captured and wrote 0.002 MB perf.data (~102 samples) ]
>>
>> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/perf/power8-pmu.c | 12 ++++--------
>>  1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
>> index f7d1c4f..3a58416 100644
>> --- a/arch/powerpc/perf/power8-pmu.c
>> +++ b/arch/powerpc/perf/power8-pmu.c
>> @@ -525,16 +525,12 @@ static u64 power8_bhrb_filter_map(u64 branch_sample_type)
>>  	u64 pmu_bhrb_filter = 0;
>>  	u64 br_privilege = branch_sample_type & ONLY_PLM;
>>  
>> -	/* BHRB and regular PMU events share the same prvillege state
>> +	/* BHRB and regular PMU events share the same privilege state
>>  	 * filter configuration. BHRB is always recorded along with a
>> -	 * regular PMU event. So privilege state filter criteria for BHRB
>> -	 * and the companion PMU events has to be the same. As a default
>> -	 * "perf record" tool sets all privillege bits ON when no filter
>> -	 * criteria is provided in the command line. So as along as all
>> -	 * privillege bits are ON or they are OFF, we are good to go.
>> +	 * regular PMU event. As the privilege state filter is handled
>> +	 * in the basic PMC configuration of the accompanying regular
>> +	 * PMU event, we ignore any separate BHRB specific request.
>>  	 */
>> -	if ((br_privilege != 7) && (br_privilege != 0))
>> -		return -1;
> 
> br_privilege is now unused which causes this compile error.
> 
> arch/powerpc/perf/power8-pmu.c:526:6: error: unused variable ‘br_privilege’ [-Werror=unused-variable]
> 
> I assume since you didn't compile test this code, you also didn't
> runtime test it either?!!?!?!?


Always compile with the cross compiler "ARCH=powerpc CROSS_COMPILE=powerpc64-linux-gnu- make"
I copied over arch/powerpc/config/pseries_defconfig and compiled it. Some how it missed
to be a 64 bit kernel config which resulted in shutting down compilation of arch/powerpc/perf/power8-pmu.c
file. So never encountered this compiler warning message before. Will fix this.

Regards
Anshuman