Re: [RFC PATCH v4 3/9] mm/damon: add perf-event overflow handler feeding the report ring

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm: damon-perf drops valid memory accesses at physical or virtual page 0
- [Medium] mm: damon-perf setup silently succeeds even if PMU event creation fails on all CPUs
--

commit 8f5b12d04ef4011765be3438258e3b2a5a8d8eb7
Author: Ravi Jonnalagadda <ravis.opensrc@gmail.com>

mm/damon: add perf-event overflow handler feeding the report ring

This patch adds a PMU-agnostic perf-event overflow handler for DAMON. It
routes physical and virtual address access samples from a perf-event into
a per-context report ring.

> diff --git a/mm/damon/perf_source.c b/mm/damon/perf_source.c
> new file mode 100644
> index 0000000000000..65bbfe850e708
> --- /dev/null
> +++ b/mm/damon/perf_source.c
[ ... ]
> +static void damon_perf_overflow(struct perf_event *perf_event,
> +				struct perf_sample_data *data,
> +				struct pt_regs *regs)
> +{
[ ... ]
> +	if (data->sample_flags & PERF_SAMPLE_PHYS_ADDR)
> +		report.paddr = data->phys_addr & PAGE_MASK;
> +	if (data->sample_flags & PERF_SAMPLE_ADDR)
> +		report.vaddr = data->addr & PAGE_MASK;
> +
> +	if (!report.paddr && !report.vaddr)
> +		return;

[Severity: High]
Does this check mistakenly drop legitimate accesses to physical or virtual
page 0? If only PERF_SAMPLE_PHYS_ADDR is requested, accesses to the first
physical page will have report.paddr == 0 and report.vaddr == 0, causing
this logic to silently discard valid memory accesses.

> +
> +	if (data->sample_flags & PERF_SAMPLE_DATA_SRC)
[ ... ]
> +static int damon_perf_cpu_online(unsigned int cpu, struct hlist_node *node)
> +{
[ ... ]
> +	perf_event = perf_event_create_kernel_counter(&attr, cpu, NULL,
> +						      damon_perf_overflow,
> +						      event);
> +	if (IS_ERR(perf_event)) {
> +		pr_warn_ratelimited("damon-perf: cpu %u event create failed: %ld\n",
> +				    cpu, PTR_ERR(perf_event));
> +		return 0;
> +	}
> +	per_cpu(*perf->event, cpu) = perf_event;
> +
> +	perf_event_enable(perf_event);
> +	return 0;
> +}
[ ... ]
> +int damon_perf_probe_setup(struct damon_ctx *ctx,
> +			   struct damon_probe *probe,
> +			   struct damon_perf_probe_event *event)
> +{
[ ... ]
> +	err = cpuhp_state_add_instance(damon_perf_cpuhp_state,
> +				       &event->hlist_node);
> +	if (err)
> +		goto free_event;
> +
> +	return 0;

[Severity: Medium]
Could this setup silently succeed even if PMU event creation fails on all
CPUs?

In damon_perf_cpu_online(), when perf_event_create_kernel_counter() returns
an error, it rightly returns 0 to avoid breaking CPU hotplug. However,
damon_perf_probe_setup() relies on the return value of
cpuhp_state_add_instance().

If a user provides an invalid PMU configuration, event creation will fail
everywhere, but the setup will succeed without error, leaving DAMON running
without any armed counters. Should there be a check to verify that at least
one counter was successfully created?

> +
> +free_event:
> +	free_percpu(perf->event);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005-damon-perf-rfc-v3-send-2026-10-03-v4-0-b03452e137f3@gmail.com?part=3