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

[Paul Moses]

On Tuesday, October 20th, 2026 at 12:20 PM, Paul Moses <p@xxxxxxx> wrote:

> 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@xxxxxxx>
> ---
>  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
> 
> 

This originally occurred while profiling packet flows and a duplicate
marker was "luckily" detected by validation. It might be statistically 
insignificant to most workloads, but when looking at sequences, it is 
a real pain.

Below is a more standardized reproduction of the bug than originally 
used.

Let me know if anything else is needed.

results:

  unpatched 
  BUG: push=17274 copied=[120,144) saved_prev=120 replay_bytes=24
  replay: records=64548 unique=49974 duplicates=14574 cursor_mismatches=14574 capture_bytes=1549152
  replay: result=FAIL

  patched 
  replay: records=49994 unique=49994 duplicates=0 cursor_mismatches=0 capture_bytes=1199856
  replay: result=PASS

  Under GDB, two unpatched runs both reported the first push as:

  BUG: push=0 copied=[0,48816) saved_prev=0 replay_bytes=48816


# Force the two producer-head reads in perf_mmap__push() to straddle
# producer progress. In the parent, the first read happens in
# perf_mmap__push() and the second in perf_mmap__read_init(). The child is
# detached and keeps sampling while GDB sleeps in the parent.
#
#   gdb -q -batch -x replay.gdb ./replay
#
# The breakpoint is disabled after its first hit so only the first push is
# delayed.

set pagination off
set confirm off
set follow-fork-mode parent
set detach-on-fork on

break perf_mmap__read_init
commands
  silent
  disable
  shell sleep 0.2
  continue
end

run

# Preserve the PASS/FAIL status as GDB's exit status.
if $_exitcode != 0
  quit 1
end
quit



/*
 * perf_mmap__push() saves a head older than the range it
 * copies, so the next push copies the same records again.
 *
 * A child runs a busy loop for five seconds. The parent samples it with
 * PERF_COUNT_SW_CPU_CLOCK, maps the real perf ring, and calls perf_mmap__push()
 * while the child runs.
 *
 * Each successful forward push that copies data must leave
 * core.prev == core.end.  The capture loop checks that invariant directly.
 * Every copied segment is tagged with its push number, so duplicate records
 * identify the two pushes that copied them; the record hash includes the PID,
 * TID, and perf timestamp.
 *
 * replay.gdb widens the window between the two producer-head reads without
 * changing the implementation.
 *
 * Build:
 *   K=/usr/src/linux-7.x
 *   cc -O2 -g -ffunction-sections -fdata-sections -Wl,--gc-sections \
 *      -D_GNU_SOURCE \
 *      -I$K/tools/lib/perf/include -I$K/tools/lib/perf -I$K/tools/lib \
 *      -I$K/tools/include -I$K/tools/arch/x86/include \
 *      -I$K/tools/arch/x86/include/uapi -I$K/tools/include/uapi \
 *      -I$K/tools/perf -I$K/tools/perf/util \
 *      replay.c $K/tools/perf/util/mmap.c $K/tools/lib/perf/mmap.c \
 *      $K/tools/lib/perf/core.c $K/tools/lib/perf/lib.c \
 *      $K/tools/lib/zalloc.c -o replay
 */

#include <errno.h>
#include <inttypes.h>
#include <linux/perf_event.h>
#include <poll.h>
#include <signal.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <sys/mman.h>
#include <sys/syscall.h>
#include <sys/wait.h>
#include <time.h>
#include <unistd.h>

#include <perf/core.h>
#include <perf/event.h>
#include <perf/mmap.h>
#include <internal/lib.h>
#include <internal/mmap.h>
#include "mmap.h"

#define RING_PAGES	128
#define WORKLOAD_MS	5000
#define SAMPLE_PERIOD_NS	100000
#define CAPTURE_TIMEOUT_MS 30000

static volatile sig_atomic_t interrupted;

static void interrupt_handler(int sig)
{
	(void)sig;
	interrupted = 1;
}

struct push_seg {
	size_t		off;
	unsigned long	push;
};

