From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1E1A878F2E for ; Tue, 14 Apr 2026 03:00:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.12 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776135630; cv=none; b=Er0sgVWukpeXq45q/KO+feBCevPFMnlolJ3ocP5jmX2Km+MVZG+PZnVMJbqZhhhUH6junrd22h4EphKxubNAA93laqccwGIhHQWNhfxcDryn4SSW7AgtJ3BdBSlkONI0lys/N9EgtJ3gNq9e+sA/gG6HEqtvkknLpYCEfzU16uA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1776135630; c=relaxed/simple; bh=snEL+u1iXKB5S1S+Xa0IgVvXR72Ovnt2OhYzwlBBdtE=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=SkhqFG4UnxbVbPwGqOMtlaOURX/Py0II9MCtU+zbDsCdj9uvWWE6FSvxKP/AWBdnDNbMHakrELxaMh8mzKIADyO3INC4Pcq2yIhxckC86XHUwFU5Bxfi+lWl7mBKRUmi3nsOTnj2mBBpxH9+uKUeLbq04Anc9/KuS/rU7rqEV2s= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com; spf=pass smtp.mailfrom=linux.intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=Dr7lmbbt; arc=none smtp.client-ip=192.198.163.12 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="Dr7lmbbt" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1776135629; x=1807671629; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=snEL+u1iXKB5S1S+Xa0IgVvXR72Ovnt2OhYzwlBBdtE=; b=Dr7lmbbtIVJRPaZseKdFdViNEX+8igaXCOoTlqCqSkWgrZpoSeBMZo+8 vBmVSvAMh9lC7whxx4C3jAsZPnRqvn6QfGGMWXmaCKG+SSQdnu8rOMMTq a9vSgrZ8QaIxQDsGN0NZqRwOqjTZL5EHm7ZtjahoTe3dVQgyAdIxL86fR 7s0h9bx+/4h+fExrdF7oCDksDyOwM3x7ExyMHjqa37jd5fLXaRtv0B9cb nRgYvHfPdH9gl/GcYtZCmW8DkYkU3Z+la2/DXuFXdPRNhsrrY871BZXB9 sS7PfjMBjnnc8NIe7lMKW2vVbXtmlaO0HKpPG6w5HqEY91QcwJ986VTau g==; X-CSE-ConnectionGUID: 6tBZOYgsQ1CcJk7c1n64EQ== X-CSE-MsgGUID: O1IvNYyoQ8+gbN4BAPME3g== X-IronPort-AV: E=McAfee;i="6800,10657,11758"; a="80968089" X-IronPort-AV: E=Sophos;i="6.23,178,1770624000"; d="scan'208";a="80968089" Received: from fmviesa005.fm.intel.com ([10.60.135.145]) by fmvoesa106.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Apr 2026 20:00:28 -0700 X-CSE-ConnectionGUID: VvDCiJ0FTcmhjveoTx049A== X-CSE-MsgGUID: zLDT/+htTUuT+2/W7UAGHg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.23,178,1770624000"; d="scan'208";a="234895467" Received: from unknown (HELO [10.238.1.30]) ([10.238.1.30]) by fmviesa005-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 13 Apr 2026 20:00:28 -0700 Message-ID: <133ce7fc-3dcc-4e34-8101-b501107d4b5c@linux.intel.com> Date: Tue, 14 Apr 2026 11:00:26 +0800 Precedence: bulk X-Mailing-List: sashiko@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 1/2] perf/x86/intel: Fix redundant branch type check in intel_pmu_lbr_filter() To: sashiko@lists.linux.dev Cc: linux-perf-users@vger.kernel.org References: <20260414021440.928068-1-dapeng1.mi@linux.intel.com> <20260414025359.E9F0FC2BCAF@smtp.kernel.org> Content-Language: en-US From: "Mi, Dapeng" In-Reply-To: <20260414025359.E9F0FC2BCAF@smtp.kernel.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 4/14/2026 10:53 AM, sashiko-bot@kernel.org wrote: > Sashiko AI review found 1 potential issue(s): > - [Medium] Meaningful code and commit message mismatch due to a false premise > -- > > commit 98c2839085a2b609f94d9f3f338217d29525fd08 > Author: Dapeng Mi > > perf/x86/intel: Fix redundant branch type check in intel_pmu_lbr_filter() > > This commit modifies the branch filter logic to mask out privilege level > bits before checking if a branch type is X86_BR_NONE. > >> 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. > Is this description entirely accurate? > > Looking at intel_pmu_lbr_filter(), the bitwise OR with to_plm only happens > in the X86_FEATURE_ARCH_LBR path: > > 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 software fallback path, type is assigned the result of branch_type(). > If branch_type() cannot decode the branch and returns X86_BR_NONE (0), > wouldn't type evaluate to exactly X86_BR_NONE without any to_plm bits set? No, the branch_type() would bitwise ored the to_plm at last as below code shows. ``` /*      * branch priv level determined by target as      * is done by HW when LBR_SELECT is implemented      */     if (ret != X86_BR_NONE)         ret |= to_plm; ``` Thanks. > >> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c >> index 72f2adcda7c68..16977e4c6f8a5 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) { > While masking out ~X86_BR_PLM doesn't appear to break the software fallback > behavior since (0 & ~X86_BR_PLM) == 0, the original check for X86_BR_NONE > appears to have been functional and necessary for that path. > > Could the commit message be updated to clarify that the check was not actually > dead code for the software fallback path? >