[PATCH V4 00/16] perf: Fix the throttle logic for group

[PATCH V4 00/16] perf: Fix the throttle logic for group

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

Changes since V3:
- Add a new patch to only dump the throttle log for the leader
- Apply the perf_event_unthrottle() in event_sched_in() as well
- Not include the cleanup patch since it has been merged
- Add Acked-by from Vineet

Changes since V2:
- Add a cleanup patch to check if an event is in freq mode
- Rename the parameter of the perf_event_unthrottle_group()
- Add Tested-by from Leo and Thomas
- Add Acked-by from Guo Ren

Changes since V1:
- Apply the suggested throttle/unthrottle functions from Peter.
  The MAX_INTERRUPTS and throttle logs are applied to all events.
- Update the description and comments accordingly
- Add Reviewed-by from Ravi and Max

The sampling read doesn't work well with a group.
The issue was originally found by the 'Basic leader sampling test' case
failed on s390.
https://lore.kernel.org/all/20250228062241.303309-1-tmricht@linux.ibm.com/

Stephane debugged it and found it was caused by the throttling logic.
https://lore.kernel.org/all/CABPqkBQzCMNS_PfLZBWVuX9o8Z55PovwJvpVWMWzyeExFJ5R4Q@mail.gmail.com/

The throttle logic is generic and shared by all ARCHs.
It also impacts other ARCHs, e.g., X86.

On an Intel GNR machine,
$ 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 patch set tries to provide a generic fix for the group throttle
support. So the buggy driver-specific implementation can be removed.

The patch set is verified on newer Intel platforms (Kan), ARM (Leo Yan),
and s390 (Thomas Richter).

Kan Liang (16):
  perf: Fix the throttle logic for a group
  perf: Only dump the throttle log for the leader
  perf/x86/intel: Remove driver-specific throttle support
  perf/x86/amd: Remove driver-specific throttle support
  perf/x86/zhaoxin: Remove driver-specific throttle support
  powerpc/perf: Remove driver-specific throttle support
  s390/perf: Remove driver-specific throttle support
  perf/arm: Remove driver-specific throttle support
  perf/apple_m1: Remove driver-specific throttle support
  alpha/perf: Remove driver-specific throttle support
  arc/perf: Remove driver-specific throttle support
  csky/perf: Remove driver-specific throttle support
  loongarch/perf: Remove driver-specific throttle support
  sparc/perf: Remove driver-specific throttle support
  xtensa/perf: Remove driver-specific throttle support
  mips/perf: Remove driver-specific throttle support

 arch/alpha/kernel/perf_event.c       | 11 ++---
 arch/arc/kernel/perf_event.c         |  6 +--
 arch/csky/kernel/perf_event.c        |  3 +-
 arch/loongarch/kernel/perf_event.c   |  3 +-
 arch/mips/kernel/perf_event_mipsxx.c |  3 +-
 arch/powerpc/perf/core-book3s.c      |  6 +--
 arch/powerpc/perf/core-fsl-emb.c     |  3 +-
 arch/s390/kernel/perf_cpum_cf.c      |  2 -
 arch/s390/kernel/perf_cpum_sf.c      |  5 +-
 arch/sparc/kernel/perf_event.c       |  3 +-
 arch/x86/events/amd/core.c           |  3 +-
 arch/x86/events/amd/ibs.c            |  4 +-
 arch/x86/events/core.c               |  3 +-
 arch/x86/events/intel/core.c         |  6 +--
 arch/x86/events/intel/ds.c           |  7 ++-
 arch/x86/events/intel/knc.c          |  3 +-
 arch/x86/events/intel/p4.c           |  3 +-
 arch/x86/events/zhaoxin/core.c       |  3 +-
 arch/xtensa/kernel/perf_event.c      |  3 +-
 drivers/perf/apple_m1_cpu_pmu.c      |  3 +-
 drivers/perf/arm_pmuv3.c             |  3 +-
 drivers/perf/arm_v6_pmu.c            |  3 +-
 drivers/perf/arm_v7_pmu.c            |  3 +-
 drivers/perf/arm_xscale_pmu.c        |  6 +--
 kernel/events/core.c                 | 68 ++++++++++++++++++++--------
 25 files changed, 79 insertions(+), 87 deletions(-)

-- 
2.38.1

[PATCH V4 01/16] perf: Fix the throttle logic for a group

[kan.liang]

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. For all ARCHs, the implementation is similar. Only the event is
disabled, 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.
The MAX_INTERRUPTS is set to all throttle events.

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

Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---
 kernel/events/core.c | 66 ++++++++++++++++++++++++++++++--------------
 1 file changed, 46 insertions(+), 20 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index af78ec118e8f..915698f47682 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2739,6 +2739,39 @@ 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_unthrottle(struct perf_event *event, bool start)
+{
+	event->hw.interrupts = 0;
+	if (start)
+		event->pmu->start(event, 0);
+	perf_log_throttle(event, 1);
+}
+
+static void perf_event_throttle(struct perf_event *event)
+{
+	event->pmu->stop(event, 0);
+	event->hw.interrupts = MAX_INTERRUPTS;
+	perf_log_throttle(event, 0);
+}
+
+static void perf_event_unthrottle_group(struct perf_event *event, bool skip_start_event)
+{
+	struct perf_event *sibling, *leader = event->group_leader;
+
+	perf_event_unthrottle(leader, skip_start_event ? leader != event : true);
+	for_each_sibling_event(sibling, leader)
+		perf_event_unthrottle(sibling, skip_start_event ? sibling != event : true);
+}
+
+static void perf_event_throttle_group(struct perf_event *event)
+{
+	struct perf_event *sibling, *leader = event->group_leader;
+
+	perf_event_throttle(leader);
+	for_each_sibling_event(sibling, leader)
+		perf_event_throttle(sibling);
+}
+
 static int
 event_sched_in(struct perf_event *event, struct perf_event_context *ctx)
 {
@@ -2767,10 +2800,8 @@ event_sched_in(struct perf_event *event, struct perf_event_context *ctx)
 	 * ticks already, also for a heavily scheduling task there is little
 	 * guarantee it'll get a tick in a timely manner.
 	 */
-	if (unlikely(event->hw.interrupts == MAX_INTERRUPTS)) {
-		perf_log_throttle(event, 1);
-		event->hw.interrupts = 0;
-	}
+	if (unlikely(event->hw.interrupts == MAX_INTERRUPTS))
+		perf_event_unthrottle(event, false);
 
 	perf_pmu_disable(event->pmu);
 
@@ -4393,12 +4424,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 (!is_event_in_freq_mode(event))
-				event->pmu->start(event, 0);
-		}
+		if (hwc->interrupts == MAX_INTERRUPTS)
+			perf_event_unthrottle_group(event, is_event_in_freq_mode(event));
 
 		if (!is_event_in_freq_mode(event))
 			continue;
@@ -6426,14 +6453,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);
 	}
 
@@ -6441,6 +6460,14 @@ static void __perf_event_period(struct perf_event *event,
 
 	if (active) {
 		event->pmu->start(event, PERF_EF_RELOAD);
+		/*
+		 * Once the period is force-reset, the event starts immediately.
+		 * But the event/group could be throttled. Unthrottle the
+		 * event/group now to avoid the next tick trying to unthrottle
+		 * while we already re-started the event/group.
+		 */
+		if (event->hw.interrupts == MAX_INTERRUPTS)
+			perf_event_unthrottle_group(event, true);
 		perf_pmu_enable(event->pmu);
 	}
 }
@@ -10331,8 +10358,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_throttle_group(event);
 		ret = 1;
 	}
 
-- 
2.38.1

[PATCH V4 02/16] perf: Only dump the throttle log for the leader

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

The PERF_RECORD_THROTTLE records are dumped for all throttled events.
It's not necessary for group events, which are throttled altogether.

Optimize it by only dump the throttle log for the leader.

The sample right after the THROTTLE record must be generated by the
actual target event. It is good enough for the perf tool to locate the
actual target event.

Suggested-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---
 kernel/events/core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 915698f47682..cd559501cfbd 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2744,14 +2744,16 @@ static void perf_event_unthrottle(struct perf_event *event, bool start)
 	event->hw.interrupts = 0;
 	if (start)
 		event->pmu->start(event, 0);
-	perf_log_throttle(event, 1);
+	if (event == event->group_leader)
+		perf_log_throttle(event, 1);
 }
 
 static void perf_event_throttle(struct perf_event *event)
 {
 	event->pmu->stop(event, 0);
 	event->hw.interrupts = MAX_INTERRUPTS;
-	perf_log_throttle(event, 0);
+	if (event == event->group_leader)
+		perf_log_throttle(event, 0);
 }
 
 static void perf_event_unthrottle_group(struct perf_event *event, bool skip_start_event)
-- 
2.38.1

[PATCH V4 03/16] perf/x86/intel: Remove driver-specific throttle support

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

The throttle support has been added in the generic code. Remove
the driver-specific throttle support.

Besides the throttle, perf_event_overflow may return true because of
event_limit. It already does an inatomic event disable. The pmu->stop
is not required either.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---
 arch/x86/events/core.c       | 3 +--
 arch/x86/events/intel/core.c | 6 ++----
 arch/x86/events/intel/ds.c   | 7 +++----
 arch/x86/events/intel/knc.c  | 3 +--
 arch/x86/events/intel/p4.c   | 3 +--
 5 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 186e31cd0c14..8a2f73333a50 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -1730,8 +1730,7 @@ int x86_pmu_handle_irq(struct pt_regs *regs)
 
 		perf_sample_save_brstack(&data, event, &cpuc->lbr_stack, NULL);
 
-		if (perf_event_overflow(event, &data, regs))
-			x86_pmu_stop(event, 0);
+		perf_event_overflow(event, &data, regs);
 	}
 
 	if (handled)
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index b7562d66c6ea..a8309a67693e 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -3138,8 +3138,7 @@ static void x86_pmu_handle_guest_pebs(struct pt_regs *regs,
 			continue;
 
 		perf_sample_data_init(data, 0, event->hw.last_period);
-		if (perf_event_overflow(event, data, regs))
-			x86_pmu_stop(event, 0);
+		perf_event_overflow(event, data, regs);
 
 		/* Inject one fake event is enough. */
 		break;
@@ -3282,8 +3281,7 @@ static int handle_pmi_common(struct pt_regs *regs, u64 status)
 		if (has_branch_stack(event))
 			intel_pmu_lbr_save_brstack(&data, cpuc, event);
 
-		if (perf_event_overflow(event, &data, regs))
-			x86_pmu_stop(event, 0);
+		perf_event_overflow(event, &data, regs);
 	}
 
 	return handled;
diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index 58c054fa56b5..f8610f7196f0 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -2368,8 +2368,7 @@ __intel_pmu_pebs_last_event(struct perf_event *event,
 		 * All but the last records are processed.
 		 * The last one is left to be able to call the overflow handler.
 		 */
-		if (perf_event_overflow(event, data, regs))
-			x86_pmu_stop(event, 0);
+		perf_event_overflow(event, data, regs);
 	}
 
 	if (hwc->flags & PERF_X86_EVENT_AUTO_RELOAD) {
@@ -2597,8 +2596,8 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs, struct perf_sample_d
 		if (error[bit]) {
 			perf_log_lost_samples(event, error[bit]);
 
-			if (iregs && perf_event_account_interrupt(event))
-				x86_pmu_stop(event, 0);
+			if (iregs)
+				perf_event_account_interrupt(event);
 		}
 
 		if (counts[bit]) {
diff --git a/arch/x86/events/intel/knc.c b/arch/x86/events/intel/knc.c
index 3e8ec049b46d..384589168c1a 100644
--- a/arch/x86/events/intel/knc.c
+++ b/arch/x86/events/intel/knc.c
@@ -254,8 +254,7 @@ static int knc_pmu_handle_irq(struct pt_regs *regs)
 
 		perf_sample_data_init(&data, 0, last_period);
 
-		if (perf_event_overflow(event, &data, regs))
-			x86_pmu_stop(event, 0);
+		perf_event_overflow(event, &data, regs);
 	}
 
 	/*
diff --git a/arch/x86/events/intel/p4.c b/arch/x86/events/intel/p4.c
index c85a9fc44355..126d5ae264cb 100644
--- a/arch/x86/events/intel/p4.c
+++ b/arch/x86/events/intel/p4.c
@@ -1072,8 +1072,7 @@ static int p4_pmu_handle_irq(struct pt_regs *regs)
 			continue;
 
 
-		if (perf_event_overflow(event, &data, regs))
-			x86_pmu_stop(event, 0);
+		perf_event_overflow(event, &data, regs);
 	}
 
 	if (handled)
-- 
2.38.1

[PATCH V4 04/16] perf/x86/amd: Remove driver-specific throttle support

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

The throttle support has been added in the generic code. Remove
the driver-specific throttle support.

Besides the throttle, perf_event_overflow may return true because of
event_limit. It already does an inatomic event disable. The pmu->stop
is not required either.

Reviewed-by: Ravi Bangoria <ravi.bangoria@amd.com>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: Sandipan Das <sandipan.das@amd.com>
Cc: Ravi Bangoria <ravi.bangoria@amd.com>
---
 arch/x86/events/amd/core.c | 3 +--
 arch/x86/events/amd/ibs.c  | 4 +---
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index 30d6ceb4c8ad..5e64283b9bf2 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -1003,8 +1003,7 @@ static int amd_pmu_v2_handle_irq(struct pt_regs *regs)
 
 		perf_sample_save_brstack(&data, event, &cpuc->lbr_stack, NULL);
 
-		if (perf_event_overflow(event, &data, regs))
-			x86_pmu_stop(event, 0);
+		perf_event_overflow(event, &data, regs);
 	}
 
 	/*
diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
index 0252b7ea8bca..4bbbca02aeb1 100644
--- a/arch/x86/events/amd/ibs.c
+++ b/arch/x86/events/amd/ibs.c
@@ -1373,9 +1373,7 @@ static int perf_ibs_handle_irq(struct perf_ibs *perf_ibs, struct pt_regs *iregs)
 		hwc->sample_period = perf_ibs->min_period;
 
 out:
-	if (throttle) {
-		perf_ibs_stop(event, 0);
-	} else {
+	if (!throttle) {
 		if (perf_ibs == &perf_ibs_op) {
 			if (ibs_caps & IBS_CAPS_OPCNTEXT) {
 				new_config = period & IBS_OP_MAX_CNT_EXT_MASK;
-- 
2.38.1

[PATCH V4 05/16] perf/x86/zhaoxin: Remove driver-specific throttle support

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

The throttle support has been added in the generic code. Remove
the driver-specific throttle support.

Besides the throttle, perf_event_overflow may return true because of
event_limit. It already does an inatomic event disable. The pmu->stop
is not required either.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: silviazhao <silviazhao-oc@zhaoxin.com>
Cc: CodyYao-oc <CodyYao-oc@zhaoxin.com>
---
 arch/x86/events/zhaoxin/core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/events/zhaoxin/core.c b/arch/x86/events/zhaoxin/core.c
index 2fd9b0cf9a5e..49a5944fac63 100644
--- a/arch/x86/events/zhaoxin/core.c
+++ b/arch/x86/events/zhaoxin/core.c
@@ -397,8 +397,7 @@ static int zhaoxin_pmu_handle_irq(struct pt_regs *regs)
 		if (!x86_perf_event_set_period(event))
 			continue;
 
-		if (perf_event_overflow(event, &data, regs))
-			x86_pmu_stop(event, 0);
+		perf_event_overflow(event, &data, regs);
 	}
 
 	/*
-- 
2.38.1

[PATCH V4 06/16] powerpc/perf: Remove driver-specific throttle support

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

The throttle support has been added in the generic code. Remove
the driver-specific throttle support.

Besides the throttle, perf_event_overflow may return true because of
event_limit. It already does an inatomic event disable. The pmu->stop
is not required either.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
---
 arch/powerpc/perf/core-book3s.c  | 6 ++----
 arch/powerpc/perf/core-fsl-emb.c | 3 +--
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 42ff4d167acc..8b0081441f85 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -2344,12 +2344,10 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
 			ppmu->get_mem_weight(&data.weight.full, event->attr.sample_type);
 			data.sample_flags |= PERF_SAMPLE_WEIGHT_TYPE;
 		}
-		if (perf_event_overflow(event, &data, regs))
-			power_pmu_stop(event, 0);
+		perf_event_overflow(event, &data, regs);
 	} else if (period) {
 		/* Account for interrupt in case of invalid SIAR */
-		if (perf_event_account_interrupt(event))
-			power_pmu_stop(event, 0);
+		perf_event_account_interrupt(event);
 	}
 }
 
diff --git a/arch/powerpc/perf/core-fsl-emb.c b/arch/powerpc/perf/core-fsl-emb.c
index d2ffcc7021c5..7120ab20cbfe 100644
--- a/arch/powerpc/perf/core-fsl-emb.c
+++ b/arch/powerpc/perf/core-fsl-emb.c
@@ -635,8 +635,7 @@ static void record_and_restart(struct perf_event *event, unsigned long val,
 
 		perf_sample_data_init(&data, 0, last_period);
 
-		if (perf_event_overflow(event, &data, regs))
-			fsl_emb_pmu_stop(event, 0);
+		perf_event_overflow(event, &data, regs);
 	}
 }
 
-- 
2.38.1

[PATCH V4 07/16] s390/perf: Remove driver-specific throttle support

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

The throttle support has been added in the generic code. Remove
the driver-specific throttle support.

Besides the throttle, perf_event_overflow may return true because of
event_limit. It already does an inatomic event disable. The pmu->stop
is not required either.

Tested-by: Thomas Richter <tmricht@linux.ibm.com>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: Thomas Richter <tmricht@linux.ibm.com>
Cc: linux-s390@vger.kernel.org
---
 arch/s390/kernel/perf_cpum_cf.c | 2 --
 arch/s390/kernel/perf_cpum_sf.c | 5 +----
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/s390/kernel/perf_cpum_cf.c b/arch/s390/kernel/perf_cpum_cf.c
index e657fad7e376..6a262e198e35 100644
--- a/arch/s390/kernel/perf_cpum_cf.c
+++ b/arch/s390/kernel/perf_cpum_cf.c
@@ -980,8 +980,6 @@ static int cfdiag_push_sample(struct perf_event *event,
 	}
 
 	overflow = perf_event_overflow(event, &data, &regs);
-	if (overflow)
-		event->pmu->stop(event, 0);
 
 	perf_event_update_userpage(event);
 	return overflow;
diff --git a/arch/s390/kernel/perf_cpum_sf.c b/arch/s390/kernel/perf_cpum_sf.c
index ad22799d8a7d..91469401f2c9 100644
--- a/arch/s390/kernel/perf_cpum_sf.c
+++ b/arch/s390/kernel/perf_cpum_sf.c
@@ -1072,10 +1072,7 @@ static int perf_push_sample(struct perf_event *event,
 	overflow = 0;
 	if (perf_event_exclude(event, &regs, sde_regs))
 		goto out;
-	if (perf_event_overflow(event, &data, &regs)) {
-		overflow = 1;
-		event->pmu->stop(event, 0);
-	}
+	overflow = perf_event_overflow(event, &data, &regs);
 	perf_event_update_userpage(event);
 out:
 	return overflow;
-- 
2.38.1

[PATCH V4 08/16] perf/arm: Remove driver-specific throttle support

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

The throttle support has been added in the generic code. Remove
the driver-specific throttle support.

Besides the throttle, perf_event_overflow may return true because of
event_limit. It already does an inatomic event disable. The pmu->stop
is not required either.

Tested-by: Leo Yan <leo.yan@arm.com>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Rob Herring (Arm) <robh@kernel.org>
Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Leo Yan <leo.yan@arm.com>
---
 drivers/perf/arm_pmuv3.c      | 3 +--
 drivers/perf/arm_v6_pmu.c     | 3 +--
 drivers/perf/arm_v7_pmu.c     | 3 +--
 drivers/perf/arm_xscale_pmu.c | 6 ++----
 4 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
index e506d59654e7..3db9f4ed17e8 100644
--- a/drivers/perf/arm_pmuv3.c
+++ b/drivers/perf/arm_pmuv3.c
@@ -887,8 +887,7 @@ static irqreturn_t armv8pmu_handle_irq(struct arm_pmu *cpu_pmu)
 		 * an irq_work which will be taken care of in the handling of
 		 * IPI_IRQ_WORK.
 		 */
-		if (perf_event_overflow(event, &data, regs))
-			cpu_pmu->disable(event);
+		perf_event_overflow(event, &data, regs);
 	}
 	armv8pmu_start(cpu_pmu);
 
diff --git a/drivers/perf/arm_v6_pmu.c b/drivers/perf/arm_v6_pmu.c
index b09615bb2bb2..7cb12c8e06c7 100644
--- a/drivers/perf/arm_v6_pmu.c
+++ b/drivers/perf/arm_v6_pmu.c
@@ -276,8 +276,7 @@ armv6pmu_handle_irq(struct arm_pmu *cpu_pmu)
 		if (!armpmu_event_set_period(event))
 			continue;
 
