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

[Falcon, Thomas]

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.

Thanks,
Tom

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