[PATCH] perf/core: Strengthen userpage update ordering

[Thaumy Cheng]

The perf event mmap userpage uses a sequence counter to let userspace
obtain a consistent snapshot of its data. The existing compiler
barriers reflect the original self-monitoring use case, where the
counter was updated and consumed on the same CPU.

Some consumers also use the userpage only for time conversion and may
read it from a CPU other than the one updating the event. For example,
perf_read_tsc_conversion() reads the time conversion fields
without constraining the caller to the event's CPU.

Make the publication ordering explicit for such readers by replacing the
compiler barriers around the userpage payload update with smp_wmb().
Document that cross-CPU readers of the time conversion fields must use
read memory barriers and reject odd or changed sequence values.

This does not change the UAPI layout or the values exposed to
userspace. It strengthens the ordering guarantee for cross-CPU readers
on weakly ordered architectures.

Signed-off-by: Thaumy Cheng <thaumy.love@xxxxxxxxx>
---
 include/uapi/linux/perf_event.h       | 6 ++++--
 kernel/events/core.c                  | 6 ++++--
 tools/include/uapi/linux/perf_event.h | 6 ++++--
 tools/perf/design.txt                 | 6 ++++--
 4 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index fd10aa8d697f..a7db00b9b455 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -629,8 +629,10 @@ struct perf_event_mmap_page {
 	 *     barrier();
 	 *   } while (pc->lock != seq);
 	 *
-	 * NOTE: for obvious reason this only works on self-monitoring
-	 *       processes.
+	 * NOTE: Reading the hardware counter as shown above only works for
+	 *       self-monitoring processes. A reader on another CPU may snapshot
+	 *       the time conversion fields, but must use rmb() around the field
+	 *       reads and retry if lock is odd or changes.
 	 */
 	__u32	lock;			/* seqlock for synchronization */
 	__u32	index;			/* hardware event identifier */
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 89b40e439717..72605776273f 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6852,7 +6852,8 @@ void perf_event_update_userpage(struct perf_event *event)
 	userpg = rb->user_page;
 
 	++userpg->lock;
-	barrier();
+	/* Publish the odd lock value before updating the payload. */
+	smp_wmb();
 	userpg->index = perf_event_index(event);
 	userpg->offset = perf_event_count(event, false);
 	if (userpg->index)
@@ -6866,7 +6867,8 @@ void perf_event_update_userpage(struct perf_event *event)
 
 	arch_perf_update_userpage(event, userpg, now);
 
-	barrier();
+	/* Publish the payload before the final lock update. */
+	smp_wmb();
 	++userpg->lock;
 	preempt_enable();
 unlock:
diff --git a/tools/include/uapi/linux/perf_event.h b/tools/include/uapi/linux/perf_event.h
index fd10aa8d697f..a7db00b9b455 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -629,8 +629,10 @@ struct perf_event_mmap_page {
 	 *     barrier();
 	 *   } while (pc->lock != seq);
 	 *
-	 * NOTE: for obvious reason this only works on self-monitoring
-	 *       processes.
+	 * NOTE: Reading the hardware counter as shown above only works for
+	 *       self-monitoring processes. A reader on another CPU may snapshot
+	 *       the time conversion fields, but must use rmb() around the field
+	 *       reads and retry if lock is odd or changes.
 	 */
 	__u32	lock;			/* seqlock for synchronization */
 	__u32	index;			/* hardware event identifier */
diff --git a/tools/perf/design.txt b/tools/perf/design.txt
index aa8cfeabb743..111afc90c442 100644
--- a/tools/perf/design.txt
+++ b/tools/perf/design.txt
@@ -316,8 +316,10 @@ struct perf_event_mmap_page {
          *     barrier();
          *   } while (pc->lock != seq);
          *
-         * NOTE: for obvious reason this only works on self-monitoring
-         *       processes.
+         * NOTE: Reading the hardware counter as shown above only works for
+         *       self-monitoring processes. A reader on another CPU may snapshot
+         *       the time conversion fields, but must use rmb() around the field
+         *       reads and retry if lock is odd or changes.
          */
         __u32   lock;                   /* seqlock for synchronization */
         __u32   index;                  /* hardware counter identifier */
-- 
2.55.0