-		if (perf_event_overflow(event, &data, regs))
-			cpu_pmu->disable(event);
+		perf_event_overflow(event, &data, regs);
 	}
 
 	/*
diff --git a/drivers/perf/arm_v7_pmu.c b/drivers/perf/arm_v7_pmu.c
index 17831e1920bd..a1e438101114 100644
--- a/drivers/perf/arm_v7_pmu.c
+++ b/drivers/perf/arm_v7_pmu.c
@@ -930,8 +930,7 @@ static irqreturn_t armv7pmu_handle_irq(struct arm_pmu *cpu_pmu)
 		if (!armpmu_event_set_period(event))
 			continue;
 
-		if (perf_event_overflow(event, &data, regs))
-			cpu_pmu->disable(event);
+		perf_event_overflow(event, &data, regs);
 	}
 
 	/*
diff --git a/drivers/perf/arm_xscale_pmu.c b/drivers/perf/arm_xscale_pmu.c
index 638fea9b1263..c2ac41dd9e19 100644
--- a/drivers/perf/arm_xscale_pmu.c
+++ b/drivers/perf/arm_xscale_pmu.c
@@ -186,8 +186,7 @@ xscale1pmu_handle_irq(struct arm_pmu *cpu_pmu)
 		if (!armpmu_event_set_period(event))
 			continue;
 
-		if (perf_event_overflow(event, &data, regs))
-			cpu_pmu->disable(event);
+		perf_event_overflow(event, &data, regs);
 	}
 
 	irq_work_run();
@@ -519,8 +518,7 @@ xscale2pmu_handle_irq(struct arm_pmu *cpu_pmu)
 		if (!armpmu_event_set_period(event))
 			continue;
 
-		if (perf_event_overflow(event, &data, regs))
-			cpu_pmu->disable(event);
+		perf_event_overflow(event, &data, regs);
 	}
 
 	irq_work_run();
-- 
2.38.1

[PATCH V4 09/16] perf/apple_m1: Remove driver-specific throttle support

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

The throttle support has been added in the generic code. Remove
the driver-specific throttle support.

Besides the throttle, perf_event_overflow may return true because of
event_limit. It already does an inatomic event disable. The pmu->stop
is not required either.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: Oliver Upton <oliver.upton@linux.dev>
---
 drivers/perf/apple_m1_cpu_pmu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/perf/apple_m1_cpu_pmu.c b/drivers/perf/apple_m1_cpu_pmu.c
index df9a28ba69dc..81b6f1a62349 100644
--- a/drivers/perf/apple_m1_cpu_pmu.c
+++ b/drivers/perf/apple_m1_cpu_pmu.c
@@ -474,8 +474,7 @@ static irqreturn_t m1_pmu_handle_irq(struct arm_pmu *cpu_pmu)
 		if (!armpmu_event_set_period(event))
 			continue;
 
-		if (perf_event_overflow(event, &data, regs))
-			m1_pmu_disable_event(event);
+		perf_event_overflow(event, &data, regs);
 	}
 
 	cpu_pmu->start(cpu_pmu);
-- 
2.38.1

[PATCH V4 10/16] alpha/perf: Remove driver-specific throttle support

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

The throttle support has been added in the generic code. Remove
the driver-specific throttle support.

Besides the throttle, perf_event_overflow may return true because of
event_limit. It already does an inatomic event disable. The pmu->stop
is not required either.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: linux-alpha@vger.kernel.org
---
 arch/alpha/kernel/perf_event.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/alpha/kernel/perf_event.c b/arch/alpha/kernel/perf_event.c
index 1f0eb4f25c0f..a3eaab094ece 100644
--- a/arch/alpha/kernel/perf_event.c
+++ b/arch/alpha/kernel/perf_event.c
@@ -852,14 +852,9 @@ static void alpha_perf_event_irq_handler(unsigned long la_ptr,
 	alpha_perf_event_update(event, hwc, idx, alpha_pmu->pmc_max_period[idx]+1);
 	perf_sample_data_init(&data, 0, hwc->last_period);
 
-	if (alpha_perf_event_set_period(event, hwc, idx)) {
-		if (perf_event_overflow(event, &data, regs)) {
-			/* Interrupts coming too quickly; "throttle" the
-			 * counter, i.e., disable it for a little while.
-			 */
-			alpha_pmu_stop(event, 0);
-		}
-	}
+	if (alpha_perf_event_set_period(event, hwc, idx))
+		perf_event_overflow(event, &data, regs);
+
 	wrperfmon(PERFMON_CMD_ENABLE, cpuc->idx_mask);
 
 	return;
-- 
2.38.1

[PATCH V4 11/16] arc/perf: Remove driver-specific throttle support

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

The throttle support has been added in the generic code. Remove
the driver-specific throttle support.

Besides the throttle, perf_event_overflow may return true because of
event_limit. It already does an inatomic event disable. The pmu->stop
is not required either.

Acked-by: Vineet Gupta <vgupta@kernel.org>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: Vineet Gupta <vgupta@ikernel.org>
Cc: linux-snps-arc@lists.infradead.org
---
 arch/arc/kernel/perf_event.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/arc/kernel/perf_event.c b/arch/arc/kernel/perf_event.c
index 6e5a651cd75c..ed6d4f0cd621 100644
--- a/arch/arc/kernel/perf_event.c
+++ b/arch/arc/kernel/perf_event.c
@@ -599,10 +599,8 @@ static irqreturn_t arc_pmu_intr(int irq, void *dev)
 
 		arc_perf_event_update(event, &event->hw, event->hw.idx);
 		perf_sample_data_init(&data, 0, hwc->last_period);
-		if (arc_pmu_event_set_period(event)) {
-			if (perf_event_overflow(event, &data, regs))
-				arc_pmu_stop(event, 0);
-		}
+		if (arc_pmu_event_set_period(event))
+			perf_event_overflow(event, &data, regs);
 
 		active_ints &= ~BIT(idx);
 	} while (active_ints);
-- 
2.38.1

[PATCH V4 12/16] csky/perf: Remove driver-specific throttle support

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

The throttle support has been added in the generic code. Remove
the driver-specific throttle support.

Besides the throttle, perf_event_overflow may return true because of
event_limit. It already does an inatomic event disable. The pmu->stop
is not required either.

Acked-by: Guo Ren <guoren@kernel.org>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: Mao Han <han_mao@c-sky.com>
Cc: Guo Ren <ren_guo@c-sky.com>
Cc: linux-csky@vger.kernel.org
---
 arch/csky/kernel/perf_event.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/csky/kernel/perf_event.c b/arch/csky/kernel/perf_event.c
index e5f18420ce64..e0a36acd265b 100644
--- a/arch/csky/kernel/perf_event.c
+++ b/arch/csky/kernel/perf_event.c
@@ -1139,8 +1139,7 @@ static irqreturn_t csky_pmu_handle_irq(int irq_num, void *dev)
 		perf_sample_data_init(&data, 0, hwc->last_period);
 		csky_pmu_event_set_period(event);
 
-		if (perf_event_overflow(event, &data, regs))
-			csky_pmu_stop_event(event);
+		perf_event_overflow(event, &data, regs);
 	}
 
 	csky_pmu_enable(&csky_pmu.pmu);
-- 
2.38.1

[PATCH V4 13/16] loongarch/perf: Remove driver-specific throttle support

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

The throttle support has been added in the generic code. Remove
the driver-specific throttle support.

Besides the throttle, perf_event_overflow may return true because of
event_limit. It already does an inatomic event disable. The pmu->stop
is not required either.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: Bibo Mao <maobibo@loongson.cn>
Cc: Huacai Chen <chenhuacai@loongson.cn>
Cc: loongarch@lists.linux.dev
---
 arch/loongarch/kernel/perf_event.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/loongarch/kernel/perf_event.c b/arch/loongarch/kernel/perf_event.c
index f86a4b838dd7..8ad098703488 100644
--- a/arch/loongarch/kernel/perf_event.c
+++ b/arch/loongarch/kernel/perf_event.c
@@ -479,8 +479,7 @@ static void handle_associated_event(struct cpu_hw_events *cpuc, int idx,
 	if (!loongarch_pmu_event_set_period(event, hwc, idx))
 		return;
 
-	if (perf_event_overflow(event, data, regs))
-		loongarch_pmu_disable_event(idx);
+	perf_event_overflow(event, data, regs);
 }
 
 static irqreturn_t pmu_handle_irq(int irq, void *dev)
-- 
2.38.1

[PATCH V4 14/16] sparc/perf: Remove driver-specific throttle support

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

The throttle support has been added in the generic code. Remove
the driver-specific throttle support.

Besides the throttle, perf_event_overflow may return true because of
event_limit. It already does an inatomic event disable. The pmu->stop
is not required either.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: sparclinux@vger.kernel.org
---
 arch/sparc/kernel/perf_event.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/sparc/kernel/perf_event.c b/arch/sparc/kernel/perf_event.c
index f02a283a8e8f..cae4d33002a5 100644
--- a/arch/sparc/kernel/perf_event.c
+++ b/arch/sparc/kernel/perf_event.c
@@ -1668,8 +1668,7 @@ static int __kprobes perf_event_nmi_handler(struct notifier_block *self,
 		if (!sparc_perf_event_set_period(event, hwc, idx))
 			continue;
 
-		if (perf_event_overflow(event, &data, regs))
-			sparc_pmu_stop(event, 0);
+		perf_event_overflow(event, &data, regs);
 	}
 
 	finish_clock = sched_clock();
-- 
2.38.1

[PATCH V4 15/16] xtensa/perf: Remove driver-specific throttle support

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

The throttle support has been added in the generic code. Remove
the driver-specific throttle support.

Besides the throttle, perf_event_overflow may return true because of
event_limit. It already does an inatomic event disable. The pmu->stop
is not required either.

Reviewed-by: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: Max Filippov <jcmvbkbc@gmail.com>
---
 arch/xtensa/kernel/perf_event.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/xtensa/kernel/perf_event.c b/arch/xtensa/kernel/perf_event.c
index 183618090d05..223f1d452310 100644
--- a/arch/xtensa/kernel/perf_event.c
+++ b/arch/xtensa/kernel/perf_event.c
@@ -388,8 +388,7 @@ irqreturn_t xtensa_pmu_irq_handler(int irq, void *dev_id)
 			struct pt_regs *regs = get_irq_regs();
 
 			perf_sample_data_init(&data, 0, last_period);
-			if (perf_event_overflow(event, &data, regs))
-				xtensa_pmu_stop(event, 0);
+			perf_event_overflow(event, &data, regs);
 		}
 
 		rc = IRQ_HANDLED;
-- 
2.38.1

[PATCH V4 16/16] mips/perf: Remove driver-specific throttle support

[kan.liang]

From: Kan Liang <kan.liang@linux.intel.com>

The throttle support has been added in the generic code. Remove
the driver-specific throttle support.

Besides the throttle, perf_event_overflow may return true because of
event_limit. It already does an inatomic event disable. The pmu->stop
is not required either.

Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: linux-mips@vger.kernel.org
---
 arch/mips/kernel/perf_event_mipsxx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/mips/kernel/perf_event_mipsxx.c b/arch/mips/kernel/perf_event_mipsxx.c
index c4d6b09136b1..196a070349b0 100644
--- a/arch/mips/kernel/perf_event_mipsxx.c
+++ b/arch/mips/kernel/perf_event_mipsxx.c
@@ -791,8 +791,7 @@ static void handle_associated_event(struct cpu_hw_events *cpuc,
 	if (!mipspmu_event_set_period(event, hwc, idx))
 		return;
 
-	if (perf_event_overflow(event, data, regs))
-		mipsxx_pmu_disable_event(idx);
+	perf_event_overflow(event, data, regs);
 }
 
 
-- 
2.38.1

Re: [PATCH V4 01/16] perf: Fix the throttle logic for a group

[Namhyung Kim]

On Tue, May 20, 2025 at 11:16:29AM -0700, kan.liang@linux.intel.com wrote:
> 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. For all ARCHs, the implementation is similar. Only the event is
> disabled, 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.
> The MAX_INTERRUPTS is set to all throttle events.
> 
> 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
> 
> Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> ---
>  kernel/events/core.c | 66 ++++++++++++++++++++++++++++++--------------
>  1 file changed, 46 insertions(+), 20 deletions(-)
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index af78ec118e8f..915698f47682 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -2739,6 +2739,39 @@ 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_unthrottle(struct perf_event *event, bool start)
> +{
> +	event->hw.interrupts = 0;
> +	if (start)
> +		event->pmu->start(event, 0);
> +	perf_log_throttle(event, 1);
> +}
> +
> +static void perf_event_throttle(struct perf_event *event)
> +{
> +	event->pmu->stop(event, 0);
> +	event->hw.interrupts = MAX_INTERRUPTS;
> +	perf_log_throttle(event, 0);
> +}
> +
> +static void perf_event_unthrottle_group(struct perf_event *event, bool skip_start_event)
> +{
> +	struct perf_event *sibling, *leader = event->group_leader;
> +
> +	perf_event_unthrottle(leader, skip_start_event ? leader != event : true);
> +	for_each_sibling_event(sibling, leader)
> +		perf_event_unthrottle(sibling, skip_start_event ? sibling != event : true);
> +}
> +
> +static void perf_event_throttle_group(struct perf_event *event)
> +{
> +	struct perf_event *sibling, *leader = event->group_leader;
> +
> +	perf_event_throttle(leader);
> +	for_each_sibling_event(sibling, leader)
> +		perf_event_throttle(sibling);
> +}
> +
>  static int
>  event_sched_in(struct perf_event *event, struct perf_event_context *ctx)
>  {
> @@ -2767,10 +2800,8 @@ event_sched_in(struct perf_event *event, struct perf_event_context *ctx)
>  	 * ticks already, also for a heavily scheduling task there is little
>  	 * guarantee it'll get a tick in a timely manner.
>  	 */
> -	if (unlikely(event->hw.interrupts == MAX_INTERRUPTS)) {
> -		perf_log_throttle(event, 1);
> -		event->hw.interrupts = 0;
> -	}
> +	if (unlikely(event->hw.interrupts == MAX_INTERRUPTS))
> +		perf_event_unthrottle(event, false);
>  
>  	perf_pmu_disable(event->pmu);
>  
> @@ -4393,12 +4424,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 (!is_event_in_freq_mode(event))
> -				event->pmu->start(event, 0);
> -		}
> +		if (hwc->interrupts == MAX_INTERRUPTS)
> +			perf_event_unthrottle_group(event, is_event_in_freq_mode(event));
>  
>  		if (!is_event_in_freq_mode(event))
>  			continue;
> @@ -6426,14 +6453,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);
>  	}
>  
> @@ -6441,6 +6460,14 @@ static void __perf_event_period(struct perf_event *event,
>  
>  	if (active) {
>  		event->pmu->start(event, PERF_EF_RELOAD);
> +		/*
> +		 * Once the period is force-reset, the event starts immediately.
> +		 * But the event/group could be throttled. Unthrottle the
> +		 * event/group now to avoid the next tick trying to unthrottle
> +		 * while we already re-started the event/group.
> +		 */
> +		if (event->hw.interrupts == MAX_INTERRUPTS)
> +			perf_event_unthrottle_group(event, true);
>  		perf_pmu_enable(event->pmu);
>  	}
>  }
> @@ -10331,8 +10358,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_throttle_group(event);
>  		ret = 1;
>  	}
>  
> -- 
> 2.38.1
> 

