Re: [PATCH 4/8] perf/x86/intel: Fix redundant branch type check in intel_pmu_lbr_filter()

[Mi, Dapeng]

On 6/6/2026 2:28 AM, Falcon, Thomas wrote:
> On Fri, 2026-06-05 at 09:11 +0800, Dapeng Mi wrote:
>> In intel_pmu_lbr_filter(), the 'type' variable is bitwise ORed with
>> 'to_plm' (which contains X86_BR_USER and/or X86_BR_KERNEL bits). Because
>> of this, 'type' can never equal X86_BR_NONE (0) after the assignment.
>>
>> As a result, the subsequent check 'if (type == X86_BR_NONE)' is dead code
>> and the entries with X86_BR_NONE type would not be skipped eventually.
>>
>> Correct this by masking out the X86_BR_KERNEL and X86_BR_USER bits
>> before performing the X86_BR_NONE comparison.
>>
>> Cc: stable@xxxxxxxxxxxxxxx
>> Fixes: 47125db27e47 ("perf/x86/intel/lbr: Support Architectural LBR")
>> Signed-off-by: Dapeng Mi <dapeng1.mi@xxxxxxxxxxxxxxx>
>> ---
>>
>> Original patch link:
>> https://lore.kernel.org/all/20260414021440.928068-1-dapeng1.mi@xxxxxxxxxxxxxxx/
>>
>>  arch/x86/events/intel/lbr.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
>> index 72f2adcda7c6..16977e4c6f8a 100644
>> --- a/arch/x86/events/intel/lbr.c
>> +++ b/arch/x86/events/intel/lbr.c
>> @@ -1245,7 +1245,7 @@ intel_pmu_lbr_filter(struct cpu_hw_events *cpuc)
>>  		}
>>  
>>  		/* if type does not correspond, then discard */
>> -		if (type == X86_BR_NONE || (br_sel & type) != type) {
>> +		if ((type & ~X86_BR_PLM) == X86_BR_NONE || (br_sel & type) != type) {
> Looking at intel_pmu_lbr_filter...
>
> 	if (static_cpu_has(X86_FEATURE_ARCH_LBR) &&
> 	    type <= ARCH_LBR_BR_TYPE_KNOWN_MAX) {
> 		to_plm = kernel_ip(to) ? X86_BR_KERNEL : X86_BR_USER;
> 		type = arch_lbr_br_type_map[type] | to_plm;
> 	} else
> 		type = branch_type(from, to, cpuc->lbr_entries[i].abort);
>
> In the else case, it does look (branch_type -> get_branch_type) can return X86_BR_NONE without OR'ing it with X86_BR_KERNEL or X86_BR_USER, so the condition checking the type for X86_BR_NONE is not exactly "dead code."
>
> One example:
>
> static int get_branch_type(unsigned long from, unsigned long to, int abort,
> 			   bool fused, int *offset)
> {
> ...
> 	 * maybe zero if lbr did not fill up after a reset by the time
> 	 * we get a PMU interrupt
> 	 */
> 	if (from == 0 || to == 0)
> 		return X86_BR_NONE;
> ...
>
> Though in those cases, it doesn't seem like this change would make a difference. I guess it isn't clear to me what issue this change is fixing.

Yes, the "dead code" is not accurate. For hardware decoding (the if case),
the validation is some kind of dead code, but for the software decoding,
it's not. I would adjust the wording in next version. Thanks.


>
> Thanks,
> Tom
>
>>  			cpuc->lbr_entries[i].from = 0;
>>  			compress = true;
>>  		}