From: kan.liang@linux.intel.com
To: peterz@infradead.org, mingo@redhat.com, namhyung@kernel.org,
irogers@google.com, mark.rutland@arm.com,
linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org
Cc: eranian@google.com, ctshao@google.com, tmricht@linux.ibm.com,
Kan Liang <kan.liang@linux.intel.com>
Subject: [RFC PATCH 01/15] perf: Fix the throttle logic for a group
Date: Tue, 6 May 2025 09:47:26 -0700 [thread overview]
Message-ID: <20250506164740.1317237-2-kan.liang@linux.intel.com> (raw)
In-Reply-To: <20250506164740.1317237-1-kan.liang@linux.intel.com>
From: Kan Liang <kan.liang@linux.intel.com>
The current throttle logic doesn't work well with a group, e.g., the
following sampling-read case.
$ perf record -e "{cycles,cycles}:S" ...
$ perf report -D | grep THROTTLE | tail -2
THROTTLE events: 426 ( 9.0%)
UNTHROTTLE events: 425 ( 9.0%)
$ perf report -D | grep PERF_RECORD_SAMPLE -a4 | tail -n 5
0 1020120874009167 0x74970 [0x68]: PERF_RECORD_SAMPLE(IP, 0x1):
... sample_read:
.... group nr 2
..... id 0000000000000327, value 000000000cbb993a, lost 0
..... id 0000000000000328, value 00000002211c26df, lost 0
The second cycles event has a much larger value than the first cycles
event in the same group.
The current throttle logic in the generic code only logs the THROTTLE
event. It relies on the specific driver implementation to disable
events. However, for all ARCHs, the implementation is similar. It only
disable the event, rather than the group.
The logic to disable the group should be generic for all ARCHs. Add the
logic in the generic code. The following patch will remove the buggy
driver-specific implementation.
The throttle only happens when an event is overflowed. Stop the entire
group when any event in the group triggers the throttle. Set the
MAX_INTERRUPTS to the leader event to indicate the group is throttled.
The unthrottled could happen in 3 places.
- event/group sched. All events in the group are scheduled one by one.
All of them will be unthrottled eventually. Nothing needs to be
changed.
- The perf_adjust_freq_unthr_events for each tick. Needs to restart the
group altogether.
- The __perf_event_period(). The whole group needs to be restarted
altogether as well.
With the fix,
$ sudo perf report -D | grep PERF_RECORD_SAMPLE -a4 | tail -n 5
0 3573470770332 0x12f5f8 [0x70]: PERF_RECORD_SAMPLE(IP, 0x2):
... sample_read:
.... group nr 2
..... id 0000000000000a28, value 00000004fd3dfd8f, lost 0
..... id 0000000000000a29, value 00000004fd3dfd8f, lost 0
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---
kernel/events/core.c | 55 +++++++++++++++++++++++++++++++++-----------
1 file changed, 41 insertions(+), 14 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index a84abc2b7f20..eb0dc871f4f1 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2734,6 +2734,38 @@ void perf_event_disable_inatomic(struct perf_event *event)
static void perf_log_throttle(struct perf_event *event, int enable);
static void perf_log_itrace_start(struct perf_event *event);
+static void perf_event_group_unthrottle(struct perf_event *event, bool start_event)
+{
+ struct perf_event *leader = event->group_leader;
+ struct perf_event *sibling;
+
+ if (leader != event || start_event)
+ leader->pmu->start(leader, 0);
+ leader->hw.interrupts = 0;
+
+ for_each_sibling_event(sibling, leader) {
+ if (sibling != event || start_event)
+ sibling->pmu->start(sibling, 0);
+ sibling->hw.interrupts = 0;
+ }
+
+ perf_log_throttle(leader, 1);
+}
+
+static void perf_event_group_throttle(struct perf_event *event)
+{
+ struct perf_event *leader = event->group_leader;
+ struct perf_event *sibling;
+
+ leader->hw.interrupts = MAX_INTERRUPTS;
+ leader->pmu->stop(leader, 0);
+
+ for_each_sibling_event(sibling, leader)
+ sibling->pmu->stop(sibling, 0);
+
+ perf_log_throttle(leader, 0);
+}
+
static int
event_sched_in(struct perf_event *event, struct perf_event_context *ctx)
{
@@ -4389,10 +4421,8 @@ static void perf_adjust_freq_unthr_events(struct list_head *event_list)
hwc = &event->hw;
if (hwc->interrupts == MAX_INTERRUPTS) {
- hwc->interrupts = 0;
- perf_log_throttle(event, 1);
- if (!event->attr.freq || !event->attr.sample_freq)
- event->pmu->start(event, 0);
+ perf_event_group_unthrottle(event,
+ !event->attr.freq || !event->attr.sample_freq);
}
if (!event->attr.freq || !event->attr.sample_freq)
@@ -6421,14 +6451,6 @@ static void __perf_event_period(struct perf_event *event,
active = (event->state == PERF_EVENT_STATE_ACTIVE);
if (active) {
perf_pmu_disable(event->pmu);
- /*
- * We could be throttled; unthrottle now to avoid the tick
- * trying to unthrottle while we already re-started the event.
- */
- if (event->hw.interrupts == MAX_INTERRUPTS) {
- event->hw.interrupts = 0;
- perf_log_throttle(event, 1);
- }
event->pmu->stop(event, PERF_EF_UPDATE);
}
@@ -6436,6 +6458,12 @@ static void __perf_event_period(struct perf_event *event,
if (active) {
event->pmu->start(event, PERF_EF_RELOAD);
+ /*
+ * We could be throttled; unthrottle now to avoid the tick
+ * trying to unthrottle while we already re-started the event.
+ */
+ if (event->group_leader->hw.interrupts == MAX_INTERRUPTS)
+ perf_event_group_unthrottle(event, false);
perf_pmu_enable(event->pmu);
}
}
@@ -10326,8 +10354,7 @@ __perf_event_account_interrupt(struct perf_event *event, int throttle)
if (unlikely(throttle && hwc->interrupts >= max_samples_per_tick)) {
__this_cpu_inc(perf_throttled_count);
tick_dep_set_cpu(smp_processor_id(), TICK_DEP_BIT_PERF_EVENTS);
- hwc->interrupts = MAX_INTERRUPTS;
- perf_log_throttle(event, 0);
+ perf_event_group_throttle(event);
ret = 1;
}
--
2.38.1
next prev parent reply other threads:[~2025-05-06 16:48 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-06 16:47 [RFC PATCH 00/15] perf: Fix the throttle logic for group kan.liang
2025-05-06 16:47 ` kan.liang [this message]
2025-05-07 16:52 ` [RFC PATCH 01/15] perf: Fix the throttle logic for a group Ian Rogers
2025-05-07 19:00 ` Liang, Kan
2025-05-13 9:41 ` Peter Zijlstra
2025-05-13 9:45 ` Peter Zijlstra
2025-05-13 14:47 ` Liang, Kan
2025-05-06 16:47 ` [RFC PATCH 02/15] perf/x86/intel: Remove driver-specific throttle support kan.liang
2025-05-06 16:47 ` [RFC PATCH 03/15] perf/x86/amd: " kan.liang
2025-05-13 8:44 ` Ravi Bangoria
2025-05-06 16:47 ` [RFC PATCH 04/15] perf/x86/zhaoxin: " kan.liang
2025-05-06 16:47 ` [RFC PATCH 05/15] powerpc/perf: " kan.liang
2025-05-06 16:47 ` [RFC PATCH 06/15] s390/perf: " kan.liang
2025-05-06 16:47 ` [RFC PATCH 07/15] perf/arm: " kan.liang
2025-05-06 16:47 ` [RFC PATCH 08/15] perf/apple_m1: " kan.liang
2025-05-06 16:47 ` [RFC PATCH 09/15] alpha/perf: " kan.liang
2025-05-06 16:47 ` [RFC PATCH 10/15] arc/perf: " kan.liang
2025-05-06 16:47 ` [RFC PATCH 11/15] csky/perf: " kan.liang
2025-05-06 16:47 ` [RFC PATCH 12/15] loongarch/perf: " kan.liang
2025-05-07 1:17 ` 陈华才
2025-05-07 13:45 ` Liang, Kan
2025-05-06 16:47 ` [RFC PATCH 13/15] sparc/perf: " kan.liang
2025-05-06 16:47 ` [RFC PATCH 14/15] xtensa/perf: " kan.liang
2025-05-11 20:43 ` Max Filippov
2025-05-06 16:47 ` [RFC PATCH 15/15] mips/perf: " kan.liang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250506164740.1317237-2-kan.liang@linux.intel.com \
--to=kan.liang@linux.intel.com \
--cc=ctshao@google.com \
--cc=eranian@google.com \
--cc=irogers@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
--cc=tmricht@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).