struct capture {
	unsigned char	*buf;
	size_t		len;
	size_t		cap;
	struct push_seg	*segs;
	size_t		nsegs;
	size_t		segcap;
	unsigned long	push;
};

struct rec {
	uint64_t	hash;
	size_t		off;
	uint32_t	size;
	unsigned long	push;
};

/* PERF_SAMPLE_TID | PERF_SAMPLE_TIME, in ABI order. */
struct clock_sample {
	struct perf_event_header header;
	uint32_t pid, tid;
	uint64_t time;
};

static void die(const char *what)
{
	fprintf(stderr, "%s: %s\n", what, strerror(errno));
	exit(1);
}

static int64_t now_ms(void)
{
	struct timespec ts;

	if (clock_gettime(CLOCK_MONOTONIC, &ts) < 0)
		return -1;
	return (int64_t)ts.tv_sec * 1000 + ts.tv_nsec / 1000000;
}

static int worker_main(int ready_fd, int go_fd)
{
	volatile uint64_t work = 0;
	int64_t now, deadline;
	char byte;

	if (write(ready_fd, "R", 1) != 1)
		die("write ready");
	if (read(go_fd, &byte, 1) != 1 || byte != 'G') {
		fprintf(stderr, "worker did not receive start command\n");
		return 1;
	}

	now = now_ms();
	if (now < 0)
		die("clock_gettime");
	deadline = now + WORKLOAD_MS;
	do {
		unsigned int i;

		for (i = 0; i < 65536; i++)
			work++;
		now = now_ms();
		if (now < 0)
			die("clock_gettime");
	} while (!interrupted && now < deadline);
	return interrupted ? 1 : 0;
}

static int open_cpu_clock(pid_t pid)
{
	struct perf_event_attr attr;
	int fd;

	memset(&attr, 0, sizeof(attr));
	attr.type = PERF_TYPE_SOFTWARE;
	attr.size = sizeof(attr);
	attr.config = PERF_COUNT_SW_CPU_CLOCK;
	attr.sample_period = SAMPLE_PERIOD_NS;
	attr.sample_type = PERF_SAMPLE_TID | PERF_SAMPLE_TIME;
	attr.disabled = 1;
	attr.exclude_kernel = 1;
	attr.exclude_hv = 1;

	fd = syscall(__NR_perf_event_open, &attr, pid, -1, -1, 0);
	if (fd < 0) {
		fprintf(stderr, "perf_event_open(%d): %s\n", pid, strerror(errno));
		return -1;
	}

	return fd;
}

static int map_ring(int fd, struct perf_mmap *map)
{
	struct perf_mmap_param mp = {
		.prot = PROT_READ | PROT_WRITE,
		.mask = RING_PAGES * page_size - 1,
	};

	perf_mmap__init(map, NULL, false, NULL);
	if (perf_mmap__mmap(map, &mp, fd, (struct perf_cpu){ .cpu = 0 })) {
		fprintf(stderr, "perf_mmap__mmap: %s\n", strerror(errno));
		return -1;
	}
	refcount_set(&map->refcnt, 2);
	return 0;
}

static int capture_push(struct mmap *map, void *to, void *buf, size_t size)
{
	struct capture *c = to;
	size_t cap;
	void *p;

	(void)map;
	if (!size)
		return 0;
	if (c->len + size < c->len)
		return -1;
	if (c->len + size > c->cap) {
		cap = c->cap ? c->cap : 65536;
		while (cap < c->len + size)
			cap *= 2;
		p = realloc(c->buf, cap);
		if (!p)
			return -1;
		c->buf = p;
		c->cap = cap;
	}
	if (c->nsegs == c->segcap) {
		size_t segcap = c->segcap ? c->segcap * 2 : 4096;
		struct push_seg *segs;

		segs = realloc(c->segs, segcap * sizeof(*segs));
		if (!segs)
			return -1;
		c->segs = segs;
		c->segcap = segcap;
	}
	memcpy(c->buf + c->len, buf, size);
	c->segs[c->nsegs].off = c->len;
	c->segs[c->nsegs].push = c->push;
	c->nsegs++;
	c->len += size;
	return 0;
}

static uint64_t hash_record(const unsigned char *p, size_t len)
{
	uint64_t h = 1469598103934665603ULL;
	size_t i;

	for (i = 0; i < len; i++) {
		h ^= p[i];
		h *= 1099511628211ULL;
	}
	return h;
}

static int rec_cmp(const void *a, const void *b, void *arg)
{
	const struct rec *ra = a, *rb = b;
	const unsigned char *buf = arg;

	if (ra->hash < rb->hash)
		return -1;
	if (ra->hash > rb->hash)
		return 1;
	if (ra->size < rb->size)
		return -1;
	if (ra->size > rb->size)
		return 1;
	return memcmp(buf + ra->off, buf + rb->off, ra->size);
}

static unsigned long push_for_offset(const struct capture *c, size_t off)
{
	size_t lo = 0, hi = c->nsegs;

	while (lo < hi) {
		size_t mid = lo + (hi - lo) / 2;

		if (c->segs[mid].off <= off)
			lo = mid + 1;
		else
			hi = mid;
	}
	return lo ? c->segs[lo - 1].push : 0;
}

static uint64_t time_of(const unsigned char *rec)
{
	struct clock_sample sample;

	memcpy(&sample, rec, sizeof(sample));
	return sample.time;
}

static int report(const struct capture *c, unsigned long cursor_mismatches)
{
	struct rec *recs = NULL;
	size_t nrec = 0, cap = 0, off = 0;
	unsigned long total = 0, unique = 0, duplicates = 0;
	int shown = 0;
	size_t i;

	while (off + sizeof(struct perf_event_header) <= c->len) {
		struct perf_event_header hdr;

		memcpy(&hdr, c->buf + off, sizeof(hdr));
		if (hdr.size < sizeof(hdr) || hdr.size > c->len - off)
			break;
		if (hdr.type == PERF_RECORD_SAMPLE) {
			if (hdr.size != sizeof(struct clock_sample))
				break;
			total++;
			if (nrec == cap) {
				cap = cap ? cap * 2 : 4096;
				recs = realloc(recs, cap * sizeof(*recs));
				if (!recs)
					die("realloc records");
			}
			recs[nrec].hash = hash_record(c->buf + off, hdr.size);
			recs[nrec].off = off;
			recs[nrec].size = hdr.size;
			recs[nrec].push = push_for_offset(c, off);
			nrec++;
		}
		off += hdr.size;
	}

	if (off != c->len) {
		fprintf(stderr, "invalid or truncated perf record at byte %zu\n", off);
		free(recs);
		return 1;
	}
	if (!total) {
		fprintf(stderr, "no cpu-clock samples captured\n");
		free(recs);
		return 1;
	}

	qsort_r(recs, nrec, sizeof(*recs), rec_cmp, c->buf);

	for (i = 0; i < nrec; i++) {
		if (!i || rec_cmp(&recs[i - 1], &recs[i], c->buf)) {
			unique++;
			continue;
		}
		duplicates++;
		if (!shown) {
			struct rec *a = &recs[i - 1], *b = &recs[i];

			printf("replay: identical record at %zu (push %lu) and "
			       "%zu (push %lu), size=%u time=%" PRIu64 " memcmp=0\n",
			       a->off, a->push, b->off, b->push, a->size,
			       time_of(c->buf + a->off));
			shown = 1;
		}
	}

	printf("replay: records=%lu unique=%lu duplicates=%lu "
	       "cursor_mismatches=%lu capture_bytes=%zu\n",
	       total, unique, duplicates, cursor_mismatches, c->len);
	printf("replay: result=%s\n",
	       duplicates || cursor_mismatches ? "FAIL" : "PASS");
	free(recs);
	if (fflush(stdout) == EOF || ferror(stdout)) {
		fprintf(stderr, "failed to write replay report\n");
		return 1;
	}
	return duplicates || cursor_mismatches ? 1 : 0;
}

int main(void)
{
	struct capture c = { 0 };
	struct mmap md = { 0 };
	int ready[2], go[2];
	pid_t worker;
	int fd = -1, status, worker_done = 0, empty = 0, result = 1;
	size_t last = 0;
	unsigned long pushes, cursor_mismatches = 0, mismatch_prints = 0;
	int64_t deadline;
	struct sigaction sa = { .sa_handler = interrupt_handler };

	libperf_init(NULL);
	sigemptyset(&sa.sa_mask);
	sigaction(SIGINT, &sa, NULL);
	sigaction(SIGTERM, &sa, NULL);
	signal(SIGPIPE, SIG_IGN);

	if (pipe(ready) || pipe(go))
		die("pipe");

	worker = fork();
	if (worker < 0)
		die("fork");
	if (!worker) {
		close(ready[0]);
		close(go[1]);
		_exit(worker_main(ready[1], go[0]));
	}

	close(ready[1]);
	close(go[0]);
	{
		struct pollfd pfd = { .fd = ready[0], .events = POLLIN };
		char byte;

		if (poll(&pfd, 1, 10000) <= 0 ||
		    read(ready[0], &byte, 1) != 1 || byte != 'R') {
			fprintf(stderr, "worker did not become ready\n");
			goto out;
		}
	}

	fd = open_cpu_clock(worker);
	if (fd < 0)
		goto out;
	if (map_ring(fd, &md.core))
		goto out;
	if (ioctl(fd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
		perror("ENABLE");
		goto out;
	}

	if (write(go[1], "G", 1) != 1) {
		perror("write go");
		goto out;
	}

	deadline = now_ms() + CAPTURE_TIMEOUT_MS;
	for (pushes = 0; ; pushes++) {
		size_t before = c.len;
		uint64_t old_prev = md.core.prev;
		int ret;

		c.push = pushes;
		ret = perf_mmap__push(&md, &c, capture_push);

		if (interrupted) {
			fprintf(stderr, "capture interrupted\n");
			goto out;
		}
		if (ret < 0) {
			fprintf(stderr, "push failed: %d\n", ret);
			goto out;
		}
		if (ret == 0 && c.len > before && md.core.prev != md.core.end) {
			cursor_mismatches++;
			if (mismatch_prints++ < 5)
				fprintf(stderr,
					"BUG: push=%lu copied=[%" PRIu64 ",%" PRIu64
					") saved_prev=%" PRIu64
					" replay_bytes=%" PRIu64 "\n",
					pushes, old_prev, (uint64_t)md.core.end,
					(uint64_t)md.core.prev,
					(uint64_t)(md.core.end - md.core.prev));
		}
		if ((pushes & 0x3ff) == 0) {
			int64_t now = now_ms();
			pid_t pid = 0;

			if (now < 0 || now >= deadline) {
				fprintf(stderr, "capture timed out\n");
				goto out;
			}
			if (!worker_done)
				pid = waitpid(worker, &status, WNOHANG);
			if (pid == worker) {
				worker_done = 1;
				if (!WIFEXITED(status) || WEXITSTATUS(status)) {
					fprintf(stderr, "worker failed: %d\n", status);
					goto out;
				}
			} else if (pid < 0 && errno != EINTR) {
				perror("waitpid worker");
				goto out;
			}
		}
		if (worker_done) {
			if (c.len == last) {
				if (++empty > 1000)
					break;
			} else {
				empty = 0;
			}
			last = c.len;
		}
	}

	if (ioctl(fd, PERF_EVENT_IOC_DISABLE, 0) < 0) {
		perror("DISABLE");
		goto out;
	}
	result = report(&c, cursor_mismatches);
out:
	if (!worker_done) {
		kill(worker, SIGKILL);
		while (waitpid(worker, &status, 0) < 0 && errno == EINTR)
			;
	}
	if (md.core.base)
		perf_mmap__munmap(&md.core);
	if (fd >= 0)
		close(fd);
	close(ready[0]);
	close(go[1]);
	free(c.buf);
	free(c.segs);
	return result;
}