Re: [PATCH V8 09/10] powerpc, perf: Enable privilege mode SW branch filters

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

On 06/11/2015 06:49 AM, Daniel Axtens wrote:
>> 	if (sw_filter & PERF_SAMPLE_BRANCH_PLM_ALL) {
>> +		flag = false;
> Would it be possible to use a more meaningful name than flag? Perhaps
> indicating what is it flagging?

sure, will change it with "select_branch"

>> +
>> +		if (sw_filter & PERF_SAMPLE_BRANCH_USER) {
>> +			if (to_plm == POWER_ADDR_USER)
>> +				flag = true;
>> +		}
>> +
>> +		if (sw_filter & PERF_SAMPLE_BRANCH_KERNEL) {
>> +			if (to_plm == POWER_ADDR_KERNEL)
>> +				flag = true;
>> +		}
>> +
>> +		if (sw_filter & PERF_SAMPLE_BRANCH_HV) {
>> +			if (cpu_has_feature(CPU_FTR_HVMODE)) {
>> +				if (to_plm == POWER_ADDR_KERNEL)
>> +					flag = true;
>> +			}
>> +		}
> 
> Is there any reason these are nested ifs rather than &&s?

No reason as such, will change it.

> 
>> +
>> +		if (!flag)
>> +			return false;
>> +	}
>> +
> 
>> @@ -700,7 +710,6 @@ static u64 power8_bhrb_filter_map(u64 branch_sample_type, u64 *bhrb_filter)
>>  			if (branch_sample_type) {
>>  				/* Multiple filters will be processed in SW */
>>  				pmu_bhrb_filter = 0;
>> -				*bhrb_filter = 0;
>>  				return pmu_bhrb_filter;
>>  			} else {
>>  				/* Individual filter will be processed in HW */
> What's the justification for the removal of this line? You added it in
> the previous patch...

Previously PMU passed the entire branch processing to SW by indicating
0 in the bhrb_filter mask although it was handling the privilege level
requests received from the normal PMU event. SW just ignored privilege
level requests while figuring out what other filters which need to be
processed for each captured branch.

Now that we support privilege level SW branch filters, PMU needs to
exclusively inform SW about it, so that SW does not do the processing
itself assuming its not already taken care of. That is the reason why
we removed the above statement and added this code block here instead.

        if (branch_sample_type & PERF_SAMPLE_BRANCH_USER)
                *bhrb_filter |= PERF_SAMPLE_BRANCH_USER;

        if (branch_sample_type & PERF_SAMPLE_BRANCH_KERNEL)
                *bhrb_filter |= PERF_SAMPLE_BRANCH_KERNEL;

        if (branch_sample_type & PERF_SAMPLE_BRANCH_HV)
                *bhrb_filter |= PERF_SAMPLE_BRANCH_HV;