diff mbox series

[V2,2/2] perf/x86/intel/ds: Use the size from each PEBS record

Message ID 20230329132921.1429538-2-kan.liang@linux.intel.com
State New
Headers
Series [V2,1/2] perf: Add sched_task callback during ctx reschedule |

Commit Message

Liang, Kan March 29, 2023, 1:29 p.m. UTC 
  From: Kan Liang <kan.liang@linux.intel.com>

The kernel warning for the unexpected PEBS record can also be observed
during a context switch, when the below commands are running in parallel
for a while on SPR.

  while true; do perf record --no-buildid -a --intr-regs=AX -e
  cpu/event=0xd0,umask=0x81/pp -c 10003 -o /dev/null ./triad; done &

  while true; do perf record -o /tmp/out -W -d -e
  '{ld_blocks.store_forward:period=1000000,
  MEM_TRANS_RETIRED.LOAD_LATENCY:u:precise=2:ldlat=4}'
  -c 1037 ./triad; done
  *The triad program is just the generation of loads/stores.

The current PEBS code assumes that all the PEBS records in the DS buffer
have the same size, aka cpuc->pebs_record_size. It's true for the most
cases, since the DS buffer is always flushed in every context switch.

However, there is a corner case that breaks the assumption.
A system-wide PEBS event with the large PEBS config may be enabled
during a context switch. Some PEBS records for the system-wide PEBS may
be generated while the old task is sched out but the new one hasn't been
sched in yet. When the new task is sched in, the cpuc->pebs_record_size
may be updated for the per-task PEBS events. So the existing system-wide
PEBS records have a different size from the later PEBS records.

Two methods were considered to fix the issue.
One is to flush the DS buffer for the system-wide PEBS right before the
new task sched in. It has to be done in the generic code via the
sched_task() call back. However, the sched_task() is shared among
different ARCHs. The movement may impact other ARCHs, e.g., AMD BRS
requires the sched_task() is called after the PMU has started on a
ctxswin. The method is dropped.

The other method is implemented here. It doesn't assume that all the
PEBS records have the same size any more. The size from each PEBS record
is used to parse the record. For the previous platform (PEBS format < 4),
which doesn't support adaptive PEBS, there is nothing changed.

Reported-by: Stephane Eranian <eranian@google.com>
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
---

Changes since V1:
- Del the unused variable 'cpuc'

 arch/x86/events/intel/ds.c        | 32 +++++++++++++++++++++++++------
 arch/x86/include/asm/perf_event.h |  6 ++++++
 2 files changed, 32 insertions(+), 6 deletions(-)
diff mbox series

Patch

diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index a2e566e53076..d34a610c93f9 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -1546,6 +1546,15 @@  static inline u64 get_pebs_status(void *n)
 	return ((struct pebs_basic *)n)->applicable_counters;
 }
 
+static inline u64 get_pebs_size(void *n)
+{
+	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
+
+	if (x86_pmu.intel_cap.pebs_format < 4)
+		return cpuc->pebs_record_size;
+	return intel_adaptive_pebs_size(((struct pebs_basic *)n)->format_size);
+}
+
 #define PERF_X86_EVENT_PEBS_HSW_PREC \
 		(PERF_X86_EVENT_PEBS_ST_HSW | \
 		 PERF_X86_EVENT_PEBS_LD_HSW | \
@@ -1903,9 +1912,9 @@  static void setup_pebs_adaptive_sample_data(struct perf_event *event,
 		}
 	}
 
-	WARN_ONCE(next_record != __pebs + (format_size >> 48),
+	WARN_ONCE(next_record != __pebs + intel_adaptive_pebs_size(format_size),
 			"PEBS record size %llu, expected %llu, config %llx\n",
-			format_size >> 48,
+			intel_adaptive_pebs_size(format_size),
 			(u64)(next_record - __pebs),
 			basic->format_size);
 }
@@ -1927,7 +1936,7 @@  get_next_pebs_record_by_bit(void *base, void *top, int bit)
 	if (base == NULL)
 		return NULL;
 
-	for (at = base; at < top; at += cpuc->pebs_record_size) {
+	for (at = base; at < top; at += get_pebs_size(at)) {
 		unsigned long status = get_pebs_status(at);
 
 		if (test_bit(bit, (unsigned long *)&status)) {
@@ -2030,7 +2039,6 @@  __intel_pmu_pebs_event(struct perf_event *event,
 					    struct perf_sample_data *,
 					    struct pt_regs *))
 {
-	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
 	struct hw_perf_event *hwc = &event->hw;
 	struct x86_perf_regs perf_regs;
 	struct pt_regs *regs = &perf_regs.regs;
@@ -2054,7 +2062,7 @@  __intel_pmu_pebs_event(struct perf_event *event,
 	while (count > 1) {
 		setup_sample(event, iregs, at, data, regs);
 		perf_event_output(event, data, regs);
-		at += cpuc->pebs_record_size;
+		at += get_pebs_size(at);
 		at = get_next_pebs_record_by_bit(at, top, bit);
 		count--;
 	}
@@ -2278,7 +2286,19 @@  static void intel_pmu_drain_pebs_icl(struct pt_regs *iregs, struct perf_sample_d
 		return;
 	}
 
-	for (at = base; at < top; at += cpuc->pebs_record_size) {
+	/*
+	 * The cpuc->pebs_record_size may be different from the
+	 * size of each PEBS record. For example, a system-wide
+	 * PEBS event with the large PEBS config may be enabled
+	 * during a context switch. Some PEBS records for the
+	 * system-wide PEBS may be generated while the old task
+	 * is sched out but the new one isn't sched in. When the
+	 * new task is sched in, the cpuc->pebs_record_size may
+	 * be updated for the per-task PEBS events. So the
+	 * existing system-wide PEBS records have a different
+	 * size from the later PEBS records.
+	 */
+	for (at = base; at < top; at += get_pebs_size(at)) {
 		u64 pebs_status;
 
 		pebs_status = get_pebs_status(at) & cpuc->pebs_enabled;
diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h
index 8fc15ed5e60b..ad5655bb90f6 100644
--- a/arch/x86/include/asm/perf_event.h
+++ b/arch/x86/include/asm/perf_event.h
@@ -386,6 +386,12 @@  static inline bool is_topdown_idx(int idx)
 /*
  * Adaptive PEBS v4
  */
+#define INTEL_ADAPTIVE_PEBS_SIZE_OFF		48
+
+static inline u64 intel_adaptive_pebs_size(u64 format_size)
+{
+	return (format_size >> INTEL_ADAPTIVE_PEBS_SIZE_OFF);
+}
 
 struct pebs_basic {
 	u64 format_size;