Re: [PATCH V4 02/16] perf: Only dump the throttle log for the leader

[Namhyung Kim]

On Tue, May 20, 2025 at 11:16:30AM -0700, kan.liang@linux.intel.com wrote:
> From: Kan Liang <kan.liang@linux.intel.com>
> 
> The PERF_RECORD_THROTTLE records are dumped for all throttled events.
> It's not necessary for group events, which are throttled altogether.
> 
> Optimize it by only dump the throttle log for the leader.
> 
> The sample right after the THROTTLE record must be generated by the
> actual target event. It is good enough for the perf tool to locate the
> actual target event.
> 
> Suggested-by: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> ---
>  kernel/events/core.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 915698f47682..cd559501cfbd 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -2744,14 +2744,16 @@ static void perf_event_unthrottle(struct perf_event *event, bool start)
>  	event->hw.interrupts = 0;
>  	if (start)
>  		event->pmu->start(event, 0);
> -	perf_log_throttle(event, 1);
> +	if (event == event->group_leader)
> +		perf_log_throttle(event, 1);
>  }
>  
>  static void perf_event_throttle(struct perf_event *event)
>  {
>  	event->pmu->stop(event, 0);
>  	event->hw.interrupts = MAX_INTERRUPTS;
> -	perf_log_throttle(event, 0);
> +	if (event == event->group_leader)
> +		perf_log_throttle(event, 0);
>  }
>  
>  static void perf_event_unthrottle_group(struct perf_event *event, bool skip_start_event)
> -- 
> 2.38.1
> 

Re: [PATCH V4 02/16] perf: Only dump the throttle log for the leader

[Peter Zijlstra]

On Tue, May 20, 2025 at 11:16:30AM -0700, kan.liang@linux.intel.com wrote:
> From: Kan Liang <kan.liang@linux.intel.com>
> 
> The PERF_RECORD_THROTTLE records are dumped for all throttled events.
> It's not necessary for group events, which are throttled altogether.
> 
> Optimize it by only dump the throttle log for the leader.
> 
> The sample right after the THROTTLE record must be generated by the
> actual target event. It is good enough for the perf tool to locate the
> actual target event.

So while both patches change behaviour; the first patch should preserve
all that was done and simply do more. OTOH this patch explicitly changes
behaviour in that what was done, is now no longer done.

Are we very sure there isn't a tool that expect per event throttle
messages?

I'll take the patches, but I'm somewhat suspect of this one.

Re: [PATCH V4 02/16] perf: Only dump the throttle log for the leader

[Liang, Kan]

On 2025-05-21 8:05 a.m., Peter Zijlstra wrote:
> On Tue, May 20, 2025 at 11:16:30AM -0700, kan.liang@linux.intel.com wrote:
>> From: Kan Liang <kan.liang@linux.intel.com>
>>
>> The PERF_RECORD_THROTTLE records are dumped for all throttled events.
>> It's not necessary for group events, which are throttled altogether.
>>
>> Optimize it by only dump the throttle log for the leader.
>>
>> The sample right after the THROTTLE record must be generated by the
>> actual target event. It is good enough for the perf tool to locate the
>> actual target event.
> 
> So while both patches change behaviour; the first patch should preserve
> all that was done and simply do more. OTOH this patch explicitly changes
> behaviour in that what was done, is now no longer done.
> 
> Are we very sure there isn't a tool that expect per event throttle
> messages?
> 

For a non-group event, the behavior is not changed. The event throttle
message is dumped for each of them, because the leader is itself.

The second patch may only change the behavior of some cases of the group
events, but not all of them. For example, the behavior of the common
usage sampling read case is not changed, since the leader is the
sampling event.
Also, as far as I know, the current perf tool doesn't track which event
is throttled.
Furthermore, the group events were broken. I doubt there are other tools
really work with group events throttle message.
All in all, we don't lose the throttle information for each event. Based
on the current information, tools can reconstruct it. Since the tool
needs to be fixed anyway, I don't see a problem.

Thanks,
Kan

Re: [PATCH V4 01/16] perf: Fix the throttle logic for a group

[Leo Yan]

Hi Kan,

[ + Aishwarya ]

On Tue, May 20, 2025 at 11:16:29AM -0700, kan.liang@linux.intel.com wrote:

[...]

> @@ -10331,8 +10358,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_throttle_group(event);
>  		ret = 1;
>  	}

Our (Arm) CI reports RCU stall that caused by this patch.  I can use a
simple command to trigger system stuck with cpu-clock:

  perf record -a -e cpu-clock -- sleep 2

I confirmed that if removing throttling code for cpu-clock event, then
the issue can be dimissed.  Based on reading code, the flow below:

  hrtimer interrupt:
   `> __perf_event_account_interrupt()
       `> perf_event_throttle_group()
           `> perf_event_throttle()
               `> cpu_clock_event_stop()
                   `> perf_swevent_cancel_hrtimer()
                       `> hrtimer_cancel()  -> Inifite loop.

In the hrtimer interrupt handler, it tries to cancel itself and causes
inifite loop.  Please consider to fix the issue.


Thanks,
Leo

Re: [PATCH V4 01/16] perf: Fix the throttle logic for a group

[Liang, Kan]

On 2025-05-27 12:16 p.m., Leo Yan wrote:
> Hi Kan,
> 
> [ + Aishwarya ]
> 
> On Tue, May 20, 2025 at 11:16:29AM -0700, kan.liang@linux.intel.com wrote:
> 
> [...]
> 
>> @@ -10331,8 +10358,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_throttle_group(event);
>>  		ret = 1;
>>  	}
> 
> Our (Arm) CI reports RCU stall that caused by this patch.  I can use a
> simple command to trigger system stuck with cpu-clock:
> 
>   perf record -a -e cpu-clock -- sleep 2
> 
> I confirmed that if removing throttling code for cpu-clock event, then
> the issue can be dimissed.  Based on reading code, the flow below:
> 
>   hrtimer interrupt:
>    `> __perf_event_account_interrupt()
>        `> perf_event_throttle_group()
>            `> perf_event_throttle()
>                `> cpu_clock_event_stop()
>                    `> perf_swevent_cancel_hrtimer()
>                        `> hrtimer_cancel()  -> Inifite loop.
> 
> In the hrtimer interrupt handler, it tries to cancel itself and causes
> inifite loop.  Please consider to fix the issue.
> 

The cpu-clock and task_clock are two special SW events, which rely on
the hrtimer. I missed them when checking the SW events. :(

For the two events, instead of invoking the stop(), the
HRTIMER_NORESTART is returned to stop the timer. Invoking the stop()
cause the issue.

There may be two ways to fix it.
- Add a check of MAX_INTERRUPTS in the event_stop. Return immediately if
the stop is invoked by the throttle.
- Introduce a PMU flag to track the case. Avoid the event_stop in
perf_event_throttle() if the flag is detected.

The latter looks more generic. It may be used if there are other cases
that want to avoid the stop. So the latter is implemented as below.

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 947ad12dfdbe..66f02f46595c 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -303,6 +303,7 @@ struct perf_event_pmu_context;
 #define PERF_PMU_CAP_AUX_OUTPUT			0x0080
 #define PERF_PMU_CAP_EXTENDED_HW_TYPE		0x0100
 #define PERF_PMU_CAP_AUX_PAUSE			0x0200
+#define PERF_PMU_CAP_NO_THROTTLE_STOP		0x0400

 /**
  * pmu::scope
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 8327ab0ee641..596597886d96 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2655,7 +2655,8 @@ static void perf_event_unthrottle(struct
perf_event *event, bool start)

 static void perf_event_throttle(struct perf_event *event)
 {
-	event->pmu->stop(event, 0);
+	if (!(event->pmu->capabilities & PERF_PMU_CAP_NO_THROTTLE_STOP))
+		event->pmu->stop(event, 0);
 	event->hw.interrupts = MAX_INTERRUPTS;
 	perf_log_throttle(event, 0);
 }
@@ -11846,7 +11847,8 @@ static int cpu_clock_event_init(struct
perf_event *event)
 static struct pmu perf_cpu_clock = {
 	.task_ctx_nr	= perf_sw_context,

-	.capabilities	= PERF_PMU_CAP_NO_NMI,
+	.capabilities	= PERF_PMU_CAP_NO_NMI |
+			  PERF_PMU_CAP_NO_THROTTLE_STOP,
 	.dev		= PMU_NULL_DEV,

 	.event_init	= cpu_clock_event_init,
@@ -11928,7 +11930,8 @@ static int task_clock_event_init(struct
perf_event *event)
 static struct pmu perf_task_clock = {
 	.task_ctx_nr	= perf_sw_context,

-	.capabilities	= PERF_PMU_CAP_NO_NMI,
+	.capabilities	= PERF_PMU_CAP_NO_NMI |
+			  PERF_PMU_CAP_NO_THROTTLE_STOP,
 	.dev		= PMU_NULL_DEV,

 	.event_init	= task_clock_event_init,


Thanks,
Kan

Re: [PATCH V4 01/16] perf: Fix the throttle logic for a group

[Leo Yan]

On Tue, May 27, 2025 at 03:30:06PM -0400, Liang, Kan wrote:

[...]

> There may be two ways to fix it.
> - Add a check of MAX_INTERRUPTS in the event_stop. Return immediately if
> the stop is invoked by the throttle.
> - Introduce a PMU flag to track the case. Avoid the event_stop in
> perf_event_throttle() if the flag is detected.
> 
> The latter looks more generic. It may be used if there are other cases
> that want to avoid the stop. So the latter is implemented as below.

Yes.  I agreed the fix below is more general and confirmed it can fix
the observed issue.

> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 947ad12dfdbe..66f02f46595c 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -303,6 +303,7 @@ struct perf_event_pmu_context;
>  #define PERF_PMU_CAP_AUX_OUTPUT			0x0080
>  #define PERF_PMU_CAP_EXTENDED_HW_TYPE		0x0100
>  #define PERF_PMU_CAP_AUX_PAUSE			0x0200
> +#define PERF_PMU_CAP_NO_THROTTLE_STOP		0x0400
> 
>  /**
>   * pmu::scope
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 8327ab0ee641..596597886d96 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -2655,7 +2655,8 @@ static void perf_event_unthrottle(struct
> perf_event *event, bool start)
> 
>  static void perf_event_throttle(struct perf_event *event)
>  {
> -	event->pmu->stop(event, 0);
> +	if (!(event->pmu->capabilities & PERF_PMU_CAP_NO_THROTTLE_STOP))
> +		event->pmu->stop(event, 0);

A background info is that even a PMU event is not stopped when
throttling, we still need to re-enable it.  This is why we don't do
particualy handling for PERF_PMU_CAP_NO_THROTTLE_STOP in
perf_event_unthrottle().

Maybe it is deserved add a comment for easier understanding.

Thanks,
Leo

>  	event->hw.interrupts = MAX_INTERRUPTS;
>  	perf_log_throttle(event, 0);
>  }
> @@ -11846,7 +11847,8 @@ static int cpu_clock_event_init(struct
> perf_event *event)
>  static struct pmu perf_cpu_clock = {
>  	.task_ctx_nr	= perf_sw_context,
> 
> -	.capabilities	= PERF_PMU_CAP_NO_NMI,
> +	.capabilities	= PERF_PMU_CAP_NO_NMI |
> +			  PERF_PMU_CAP_NO_THROTTLE_STOP,
>  	.dev		= PMU_NULL_DEV,
> 
>  	.event_init	= cpu_clock_event_init,
> @@ -11928,7 +11930,8 @@ static int task_clock_event_init(struct
> perf_event *event)
>  static struct pmu perf_task_clock = {
>  	.task_ctx_nr	= perf_sw_context,
> 
> -	.capabilities	= PERF_PMU_CAP_NO_NMI,
> +	.capabilities	= PERF_PMU_CAP_NO_NMI |
> +			  PERF_PMU_CAP_NO_THROTTLE_STOP,
>  	.dev		= PMU_NULL_DEV,
> 
>  	.event_init	= task_clock_event_init,
> 
> 
> Thanks,
> Kan
> 
> 

Re: [PATCH V4 01/16] perf: Fix the throttle logic for a group

[Liang, Kan]

On 2025-05-28 6:28 a.m., Leo Yan wrote:
> On Tue, May 27, 2025 at 03:30:06PM -0400, Liang, Kan wrote:
> 
> [...]
> 
>> There may be two ways to fix it.
>> - Add a check of MAX_INTERRUPTS in the event_stop. Return immediately if
>> the stop is invoked by the throttle.
>> - Introduce a PMU flag to track the case. Avoid the event_stop in
>> perf_event_throttle() if the flag is detected.
>>
>> The latter looks more generic. It may be used if there are other cases
>> that want to avoid the stop. So the latter is implemented as below.
> 
> Yes.  I agreed the fix below is more general and confirmed it can fix
> the observed issue.
> 
>> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
>> index 947ad12dfdbe..66f02f46595c 100644
>> --- a/include/linux/perf_event.h
>> +++ b/include/linux/perf_event.h
>> @@ -303,6 +303,7 @@ struct perf_event_pmu_context;
>>  #define PERF_PMU_CAP_AUX_OUTPUT			0x0080
>>  #define PERF_PMU_CAP_EXTENDED_HW_TYPE		0x0100
>>  #define PERF_PMU_CAP_AUX_PAUSE			0x0200
>> +#define PERF_PMU_CAP_NO_THROTTLE_STOP		0x0400
>>
>>  /**
>>   * pmu::scope
>> diff --git a/kernel/events/core.c b/kernel/events/core.c
>> index 8327ab0ee641..596597886d96 100644
>> --- a/kernel/events/core.c
>> +++ b/kernel/events/core.c
>> @@ -2655,7 +2655,8 @@ static void perf_event_unthrottle(struct
>> perf_event *event, bool start)
>>
>>  static void perf_event_throttle(struct perf_event *event)
>>  {
>> -	event->pmu->stop(event, 0);
>> +	if (!(event->pmu->capabilities & PERF_PMU_CAP_NO_THROTTLE_STOP))
>> +		event->pmu->stop(event, 0);
> 
> A background info is that even a PMU event is not stopped when
> throttling, we still need to re-enable it.  This is why we don't do
> particualy handling for PERF_PMU_CAP_NO_THROTTLE_STOP in
> perf_event_unthrottle().
> 
> Maybe it is deserved add a comment for easier understanding.

Sure. A formal patch has been sent. Please take a look.
https://lore.kernel.org/lkml/20250528144823.2996185-1-kan.liang@linux.intel.com/

Thanks,
Kan>
> Thanks,
> Leo
> 
>>  	event->hw.interrupts = MAX_INTERRUPTS;
>>  	perf_log_throttle(event, 0);
>>  }
>> @@ -11846,7 +11847,8 @@ static int cpu_clock_event_init(struct
>> perf_event *event)
>>  static struct pmu perf_cpu_clock = {
>>  	.task_ctx_nr	= perf_sw_context,
>>
>> -	.capabilities	= PERF_PMU_CAP_NO_NMI,
>> +	.capabilities	= PERF_PMU_CAP_NO_NMI |
>> +			  PERF_PMU_CAP_NO_THROTTLE_STOP,
>>  	.dev		= PMU_NULL_DEV,
>>
>>  	.event_init	= cpu_clock_event_init,
>> @@ -11928,7 +11930,8 @@ static int task_clock_event_init(struct
>> perf_event *event)
>>  static struct pmu perf_task_clock = {
>>  	.task_ctx_nr	= perf_sw_context,
>>
>> -	.capabilities	= PERF_PMU_CAP_NO_NMI,
>> +	.capabilities	= PERF_PMU_CAP_NO_NMI |
>> +			  PERF_PMU_CAP_NO_THROTTLE_STOP,
>>  	.dev		= PMU_NULL_DEV,
>>
>>  	.event_init	= task_clock_event_init,
>>
>>
>> Thanks,
>> Kan
>>
>>
> 

perf regression. Was: [PATCH V4 01/16] perf: Fix the throttle logic for a group

[Alexei Starovoitov]

On Tue, May 20, 2025 at 11:16:29AM -0700, kan.liang@linux.intel.com wrote:
> 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. For all ARCHs, the implementation is similar. Only the event is
> disabled, 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.
> The MAX_INTERRUPTS is set to all throttle events.
> 
> 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
> 
> Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
> ---
>  kernel/events/core.c | 66 ++++++++++++++++++++++++++++++--------------
>  1 file changed, 46 insertions(+), 20 deletions(-)

This patch breaks perf hw events somehow.

After merging this into bpf trees we see random "watchdog: BUG: soft lockup"
with various stack traces followed up:
[   78.620749] Sending NMI from CPU 8 to CPUs 0:
[   76.387722] NMI backtrace for cpu 0
[   76.387722] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Tainted: G           O L      6.15.0-10818-ge0f0ee1c31de #1163 PREEMPT
[   76.387722] Tainted: [O]=OOT_MODULE, [L]=SOFTLOCKUP
[   76.387722] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
[   76.387722] RIP: 0010:_raw_spin_lock_irqsave+0xc/0x40
[   76.387722] Call Trace:
[   76.387722]  <IRQ>
[   76.387722]  hrtimer_try_to_cancel.part.0+0x24/0xe0
[   76.387722]  hrtimer_cancel+0x21/0x40
[   76.387722]  cpu_clock_event_stop+0x64/0x70
[   76.387722]  __perf_event_account_interrupt+0xcf/0x140
[   76.387722]  __perf_event_overflow+0x36/0x340
[   76.387722]  ? hrtimer_start_range_ns+0x2c1/0x420
[   76.387722]  ? kvm_sched_clock_read+0x11/0x20
[   76.387722]  perf_swevent_hrtimer+0xaf/0x100
[   76.387722]  ? cpu_clock_event_add+0x6e/0x90
[   76.387722]  ? event_sched_in+0xc3/0x190
[   76.387722]  ? update_load_avg+0x87/0x3d0
[   76.387722]  ? _raw_spin_unlock+0xe/0x20
[   76.387722]  ? sched_balance_update_blocked_averages+0x59b/0x6a0
[   76.387722]  ? ctx_sched_in+0x184/0x210
[   76.387722]  ? kvm_sched_clock_read+0x11/0x20
[   76.387722]  ? sched_clock_cpu+0x55/0x190
[   76.387722]  ? perf_exclude_event+0x50/0x50
[   76.387722]  __hrtimer_run_queues+0x111/0x290
[   76.387722]  hrtimer_interrupt+0xff/0x240
[   76.387722]  __sysvec_apic_timer_interrupt+0x4f/0x110
[   76.387722]  sysvec_apic_timer_interrupt+0x6c/0x90

After reverting:
commit e800ac51202f ("perf: Only dump the throttle log for the leader")
commit 9734e25fbf5a ("perf: Fix the throttle logic for a group")
everything is back to normal.

There are many ways to reproduce.
Any test that sets up perf hw event followed up by tests that IPIs all cpus.
One way:
selftests/bpf/test_progs -t stacktrace_build_id_nmi
selftests/bpf/test_progs -t unpriv_bpf_disabled

Please take a look.

Re: perf regression. Was: [PATCH V4 01/16] perf: Fix the throttle logic for a group

[Liang, Kan]

Hi Alexei,

On 2025-06-01 8:30 p.m., Alexei Starovoitov wrote:
> On Tue, May 20, 2025 at 11:16:29AM -0700, kan.liang@linux.intel.com wrote:
>> 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. For all ARCHs, the implementation is similar. Only the event is
>> disabled, 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.
>> The MAX_INTERRUPTS is set to all throttle events.
>>
>> 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
>>
>> Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
>> ---
>>  kernel/events/core.c | 66 ++++++++++++++++++++++++++++++--------------
>>  1 file changed, 46 insertions(+), 20 deletions(-)
> 
> This patch breaks perf hw events somehow.
> 
> After merging this into bpf trees we see random "watchdog: BUG: soft lockup"
> with various stack traces followed up:
> [   78.620749] Sending NMI from CPU 8 to CPUs 0:
> [   76.387722] NMI backtrace for cpu 0
> [   76.387722] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Tainted: G           O L      6.15.0-10818-ge0f0ee1c31de #1163 PREEMPT
> [   76.387722] Tainted: [O]=OOT_MODULE, [L]=SOFTLOCKUP
> [   76.387722] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
> [   76.387722] RIP: 0010:_raw_spin_lock_irqsave+0xc/0x40
> [   76.387722] Call Trace:
> [   76.387722]  <IRQ>
> [   76.387722]  hrtimer_try_to_cancel.part.0+0x24/0xe0
> [   76.387722]  hrtimer_cancel+0x21/0x40
> [   76.387722]  cpu_clock_event_stop+0x64/0x70


The issues should be fixed by the patch.
https://lore.kernel.org/lkml/20250528175832.2999139-1-kan.liang@linux.intel.com/

Could you please give it a try?

Thanks,
Kan


> [   76.387722]  __perf_event_account_interrupt+0xcf/0x140
> [   76.387722]  __perf_event_overflow+0x36/0x340
> [   76.387722]  ? hrtimer_start_range_ns+0x2c1/0x420
> [   76.387722]  ? kvm_sched_clock_read+0x11/0x20
> [   76.387722]  perf_swevent_hrtimer+0xaf/0x100
> [   76.387722]  ? cpu_clock_event_add+0x6e/0x90
> [   76.387722]  ? event_sched_in+0xc3/0x190
> [   76.387722]  ? update_load_avg+0x87/0x3d0
> [   76.387722]  ? _raw_spin_unlock+0xe/0x20
> [   76.387722]  ? sched_balance_update_blocked_averages+0x59b/0x6a0
> [   76.387722]  ? ctx_sched_in+0x184/0x210
> [   76.387722]  ? kvm_sched_clock_read+0x11/0x20
> [   76.387722]  ? sched_clock_cpu+0x55/0x190
> [   76.387722]  ? perf_exclude_event+0x50/0x50
> [   76.387722]  __hrtimer_run_queues+0x111/0x290
> [   76.387722]  hrtimer_interrupt+0xff/0x240
> [   76.387722]  __sysvec_apic_timer_interrupt+0x4f/0x110
> [   76.387722]  sysvec_apic_timer_interrupt+0x6c/0x90
> 
> After reverting:
> commit e800ac51202f ("perf: Only dump the throttle log for the leader")
> commit 9734e25fbf5a ("perf: Fix the throttle logic for a group")
> everything is back to normal.
> 
> There are many ways to reproduce.
> Any test that sets up perf hw event followed up by tests that IPIs all cpus.
> One way:
> selftests/bpf/test_progs -t stacktrace_build_id_nmi
> selftests/bpf/test_progs -t unpriv_bpf_disabled
> 
> Please take a look.
> 

Re: perf regression. Was: [PATCH V4 01/16] perf: Fix the throttle logic for a group

[Alexei Starovoitov]

On Mon, Jun 2, 2025 at 5:55 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>
> Hi Alexei,
>
> On 2025-06-01 8:30 p.m., Alexei Starovoitov wrote:
> > On Tue, May 20, 2025 at 11:16:29AM -0700, kan.liang@linux.intel.com wrote:
> >> 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. For all ARCHs, the implementation is similar. Only the event is
> >> disabled, 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.
> >> The MAX_INTERRUPTS is set to all throttle events.
> >>
> >> 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
> >>
> >> Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> >> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
> >> ---
> >>  kernel/events/core.c | 66 ++++++++++++++++++++++++++++++--------------
> >>  1 file changed, 46 insertions(+), 20 deletions(-)
> >
> > This patch breaks perf hw events somehow.
> >
> > After merging this into bpf trees we see random "watchdog: BUG: soft lockup"
> > with various stack traces followed up:
> > [   78.620749] Sending NMI from CPU 8 to CPUs 0:
> > [   76.387722] NMI backtrace for cpu 0
> > [   76.387722] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Tainted: G           O L      6.15.0-10818-ge0f0ee1c31de #1163 PREEMPT
> > [   76.387722] Tainted: [O]=OOT_MODULE, [L]=SOFTLOCKUP
> > [   76.387722] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
> > [   76.387722] RIP: 0010:_raw_spin_lock_irqsave+0xc/0x40
> > [   76.387722] Call Trace:
> > [   76.387722]  <IRQ>
> > [   76.387722]  hrtimer_try_to_cancel.part.0+0x24/0xe0
> > [   76.387722]  hrtimer_cancel+0x21/0x40
> > [   76.387722]  cpu_clock_event_stop+0x64/0x70
>
>
> The issues should be fixed by the patch.
> https://lore.kernel.org/lkml/20250528175832.2999139-1-kan.liang@linux.intel.com/
>
> Could you please give it a try?

Thanks. It fixes it, but the commit log says that
only cpu-clock and task_clock are affected,
which are SW events.

While our tests are locking while setting up:

        struct perf_event_attr attr = {
                .freq = 1,
                .type = PERF_TYPE_HARDWARE,
                .config = PERF_COUNT_HW_CPU_CYCLES,
        };

Is it because we run in x86 VM and HW_CPU_CYCLES is mapped
to cpu-clock sw ?

Re: perf regression. Was: [PATCH V4 01/16] perf: Fix the throttle logic for a group

[Liang, Kan]

On 2025-06-02 12:24 p.m., Alexei Starovoitov wrote:
> On Mon, Jun 2, 2025 at 5:55 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>>
>> Hi Alexei,
>>
>> On 2025-06-01 8:30 p.m., Alexei Starovoitov wrote:
>>> On Tue, May 20, 2025 at 11:16:29AM -0700, kan.liang@linux.intel.com wrote:
>>>> 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. For all ARCHs, the implementation is similar. Only the event is
>>>> disabled, 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.
>>>> The MAX_INTERRUPTS is set to all throttle events.
>>>>
>>>> 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
>>>>
>>>> Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>>>> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
>>>> ---
>>>>  kernel/events/core.c | 66 ++++++++++++++++++++++++++++++--------------
>>>>  1 file changed, 46 insertions(+), 20 deletions(-)
>>>
>>> This patch breaks perf hw events somehow.
>>>
>>> After merging this into bpf trees we see random "watchdog: BUG: soft lockup"
>>> with various stack traces followed up:
>>> [   78.620749] Sending NMI from CPU 8 to CPUs 0:
>>> [   76.387722] NMI backtrace for cpu 0
>>> [   76.387722] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Tainted: G           O L      6.15.0-10818-ge0f0ee1c31de #1163 PREEMPT
>>> [   76.387722] Tainted: [O]=OOT_MODULE, [L]=SOFTLOCKUP
>>> [   76.387722] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
>>> [   76.387722] RIP: 0010:_raw_spin_lock_irqsave+0xc/0x40
>>> [   76.387722] Call Trace:
>>> [   76.387722]  <IRQ>
>>> [   76.387722]  hrtimer_try_to_cancel.part.0+0x24/0xe0
>>> [   76.387722]  hrtimer_cancel+0x21/0x40
>>> [   76.387722]  cpu_clock_event_stop+0x64/0x70
>>
>>
>> The issues should be fixed by the patch.
>> https://lore.kernel.org/lkml/20250528175832.2999139-1-kan.liang@linux.intel.com/
>>
>> Could you please give it a try?
> 
> Thanks. It fixes it, but the commit log says that
> only cpu-clock and task_clock are affected,
> which are SW events.

Yes, only the two SW events are affected.

> 
> While our tests are locking while setting up:
> 
>         struct perf_event_attr attr = {
>                 .freq = 1,
>                 .type = PERF_TYPE_HARDWARE,
>                 .config = PERF_COUNT_HW_CPU_CYCLES,
>         };
> 
> Is it because we run in x86 VM and HW_CPU_CYCLES is mapped
> to cpu-clock sw ?

No, that's from different PMU. We never map HW_CPU_CYCLES to a SW event.
It will error our if the PMU is not available.

I'm not familiar with your test case and env. At least, I saw
PERF_COUNT_SW_CPU_CLOCK is used in the case unpriv_bpf_disabled.

Thanks,
Kan

Re: perf regression. Was: [PATCH V4 01/16] perf: Fix the throttle logic for a group

[Alexei Starovoitov]

On Mon, Jun 2, 2025 at 10:51 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
>
>
>
> On 2025-06-02 12:24 p.m., Alexei Starovoitov wrote:
> > On Mon, Jun 2, 2025 at 5:55 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
> >>
> >> Hi Alexei,
> >>
> >> On 2025-06-01 8:30 p.m., Alexei Starovoitov wrote:
> >>> On Tue, May 20, 2025 at 11:16:29AM -0700, kan.liang@linux.intel.com wrote:
> >>>> 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. For all ARCHs, the implementation is similar. Only the event is
> >>>> disabled, 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.
> >>>> The MAX_INTERRUPTS is set to all throttle events.
> >>>>
> >>>> 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
> >>>>
> >>>> Suggested-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> >>>> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
> >>>> ---
> >>>>  kernel/events/core.c | 66 ++++++++++++++++++++++++++++++--------------
> >>>>  1 file changed, 46 insertions(+), 20 deletions(-)
> >>>
> >>> This patch breaks perf hw events somehow.
> >>>
> >>> After merging this into bpf trees we see random "watchdog: BUG: soft lockup"
> >>> with various stack traces followed up:
> >>> [   78.620749] Sending NMI from CPU 8 to CPUs 0:
> >>> [   76.387722] NMI backtrace for cpu 0
> >>> [   76.387722] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Tainted: G           O L      6.15.0-10818-ge0f0ee1c31de #1163 PREEMPT
> >>> [   76.387722] Tainted: [O]=OOT_MODULE, [L]=SOFTLOCKUP
> >>> [   76.387722] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
> >>> [   76.387722] RIP: 0010:_raw_spin_lock_irqsave+0xc/0x40
> >>> [   76.387722] Call Trace:
> >>> [   76.387722]  <IRQ>
> >>> [   76.387722]  hrtimer_try_to_cancel.part.0+0x24/0xe0
> >>> [   76.387722]  hrtimer_cancel+0x21/0x40
> >>> [   76.387722]  cpu_clock_event_stop+0x64/0x70
> >>
> >>
> >> The issues should be fixed by the patch.
> >> https://lore.kernel.org/lkml/20250528175832.2999139-1-kan.liang@linux.intel.com/
> >>
> >> Could you please give it a try?
> >
> > Thanks. It fixes it, but the commit log says that
> > only cpu-clock and task_clock are affected,
> > which are SW events.
>
> Yes, only the two SW events are affected.
>
> >
> > While our tests are locking while setting up:
> >
> >         struct perf_event_attr attr = {
> >                 .freq = 1,
> >                 .type = PERF_TYPE_HARDWARE,
> >                 .config = PERF_COUNT_HW_CPU_CYCLES,
> >         };
> >
> > Is it because we run in x86 VM and HW_CPU_CYCLES is mapped
> > to cpu-clock sw ?
>
> No, that's from different PMU. We never map HW_CPU_CYCLES to a SW event.
> It will error our if the PMU is not available.
>
> I'm not familiar with your test case and env. At least, I saw
> PERF_COUNT_SW_CPU_CLOCK is used in the case unpriv_bpf_disabled.

I see. The first test was necessary to create throttle conditions
for the 2nd test that actually used cpu-clock.

Feel free to add
Tested-by: Alexei Starovoitov <ast@kernel.org>

I've applied your patch to bpf tree for now to stop the bleeding.
Will drop it when the fix gets to Linus through perf trees.

Re: [PATCH V4 07/16] s390/perf: Remove driver-specific throttle support

[Sumanth Korikkar]

On Tue, May 20, 2025 at 11:16:35AM -0700, kan.liang@linux.intel.com wrote:
> From: Kan Liang <kan.liang@linux.intel.com>
> 
> The throttle support has been added in the generic code. Remove
> the driver-specific throttle support.
> 
> Besides the throttle, perf_event_overflow may return true because of
> event_limit. It already does an inatomic event disable. The pmu->stop
> is not required either.
> 
> Tested-by: Thomas Richter <tmricht@linux.ibm.com>
> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
> Cc: Thomas Richter <tmricht@linux.ibm.com>
> Cc: linux-s390@vger.kernel.org
> ---
>  arch/s390/kernel/perf_cpum_cf.c | 2 --
>  arch/s390/kernel/perf_cpum_sf.c | 5 +----
>  2 files changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/arch/s390/kernel/perf_cpum_cf.c b/arch/s390/kernel/perf_cpum_cf.c
> index e657fad7e376..6a262e198e35 100644
> --- a/arch/s390/kernel/perf_cpum_cf.c
> +++ b/arch/s390/kernel/perf_cpum_cf.c
> @@ -980,8 +980,6 @@ static int cfdiag_push_sample(struct perf_event *event,
>  	}
>  
>  	overflow = perf_event_overflow(event, &data, &regs);
> -	if (overflow)
> -		event->pmu->stop(event, 0);
>  
>  	perf_event_update_userpage(event);
>  	return overflow;
> diff --git a/arch/s390/kernel/perf_cpum_sf.c b/arch/s390/kernel/perf_cpum_sf.c
> index ad22799d8a7d..91469401f2c9 100644
> --- a/arch/s390/kernel/perf_cpum_sf.c
> +++ b/arch/s390/kernel/perf_cpum_sf.c
> @@ -1072,10 +1072,7 @@ static int perf_push_sample(struct perf_event *event,
>  	overflow = 0;
>  	if (perf_event_exclude(event, &regs, sde_regs))
>  		goto out;
> -	if (perf_event_overflow(event, &data, &regs)) {
> -		overflow = 1;
> -		event->pmu->stop(event, 0);
> -	}
> +	overflow = perf_event_overflow(event, &data, &regs);
>  	perf_event_update_userpage(event);
>  out:
>  	return overflow;
> -- 
> 2.38.1

Hi all,

This seems to break POLL_HUP delivery to userspace - when event_limit reaches 0

From perf_event_open man page:
PERF_EVENT_IOC_REFRESH
              Non-inherited overflow counters can use this to enable a
              counter for a number of overflows specified by the
              argument, after which it is disabled.  Subsequent calls of
              this ioctl add the argument value to the current count.  An
              overflow notification with POLL_IN set will happen on each
              overflow until the count reaches 0; when that happens a
              notification with POLL_HUP set is sent and the event is
              disabled.

When the event_limit reaches 0, the POLL_HUP signal is expected to be
sent. Prior to this patch, an explicit call to event->stop() was made,
which may have contributed to ensuring that the POLL_HUP signal was
ultimately delivered. However, after  this change, I often did not
observe the POLL_HUP signal being delivered as expected in the end

Example program:
output:
Computation result: 49951804672
count.hup: 0 count.pollin: 22

Expected output should be:
count.hup: 1 in the end

#define _GNU_SOURCE
#include <time.h>
#include <stdbool.h>
#include <signal.h>
#include <poll.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <time.h>

#include <sys/ioctl.h>
#include <sys/syscall.h>
#include <linux/perf_event.h>

static struct signal_counts {
        int in;
	int out;
	int hup;
	int unknown;
} count;


static unsigned long sample_type = PERF_SAMPLE_IP | PERF_SAMPLE_TID |
		PERF_SAMPLE_TIME | PERF_SAMPLE_ADDR | PERF_SAMPLE_READ |
		PERF_SAMPLE_ID | PERF_SAMPLE_CPU |
		PERF_SAMPLE_PERIOD | PERF_SAMPLE_STREAM_ID | PERF_SAMPLE_RAW;

static void sighandler(int signum, siginfo_t *info, void *uc)
{
	switch(info->si_code) {
                case POLL_IN:  count.in++;  break;
                case POLL_OUT: count.out++; break;
                case POLL_HUP: count.hup++; break;
                default: count.unknown++; break;
        }
}

void generate_load(unsigned long long iterations) {
    unsigned long long sum = 0;
    srand(time(0));

    for (unsigned long long i = 0; i < iterations; ++i) {
        int rnd = rand();
        sum += (rnd ^ (rnd >> 3)) % 1000;
    }
    printf("Computation result: %llu\n", sum);
}

void perf_attr(struct perf_event_attr *pe,
		       unsigned long config, unsigned long period, bool freq,
		       unsigned long bits)
{
	memset(pe, 0, sizeof(struct perf_event_attr));
	pe->size = sizeof(struct perf_event_attr);
	pe->type = PERF_TYPE_HARDWARE;
	pe->config = PERF_COUNT_HW_CPU_CYCLES;
	pe->exclude_kernel = 0;
	pe->sample_period = 50000;
	pe->freq = 1;
	pe->disabled = 1;
	pe->config = config;
	pe->freq = freq;
	pe->sample_type = bits;
}

int main(int argc, char **argv)
{
	int fd, signo = SIGIO, rc = -1;
	struct sigaction sa, sa_old;
	struct perf_event_attr pe;

	perf_attr(&pe, PERF_COUNT_HW_CPU_CYCLES, 50000, 1, sample_type);
	/* Set up overflow handler */
	memset(&sa, 0, sizeof(struct sigaction));
	memset(&sa_old, 0, sizeof(struct sigaction));
	sa.sa_sigaction = sighandler;
	sa.sa_flags = SA_SIGINFO;
	if (sigaction(signo, &sa, &sa_old) < 0)
		goto out;

	fd = syscall(__NR_perf_event_open, &pe, 0, -1, -1, 0);
	if (fd < 0)
		return rc;

	rc = fcntl(fd, F_SETFL, O_RDWR | O_NONBLOCK | O_ASYNC);
	rc |= fcntl(fd, F_SETSIG, signo);
	rc |= fcntl(fd, F_SETOWN, getpid());
	if (rc)
		goto out;

	rc = ioctl(fd, PERF_EVENT_IOC_REFRESH, 2500);
	if (rc)
		goto out;

	generate_load(100000000ULL);
	sigaction(signo, &sa_old, NULL);
	printf("count.hup: %d count.pollin: %d\n", count.hup, count.in);
	close(fd);
	return 0;
out:
	return rc;
}

