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

[Mi, Dapeng]

On 6/8/2026 9:56 AM, Mi, Dapeng wrote:
> 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.

Just look at the code again, the hardware decoding case would never return
X86_BR_NONE, so it looks this patch is unnecessary and would drop it in
next version. Thanks.


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