[PATCH 0/2] perf mmap: Fix replayed and skipped records

[PATCH 0/2] perf mmap: Fix replayed and skipped records

[Paul Moses]

perf_mmap__push() can copy through one producer head but save an earlier
head in prev, causing the next drain to write the same records again.

The race duplicates a small fraction of samples but reproduced in most
runs. Across 20 unpatched runs on two source baselines, each containing
500,000 unique samples:
- Replay occurred in 15 of 20 runs; five had no duplicates.
- Affected runs contained 14–53 duplicate samples each,
  representing 0.0028–0.0106% of their 500,000 unique samples.
- The longest replay duplicated a block of 53 consecutive
  timestamped records byte-for-byte.
- Overall, 485 duplicate samples were recorded alongside
  10 million unique samples (0.00485%).

The patched builds had no duplicates in the corresponding 20 runs.

The second issue is that perf_mmap__read_done() advances the forward
cursor to the producer head even when unread records remain. perf kvm
stat live processes at most 25 records per mmap pass before calling
read_done(), so it can skip the remaining records without a race.

In three runs without patch 2, perf kvm stat live reported 111,894,
6,433, and 6,838 intervals against roughly 200,600 independently
counted KVM exits per run. With patch 2, the reported counts were
within 1–3 intervals of the independent counts. Separate cursor tracing
confirmed that read_done() advanced past unread records without the
fix and preserved the consumed cursor with it.

Paul Moses (2):
  perf mmap: Use the head selected by read_init
  perf mmap: Preserve the consumed cursor in forward mode

 tools/lib/perf/mmap.c  | 2 +-
 tools/perf/util/mmap.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

-- 
2.56.GIT

[PATCH 1/2] perf mmap: Use the head selected by read_init

[Paul Moses]

perf_mmap__push() reads the producer head before calling
perf_mmap__read_init(), which reads it again to select the range to copy.

If the producer advances between those reads, push() copies through the
newer head but stores the earlier head in prev. The next drain starts at
that earlier position and can write the same records to perf.data again.

Use the head selected by read_init(): end for forward buffers and start
for overwrite buffers. Save it before the copy loop advances start so
prev matches the range that was copied.

Fixes: 8872481bd048 ("perf mmap: Introduce perf_mmap__read_init()")
Signed-off-by: Paul Moses <p@1g4.org>
---
 tools/perf/util/mmap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/mmap.c b/tools/perf/util/mmap.c
index 358e70c4f3edd077c9752115329f29040f1c2a76..6dcc4a9304b926c4a8d4f401e40bfd79d18dd199 100644
--- a/tools/perf/util/mmap.c
+++ b/tools/perf/util/mmap.c
@@ -330,7 +330,7 @@ int mmap__mmap(struct mmap *map, struct mmap_params *mp, int fd, struct perf_cpu
 int perf_mmap__push(struct mmap *md, void *to,
 		    int push(struct mmap *map, void *to, void *buf, size_t size))
 {
-	u64 head = perf_mmap__read_head(&md->core);
+	u64 head;
 	unsigned char *data = md->core.base + page_size;
 	unsigned long size;
 	void *buf;
@@ -340,6 +340,7 @@ int perf_mmap__push(struct mmap *md, void *to,
 	if (rc < 0)
 		return (rc == -EAGAIN) ? 1 : -1;
 
+	head = md->core.overwrite ? md->core.start : md->core.end;
 	size = md->core.end - md->core.start;
 
 	if ((md->core.start & md->core.mask) + size != (md->core.end & md->core.mask)) {
-- 
2.56.GIT

[PATCH 2/2] perf mmap: Preserve the consumed cursor in forward mode

[Paul Moses]

perf_mmap__read_event() updates prev as it consumes forward-buffer
records. perf_mmap__read_done() then overwrites prev with a fresh
producer head, even when unread records remain.

perf_kvm__mmap_read_idx() stops after 25 records per mmap and calls
read_done(). If more records are pending, prev advances past them. The
next drain starts at that new position and skips the unread remainder.

An event arriving after the last empty read_event() and before
read_done() can also be skipped. read_done() does not itself update the
kernel's data_tail.

Only refresh prev for overwrite buffers, which need the current head as
the end of the next backward read. Leave the forward cursor at the
consumed position.

Fixes: 53172f9057e9 ("perf kvm: Switch to new perf_mmap__read_event() interface")
Signed-off-by: Paul Moses <p@1g4.org>
---
 tools/lib/perf/mmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/perf/mmap.c b/tools/lib/perf/mmap.c
index ec124eb0ec0abbcc4a395eca7478129a74309965..ab3adf1d8baf933d5386faf743f178b0a7da44c5 100644
--- a/tools/lib/perf/mmap.c
+++ b/tools/lib/perf/mmap.c
@@ -200,7 +200,7 @@ void perf_mmap__read_done(struct perf_mmap *map)
 	/*
 	 * Check if event was unmapped due to a POLLHUP/POLLERR.
 	 */
-	if (!refcount_read(&map->refcnt))
+	if (!refcount_read(&map->refcnt) || !map->overwrite)
 		return;
 
 	map->prev = perf_mmap__read_head(map);
-- 
2.56.GIT