Thank you,
Sumanth

Re: [PATCH V4 07/16] s390/perf: Remove driver-specific throttle support

[Sumanth Korikkar]

On Wed, Jul 23, 2025 at 10:06:26AM +0200, Sumanth Korikkar wrote:
> On Tue, May 20, 2025 at 11:16:35AM -0700, kan.liang@linux.intel.com wrote:
> > From: Kan Liang <kan.liang@linux.intel.com>
> > 
> > The throttle support has been added in the generic code. Remove
> > the driver-specific throttle support.
> > 
> > Besides the throttle, perf_event_overflow may return true because of
> > event_limit. It already does an inatomic event disable. The pmu->stop
> > is not required either.
> > 
> > Tested-by: Thomas Richter <tmricht@linux.ibm.com>
> > Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
> > Cc: Thomas Richter <tmricht@linux.ibm.com>
> > Cc: linux-s390@vger.kernel.org
> > ---
> >  arch/s390/kernel/perf_cpum_cf.c | 2 --
> >  arch/s390/kernel/perf_cpum_sf.c | 5 +----
> >  2 files changed, 1 insertion(+), 6 deletions(-)
> > 
> > diff --git a/arch/s390/kernel/perf_cpum_cf.c b/arch/s390/kernel/perf_cpum_cf.c
> > index e657fad7e376..6a262e198e35 100644
> > --- a/arch/s390/kernel/perf_cpum_cf.c
> > +++ b/arch/s390/kernel/perf_cpum_cf.c
> > @@ -980,8 +980,6 @@ static int cfdiag_push_sample(struct perf_event *event,
> >  	}
> >  
> >  	overflow = perf_event_overflow(event, &data, &regs);
> > -	if (overflow)
> > -		event->pmu->stop(event, 0);
> >  
> >  	perf_event_update_userpage(event);
> >  	return overflow;
> > diff --git a/arch/s390/kernel/perf_cpum_sf.c b/arch/s390/kernel/perf_cpum_sf.c
> > index ad22799d8a7d..91469401f2c9 100644
> > --- a/arch/s390/kernel/perf_cpum_sf.c
> > +++ b/arch/s390/kernel/perf_cpum_sf.c
> > @@ -1072,10 +1072,7 @@ static int perf_push_sample(struct perf_event *event,
> >  	overflow = 0;
> >  	if (perf_event_exclude(event, &regs, sde_regs))
> >  		goto out;
> > -	if (perf_event_overflow(event, &data, &regs)) {
> > -		overflow = 1;
> > -		event->pmu->stop(event, 0);
> > -	}
> > +	overflow = perf_event_overflow(event, &data, &regs);
> >  	perf_event_update_userpage(event);
> >  out:
> >  	return overflow;
> > -- 
> > 2.38.1
> 
> Hi all,
> 
> This seems to break POLL_HUP delivery to userspace - when event_limit reaches 0
> 
> From perf_event_open man page:
> PERF_EVENT_IOC_REFRESH
>               Non-inherited overflow counters can use this to enable a
>               counter for a number of overflows specified by the
>               argument, after which it is disabled.  Subsequent calls of
>               this ioctl add the argument value to the current count.  An
>               overflow notification with POLL_IN set will happen on each
>               overflow until the count reaches 0; when that happens a
>               notification with POLL_HUP set is sent and the event is
>               disabled.
> 
> When the event_limit reaches 0, the POLL_HUP signal is expected to be
> sent. Prior to this patch, an explicit call to event->stop() was made,
> which may have contributed to ensuring that the POLL_HUP signal was
> ultimately delivered. However, after  this change, I often did not
> observe the POLL_HUP signal being delivered as expected in the end
> 
> Example program:
> output:
> Computation result: 49951804672
> count.hup: 0 count.pollin: 22
> 
> Expected output should be:
> count.hup: 1 in the end
> 
> #define _GNU_SOURCE
> #include <time.h>
> #include <stdbool.h>
> #include <signal.h>
> #include <poll.h>
> #include <fcntl.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> #include <time.h>
> 
> #include <sys/ioctl.h>
> #include <sys/syscall.h>
> #include <linux/perf_event.h>
> 
> static struct signal_counts {
>         int in;
> 	int out;
> 	int hup;
> 	int unknown;
> } count;
> 
> 
> static unsigned long sample_type = PERF_SAMPLE_IP | PERF_SAMPLE_TID |
> 		PERF_SAMPLE_TIME | PERF_SAMPLE_ADDR | PERF_SAMPLE_READ |
> 		PERF_SAMPLE_ID | PERF_SAMPLE_CPU |
> 		PERF_SAMPLE_PERIOD | PERF_SAMPLE_STREAM_ID | PERF_SAMPLE_RAW;
> 
> static void sighandler(int signum, siginfo_t *info, void *uc)
> {
> 	switch(info->si_code) {
>                 case POLL_IN:  count.in++;  break;
>                 case POLL_OUT: count.out++; break;
>                 case POLL_HUP: count.hup++; break;
>                 default: count.unknown++; break;
>         }
> }
> 
> void generate_load(unsigned long long iterations) {
>     unsigned long long sum = 0;
>     srand(time(0));
> 
>     for (unsigned long long i = 0; i < iterations; ++i) {
>         int rnd = rand();
>         sum += (rnd ^ (rnd >> 3)) % 1000;
>     }
>     printf("Computation result: %llu\n", sum);
> }
> 
> void perf_attr(struct perf_event_attr *pe,
> 		       unsigned long config, unsigned long period, bool freq,
> 		       unsigned long bits)
> {
> 	memset(pe, 0, sizeof(struct perf_event_attr));
> 	pe->size = sizeof(struct perf_event_attr);
> 	pe->type = PERF_TYPE_HARDWARE;
> 	pe->config = PERF_COUNT_HW_CPU_CYCLES;
> 	pe->exclude_kernel = 0;
> 	pe->sample_period = 50000;
> 	pe->freq = 1;
> 	pe->disabled = 1;
> 	pe->config = config;
> 	pe->freq = freq;
> 	pe->sample_type = bits;
> }
> 
> int main(int argc, char **argv)
> {
> 	int fd, signo = SIGIO, rc = -1;
> 	struct sigaction sa, sa_old;
> 	struct perf_event_attr pe;
> 
> 	perf_attr(&pe, PERF_COUNT_HW_CPU_CYCLES, 50000, 1, sample_type);
> 	/* Set up overflow handler */
> 	memset(&sa, 0, sizeof(struct sigaction));
> 	memset(&sa_old, 0, sizeof(struct sigaction));
> 	sa.sa_sigaction = sighandler;
> 	sa.sa_flags = SA_SIGINFO;
> 	if (sigaction(signo, &sa, &sa_old) < 0)
> 		goto out;
> 
> 	fd = syscall(__NR_perf_event_open, &pe, 0, -1, -1, 0);
> 	if (fd < 0)
> 		return rc;
> 
> 	rc = fcntl(fd, F_SETFL, O_RDWR | O_NONBLOCK | O_ASYNC);
> 	rc |= fcntl(fd, F_SETSIG, signo);
> 	rc |= fcntl(fd, F_SETOWN, getpid());
> 	if (rc)
> 		goto out;
> 
> 	rc = ioctl(fd, PERF_EVENT_IOC_REFRESH, 2500);
> 	if (rc)
> 		goto out;
> 
> 	generate_load(100000000ULL);
> 	sigaction(signo, &sa_old, NULL);
> 	printf("count.hup: %d count.pollin: %d\n", count.hup, count.in);
> 	close(fd);
> 	return 0;
> out:
> 	return rc;
> }

Hi Kan,

It would be great if you could share your feedback on this issue.

Thank you.

Re: [PATCH V4 07/16] s390/perf: Remove driver-specific throttle support

[Liang, Kan]

Hi Sumanth,

Sorry for the late response. I just came back from Sabbatical yesterday.

On 2025-08-06 1:37 a.m., Sumanth Korikkar wrote:
> On Wed, Jul 23, 2025 at 10:06:26AM +0200, Sumanth Korikkar wrote:
>> On Tue, May 20, 2025 at 11:16:35AM -0700, kan.liang@linux.intel.com wrote:
>>> From: Kan Liang <kan.liang@linux.intel.com>
>>>
>>> The throttle support has been added in the generic code. Remove
>>> the driver-specific throttle support.
>>>
>>> Besides the throttle, perf_event_overflow may return true because of
>>> event_limit. It already does an inatomic event disable. The pmu->stop
>>> is not required either.
>>>
>>> Tested-by: Thomas Richter <tmricht@linux.ibm.com>
>>> Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
>>> Cc: Thomas Richter <tmricht@linux.ibm.com>
>>> Cc: linux-s390@vger.kernel.org
>>> ---
>>>  arch/s390/kernel/perf_cpum_cf.c | 2 --
>>>  arch/s390/kernel/perf_cpum_sf.c | 5 +----
>>>  2 files changed, 1 insertion(+), 6 deletions(-)
>>>
>>> diff --git a/arch/s390/kernel/perf_cpum_cf.c b/arch/s390/kernel/perf_cpum_cf.c
>>> index e657fad7e376..6a262e198e35 100644
>>> --- a/arch/s390/kernel/perf_cpum_cf.c
>>> +++ b/arch/s390/kernel/perf_cpum_cf.c
>>> @@ -980,8 +980,6 @@ static int cfdiag_push_sample(struct perf_event *event,
>>>  	}
>>>  
>>>  	overflow = perf_event_overflow(event, &data, &regs);
>>> -	if (overflow)
>>> -		event->pmu->stop(event, 0);
>>>  
>>>  	perf_event_update_userpage(event);
>>>  	return overflow;
>>> diff --git a/arch/s390/kernel/perf_cpum_sf.c b/arch/s390/kernel/perf_cpum_sf.c
>>> index ad22799d8a7d..91469401f2c9 100644
>>> --- a/arch/s390/kernel/perf_cpum_sf.c
>>> +++ b/arch/s390/kernel/perf_cpum_sf.c
>>> @@ -1072,10 +1072,7 @@ static int perf_push_sample(struct perf_event *event,
>>>  	overflow = 0;
>>>  	if (perf_event_exclude(event, &regs, sde_regs))
>>>  		goto out;
>>> -	if (perf_event_overflow(event, &data, &regs)) {
>>> -		overflow = 1;
>>> -		event->pmu->stop(event, 0);
>>> -	}
>>> +	overflow = perf_event_overflow(event, &data, &regs);
>>>  	perf_event_update_userpage(event);
>>>  out:
>>>  	return overflow;
>>> -- 
>>> 2.38.1
>>
>> Hi all,
>>
>> This seems to break POLL_HUP delivery to userspace - when event_limit reaches 0
>>
>> From perf_event_open man page:
>> PERF_EVENT_IOC_REFRESH
>>               Non-inherited overflow counters can use this to enable a
>>               counter for a number of overflows specified by the
>>               argument, after which it is disabled.  Subsequent calls of
>>               this ioctl add the argument value to the current count.  An
>>               overflow notification with POLL_IN set will happen on each
>>               overflow until the count reaches 0; when that happens a
>>               notification with POLL_HUP set is sent and the event is
>>               disabled.
>>
>> When the event_limit reaches 0, the POLL_HUP signal is expected to be
>> sent. Prior to this patch, an explicit call to event->stop() was made,
>> which may have contributed to ensuring that the POLL_HUP signal was
>> ultimately delivered. However, after  this change, I often did not
>> observe the POLL_HUP signal being delivered as expected in the end

The event_limit case also returns 1. I missed it when fixing the
throttle issue. :(

I didn't use the IOC_REFRESH before. According to the kernel code, it
reschedules all the events of the event->pmu, when the ioctl is invoked.
So we just need to move the event->pmu->stop() to the generic code as
below. It should keep the behavior unchanged.

Could you please try the below fix?

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 14ae43694833..f492cbcd3bb6 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10341,6 +10341,7 @@ static int __perf_event_overflow(struct
perf_event *event,
 		ret = 1;
 		event->pending_kill = POLL_HUP;
 		perf_event_disable_inatomic(event);
+		event->pmu->stop(event, 0);
 	}

 	if (event->attr.sigtrap) {

Thanks,
Kan

>>
>> Example program:
>> output:
>> Computation result: 49951804672
>> count.hup: 0 count.pollin: 22
>>
>> Expected output should be:
>> count.hup: 1 in the end
>>
>> #define _GNU_SOURCE
>> #include <time.h>
>> #include <stdbool.h>
>> #include <signal.h>
>> #include <poll.h>
>> #include <fcntl.h>
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <string.h>
>> #include <unistd.h>
>> #include <time.h>
>>
>> #include <sys/ioctl.h>
>> #include <sys/syscall.h>
>> #include <linux/perf_event.h>
>>
>> static struct signal_counts {
>>         int in;
>> 	int out;
>> 	int hup;
>> 	int unknown;
>> } count;
>>
>>
>> static unsigned long sample_type = PERF_SAMPLE_IP | PERF_SAMPLE_TID |
>> 		PERF_SAMPLE_TIME | PERF_SAMPLE_ADDR | PERF_SAMPLE_READ |
>> 		PERF_SAMPLE_ID | PERF_SAMPLE_CPU |
>> 		PERF_SAMPLE_PERIOD | PERF_SAMPLE_STREAM_ID | PERF_SAMPLE_RAW;
>>
>> static void sighandler(int signum, siginfo_t *info, void *uc)
>> {
>> 	switch(info->si_code) {
>>                 case POLL_IN:  count.in++;  break;
>>                 case POLL_OUT: count.out++; break;
>>                 case POLL_HUP: count.hup++; break;
>>                 default: count.unknown++; break;
>>         }
>> }
>>
>> void generate_load(unsigned long long iterations) {
>>     unsigned long long sum = 0;
>>     srand(time(0));
>>
>>     for (unsigned long long i = 0; i < iterations; ++i) {
>>         int rnd = rand();
>>         sum += (rnd ^ (rnd >> 3)) % 1000;
>>     }
>>     printf("Computation result: %llu\n", sum);
>> }
>>
>> void perf_attr(struct perf_event_attr *pe,
>> 		       unsigned long config, unsigned long period, bool freq,
>> 		       unsigned long bits)
>> {
>> 	memset(pe, 0, sizeof(struct perf_event_attr));
>> 	pe->size = sizeof(struct perf_event_attr);
>> 	pe->type = PERF_TYPE_HARDWARE;
>> 	pe->config = PERF_COUNT_HW_CPU_CYCLES;
>> 	pe->exclude_kernel = 0;
>> 	pe->sample_period = 50000;
>> 	pe->freq = 1;
>> 	pe->disabled = 1;
>> 	pe->config = config;
>> 	pe->freq = freq;
>> 	pe->sample_type = bits;
>> }
>>
>> int main(int argc, char **argv)
>> {
>> 	int fd, signo = SIGIO, rc = -1;
>> 	struct sigaction sa, sa_old;
>> 	struct perf_event_attr pe;
>>
>> 	perf_attr(&pe, PERF_COUNT_HW_CPU_CYCLES, 50000, 1, sample_type);
>> 	/* Set up overflow handler */
>> 	memset(&sa, 0, sizeof(struct sigaction));
>> 	memset(&sa_old, 0, sizeof(struct sigaction));
>> 	sa.sa_sigaction = sighandler;
>> 	sa.sa_flags = SA_SIGINFO;
>> 	if (sigaction(signo, &sa, &sa_old) < 0)
>> 		goto out;
>>
>> 	fd = syscall(__NR_perf_event_open, &pe, 0, -1, -1, 0);
>> 	if (fd < 0)
>> 		return rc;
>>
>> 	rc = fcntl(fd, F_SETFL, O_RDWR | O_NONBLOCK | O_ASYNC);
>> 	rc |= fcntl(fd, F_SETSIG, signo);
>> 	rc |= fcntl(fd, F_SETOWN, getpid());
>> 	if (rc)
>> 		goto out;
>>
>> 	rc = ioctl(fd, PERF_EVENT_IOC_REFRESH, 2500);
>> 	if (rc)
>> 		goto out;
>>
>> 	generate_load(100000000ULL);
>> 	sigaction(signo, &sa_old, NULL);
>> 	printf("count.hup: %d count.pollin: %d\n", count.hup, count.in);
>> 	close(fd);
>> 	return 0;
>> out:
>> 	return rc;
>> }
> 
> Hi Kan,
> 
> It would be great if you could share your feedback on this issue.
> 
> Thank you.

Re: [PATCH V4 07/16] s390/perf: Remove driver-specific throttle support

[Sumanth Korikkar]

> >> Hi all,
> >>
> >> This seems to break POLL_HUP delivery to userspace - when event_limit reaches 0
> >>
> >> From perf_event_open man page:
> >> PERF_EVENT_IOC_REFRESH
> >>               Non-inherited overflow counters can use this to enable a
> >>               counter for a number of overflows specified by the
> >>               argument, after which it is disabled.  Subsequent calls of
> >>               this ioctl add the argument value to the current count.  An
> >>               overflow notification with POLL_IN set will happen on each
> >>               overflow until the count reaches 0; when that happens a
> >>               notification with POLL_HUP set is sent and the event is
> >>               disabled.
> >>
> >> When the event_limit reaches 0, the POLL_HUP signal is expected to be
> >> sent. Prior to this patch, an explicit call to event->stop() was made,
> >> which may have contributed to ensuring that the POLL_HUP signal was
> >> ultimately delivered. However, after  this change, I often did not
> >> observe the POLL_HUP signal being delivered as expected in the end
> 
> The event_limit case also returns 1. I missed it when fixing the
> throttle issue. :(
> 
> I didn't use the IOC_REFRESH before. According to the kernel code, it
> reschedules all the events of the event->pmu, when the ioctl is invoked.
> So we just need to move the event->pmu->stop() to the generic code as
> below. It should keep the behavior unchanged.
> 
> Could you please try the below fix?
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 14ae43694833..f492cbcd3bb6 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -10341,6 +10341,7 @@ static int __perf_event_overflow(struct
> perf_event *event,
>  		ret = 1;
>  		event->pending_kill = POLL_HUP;
>  		perf_event_disable_inatomic(event);
> +		event->pmu->stop(event, 0);
>  	}
> 
>  	if (event->attr.sigtrap) {
> 
> Thanks,
> Kan

Hi Kan,

The above fix works.

Tested-by: Sumanth Korikkar <sumanthk@linux.ibm.com>

Thank you