[Deepin-Kernel-SIG] [linux 6.6-y] [Upstream] [Alibaba] [ARM] perf vendor events: Remove UTF-8 characters from cmn.json#840
Conversation
[ Upstream commit 75aea4b ] A warning in intel_pmu_lbr_counters_reorder() may be triggered by below perf command. perf record -e "{cpu-clock,cycles/call-graph="lbr"/}" -- sleep 1 It's because the group is mistakenly treated as a branch counter group. The hw.flags of the leader are used to determine whether a group is a branch counters group. However, the hw.flags is only available for a hardware event. The field to store the flags is a union type. For a software event, it's a hrtimer. The corresponding bit may be set if the leader is a software event. For a branch counter group and other groups that have a group flag (e.g., topdown, PEBS counters snapshotting, and ACR), the leader must be a X86 event. Check the X86 event before checking the flag. The patch only fixes the issue for the branch counter group. The following patch will fix the other groups. There may be an alternative way to fix the issue by moving the hw.flags out of the union type. It should work for now. But it's still possible that the flags will be used by other types of events later. As long as that type of event is used as a leader, a similar issue will be triggered. So the alternative way is dropped. Fixes: 3374491 ("perf/x86/intel: Support branch counters logging") Closes: https://lore.kernel.org/lkml/20250412091423.1839809-1-luogengkun@huaweicloud.com/ Reported-by: Luo Gengkun <luogengkun@huaweicloud.com> Signed-off-by: Kan Liang <kan.liang@linux.intel.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Cc: stable@vger.kernel.org Link: https://lkml.kernel.org/r/20250424134718.311934-2-kan.liang@linux.intel.com [ Backport from v6.15 ] Suggested-by: Wentao Guan <guanwentao@uniontech.com> Signed-off-by: WangYuli <wangyuli@uniontech.com>
[ Upstream commit ef493f4 ] The BPF subsystem may capture LBR data on a counting event. However, the current implementation assumes that LBR can/should only be used with sampling events. For instance, retsnoop tool ([0]) makes an extensive use of this functionality and sets up perf event as follows: struct perf_event_attr attr; memset(&attr, 0, sizeof(attr)); attr.size = sizeof(attr); attr.type = PERF_TYPE_HARDWARE; attr.config = PERF_COUNT_HW_CPU_CYCLES; attr.sample_type = PERF_SAMPLE_BRANCH_STACK; attr.branch_sample_type = PERF_SAMPLE_BRANCH_KERNEL; To limit the LBR for a sampling event is to avoid unnecessary branch stack setup for a counting event in the sample read. Because LBR is only read in the sampling event's overflow. Although in most cases LBR is used in sampling, there is no HW limit to bind LBR to the sampling mode. Allow an LBR setup for a counting event unless in the sample read mode. Fixes: 85846b2 ("perf/x86: Add PERF_X86_EVENT_NEEDS_BRANCH_STACK flag") Closes: https://lore.kernel.org/lkml/20240905180055.1221620-1-andrii@kernel.org/ Reported-by: Andrii Nakryiko <andrii.nakryiko@gmail.com> Signed-off-by: Kan Liang <kan.liang@linux.intel.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Acked-by: Andrii Nakryiko <andrii@kernel.org> Tested-by: Andrii Nakryiko <andrii@kernel.org> Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20240909155848.326640-1-kan.liang@linux.intel.com [ Backport from v6.12 ] Suggested-by: Wentao Guan <guanwentao@uniontech.com> Signed-off-by: WangYuli <wangyuli@uniontech.com>
Reviewer's GuideThis PR removes non-ASCII characters from the CMN PMU JSON metadata to enforce US-ASCII encoding, backports and extends support for ARM CMN PMU events—including new event definitions and test entries—enhances the jevents parser to handle EventidCode and NodeType, and improves matching logic in the perf test suite and metric group code. ER Diagram: Attributes in PMU Event JSONerDiagram
PMUEventJSON {
string EventName PK "EventName from JSON"
string EventCode "Optional: Parsed as eventcode"
string ConfigCode "Optional: Parsed as configcode"
string EventidCode "Optional, new: Parsed as eventidcode"
string UMask "Optional"
string Invert "Optional"
string NodeType "Optional, new: Used in event string"
string BriefDescription "Description (encoding changed to US-ASCII)"
string Compat "Optional"
string Unit "Optional (e.g., 'arm_cmn' is a new unit value)"
string Topic "Optional"
string ExtSel "Optional"
string AnyThread "Optional"
string PortMask "Optional"
string EdgeDetect "Optional"
string SampleAfterValue "Optional"
}
Class Diagram: Conceptual Event Object in jevents.pyclassDiagram
class Event {
+str name
+str topic
+str compat
+str desc
+str long_desc
+str pmu
+str unit
+str perpkg
+str metric_name
+str metric_group
+str metric_expr
+int eventcode
+int configcode
+int eventidcode # new
+str event # constructed string
+__init__(jd: JSONData) # logic for eventidcode, NodeType, and event string construction
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes perf vendor event issues on ARM by removing non-ASCII UTF-8 characters in cmn.json and adapting matching logic for uncore PMU events. Key changes include:
- Updating matching logic in metricgroup.c to use pmu_uncore_identifier_match.
- Adjusting test event definitions and validations in pmu-events.c.
- Enhancing event string construction and unit mapping in jevents.py.
- Adding or updating JSON definitions for ARM CMN PMU events.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/perf/util/metricgroup.c | Updated the condition to use pmu_uncore_identifier_match. |
| tools/perf/tests/pmu-events.c | Modified matching_pmu values and added new test events for ARM CMN. |
| tools/perf/pmu-events/jevents.py | Added arm_cmn mapping and adjusted event string lookup logic. |
| tools/perf/pmu-events/empty-pmu-events.c | Introduced a new event definition for sys_cmn_pmu.hnf_cache_miss. |
| tools/perf/pmu-events/arch/test/test_soc/sys/uncore.json | Extended tests to include the new ARM CMN event. |
| tools/perf/pmu-events/arch/arm64/arm/cmn/sys/metric.json | Added new metric definitions for ARM CMN events. |
| tools/perf/pmu-events/arch/arm64/arm/cmn/sys/cmn.json | Added a new JSON file with UTF-8 characters removed from descriptions. |
Comments suppressed due to low confidence (2)
tools/perf/tests/pmu-events.c:262
- [nitpick] Please ensure the updated matching_pmu value 'uncore_sys_ccn_pmu4' correctly reflects the hardware identifier change for ARM events.
.matching_pmu = "uncore_sys_ccn_pmu4",
tools/perf/util/metricgroup.c:501
- The matching condition now uses pmu_uncore_identifier_match; please confirm that its behavior precisely aligns with the intended semantics compared to the previous strcmp approach.
if (!pmu->id || !pmu_uncore_identifier_match(pm->compat, pmu->id))
| .count = &matched_count, | ||
| }; | ||
|
|
||
| if (strcmp(pmu_name, test_event.matching_pmu)) { |
There was a problem hiding this comment.
[nitpick] The test uses strcmp for matching PMU names while production code now uses pmu_uncore_identifier_match; consider aligning the matching logic in tests for consistency.
| if (strcmp(pmu_name, test_event.matching_pmu)) { | |
| if (!pmu_uncore_identifier_match(pmu_name, test_event.matching_pmu)) { |
| extra_desc += ' (Must be precise)' if precise == '2' else (' (Precise ' | ||
| 'event)') | ||
| event = f'config={llx(configcode)}' if configcode is not None else f'event={llx(eventcode)}' | ||
| event = None |
There was a problem hiding this comment.
[nitpick] The conditional assignment of 'event' could be streamlined to eliminate the initial assignment to None; consider refactoring for enhanced clarity.
| event = None |
572e5a2 to
611408b
Compare
There was a problem hiding this comment.
Hey @Avenger-285714 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
[ Upstream commit 457caad ] cmn.json contains UTF-8 characters in brief description which could break the perf build on some distros. Fix this issue by removing the UTF-8 characters from cmn.json. without this fix: $find tools/perf/pmu-events/ -name "*.json" | xargs file -i | grep -v us-ascii tools/perf/pmu-events/arch/arm64/arm/cmn/sys/cmn.json: application/json; charset=utf-8 with it: $ file -i tools/perf/pmu-events/arch/arm64/arm/cmn/sys/cmn.json tools/perf/pmu-events/arch/arm64/arm/cmn/sys/cmn.json: text/plain; charset=us-ascii Fixes: 0b4de7b ("perf jevents: Add support for Arm CMN PMU aliasing") Reported-by: Arnaldo Carvalho de Melo <acme@kernel.com> Signed-off-by: Jing Zhang <renyu.zj@linux.alibaba.com> Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Heiko Carstens <hca@linux.ibm.com> Cc: Ian Rogers <irogers@google.com> Cc: Jing Zhang <renyu.zj@linux.alibaba.com> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Kajol Jain <kjain@linux.ibm.com> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com> Cc: Thomas Richter <tmricht@linux.ibm.com> Link: https://lore.kernel.org/r/1703138593-50486-1-git-send-email-renyu.zj@linux.alibaba.com Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> [ Backport from v6.8 ] Suggested-by: Wentao Guan <guanwentao@uniontech.com> Signed-off-by: WangYuli <wangyuli@uniontech.com>
611408b to
184c99c
Compare
deepin pr auto review关键摘要:
是否建议立即修改:
|
cmn.json contains UTF-8 characters in brief description which
could break the perf build on some distros.
Fix this issue by removing the UTF-8 characters from cmn.json.
without this fix:
$find tools/perf/pmu-events/ -name "*.json" | xargs file -i | grep -v us-ascii
tools/perf/pmu-events/arch/arm64/arm/cmn/sys/cmn.json: application/json; charset=utf-8
with it:
$ file -i tools/perf/pmu-events/arch/arm64/arm/cmn/sys/cmn.json
tools/perf/pmu-events/arch/arm64/arm/cmn/sys/cmn.json: text/plain; charset=us-ascii
Fixes: 0b4de7b ("perf jevents: Add support for Arm CMN PMU aliasing")
Reported-by: Arnaldo Carvalho de Melo acme@kernel.com
Signed-off-by: Jing Zhang renyu.zj@linux.alibaba.com
Cc: Adrian Hunter adrian.hunter@intel.com
Cc: Heiko Carstens hca@linux.ibm.com
Cc: Ian Rogers irogers@google.com
Cc: Jing Zhang renyu.zj@linux.alibaba.com
Cc: Jiri Olsa jolsa@kernel.org
Cc: Kajol Jain kjain@linux.ibm.com
Cc: Namhyung Kim namhyung@kernel.org
Cc: Sukadev Bhattiprolu sukadev@linux.vnet.ibm.com
Cc: Thomas Richter tmricht@linux.ibm.com
Link: https://lore.kernel.org/r/1703138593-50486-1-git-send-email-renyu.zj@linux.alibaba.com
Signed-off-by: Arnaldo Carvalho de Melo acme@redhat.com
[ Backport from v6.8 ]
Suggested-by: Wentao Guan guanwentao@uniontech.com
Signed-off-by: WangYuli wangyuli@uniontech.com
Fixes: #832
Summary by Sourcery
Enable and test support for Arm CMN uncore PMU events in perf by extending the JSON parser, adding new event definitions and tests, and updating related filtering logic.
New Features:
Enhancements:
Tests:
Chores: