Re: [PATCH] perf/core: Prevent duplicate sigtrap task work enqueue

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] perf/core: redundant store to event->pending_work in __perf_event_overflow()
--

commit badcfdb790f5048e93691fc4bd20053f8cc9e2d2
Author: Tengda Wu <wutengda@huaweicloud.com>

perf/core: Prevent duplicate sigtrap task work enqueue

This commit uses cmpxchg() to atomically claim event->pending_work before
calling task_work_add() in __perf_event_overflow() so that a nested NMI
cannot enqueue the same sigtrap pending_task twice.

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 457d75ea71420..f1e4e153c49a4 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -10925,16 +10925,18 @@ static int __perf_event_overflow(struct perf_event *event,
>  
>  		notify_mode = in_nmi() ? TWA_NMI_CURRENT : TWA_RESUME;
>  
> -		if (!event->pending_work &&
> -		    !task_work_add(current, &event->pending_task, notify_mode)) {
> -			event->pending_work = pending_id;
> -			local_inc(&event->ctx->nr_no_switch_fast);
> -			WARN_ON_ONCE(!atomic_long_inc_not_zero(&event->refcount));
> -
> -			event->pending_addr = 0;
> -			if (valid_sample && (data->sample_flags & PERF_SAMPLE_ADDR))
> -				event->pending_addr = data->addr;
> -
> +		if (cmpxchg(&event->pending_work, 0, pending_id) == 0) {
> +			if (!task_work_add(current, &event->pending_task, notify_mode)) {
> +				event->pending_work = pending_id;

[Severity: Low]
Is this store to event->pending_work in __perf_event_overflow() in
kernel/events/core.c still needed when task_work_add() succeeds?

The outer cmpxchg(&event->pending_work, 0, pending_id) == 0 check already
sets event->pending_work to pending_id before calling task_work_add(), and
nothing can clear or modify event->pending_work while inside this branch
(interrupts are disabled, perf_pending_task() cannot run until return to
user space or task exit, and any nested NMI sees a non-zero value in
cmpxchg()).

Leaving this assignment also performs an unmarked plain write to
event->pending_work, whereas the rest of this change accesses it via
cmpxchg() and READ_ONCE(). Can this redundant assignment be dropped?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261031114520.642476-1-wutengda@huaweicloud.com?part=1