linux-kernel - Re: [PATCH v10 07/11] perf: Support deferred user callchains for per CPU events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250614224107.5dfc9f9b@batman.local.home>
Date: Sat, 14 Jun 2025 22:41:07 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
 bpf@...r.kernel.org, x86@...nel.org
Cc: Masami Hiramatsu <mhiramat@...nel.org>, Mathieu Desnoyers
 <mathieu.desnoyers@...icios.com>, Josh Poimboeuf <jpoimboe@...nel.org>,
 Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>, Jiri
 Olsa <jolsa@...nel.org>, Namhyung Kim <namhyung@...nel.org>, Thomas
 Gleixner <tglx@...utronix.de>, Andrii Nakryiko <andrii@...nel.org>, Indu
 Bhagat <indu.bhagat@...cle.com>, "Jose E. Marchesi" <jemarch@....org>, Beau
 Belgrave <beaub@...ux.microsoft.com>, Jens Remus <jremus@...ux.ibm.com>,
 Linus Torvalds <torvalds@...ux-foundation.org>, Andrew Morton
 <akpm@...ux-foundation.org>
Subject: Re: [PATCH v10 07/11] perf: Support deferred user callchains for
 per CPU events

On Fri, 13 Jun 2025 22:46:12 -0400
Steven Rostedt <rostedt@...dmis.org> wrote:

> +/*
> + * Deferred unwinding callback for per CPU events.
> + * Note, the request for the deferred unwinding may have happened
> + * on a different CPU.
> + */
> +static void perf_event_deferred_cpu(struct unwind_work *work,
> +				    struct unwind_stacktrace *trace, u64 timestamp)
> +{
> +	struct perf_unwind_deferred *defer =
> +		container_of(work, struct perf_unwind_deferred, unwind_work);
> +	struct perf_unwind_cpu *cpu_events, *cpu_unwind;
> +	struct perf_event *event;
> +	int cpu;
> +
> +	guard(rcu)();
> +	guard(preempt)();
> +
> +	cpu = smp_processor_id();
> +	cpu_events = rcu_dereference(defer->cpu_events);
> +	cpu_unwind = &cpu_events[cpu];
> +
> +	WRITE_ONCE(cpu_unwind->processing, 1);
> +	/*
> +	 * Make sure the above is seen before the event->unwind_deferred
> +	 * is checked. This matches the mb() in rcuwait_rcu_wait_event() in
> +	 * perf_remove_unwind_deferred().
> +	 */
> +	smp_mb();
> +
> +	list_for_each_entry_rcu(event, &cpu_unwind->list, unwind_list) {
> +		/* If unwind_deferred is NULL the event is going away */
> +		if (unlikely(!event->unwind_deferred))
> +			continue;
> +		perf_event_callchain_deferred(event, trace, timestamp);
> +		/* Only the first CPU event gets the trace */
> +		break;
> +	}
> +

Hmm, I think I need a smp_mb() here too.

> +	WRITE_ONCE(cpu_unwind->processing, 0);
> +	rcuwait_wake_up(&cpu_unwind->pending_unwind_wait);
> +}

The first smp_mb() is for synchronizing removing of the event from
perf_remove_unwind_deferred() that has:

	event->unwind_deferred = NULL;

	/*
	 * Make sure perf_event_deferred_cpu() is done with this event.
	 * That function will set cpu_unwind->processing and then
	 * call smp_mb() before iterating the list of its events.
	 * If the event's unwind_deferred is NULL, it will be skipped.
	 * The smp_mb() in that function matches the mb() in
	 * rcuwait_wait_event().
	 */
	rcuwait_wait_event(&cpu_unwind->pending_unwind_wait,
				   !cpu_unwind->processing, TASK_UNINTERRUPTIBLE);

So that the unwind_deferred setting to NULL is seen before the
cpu_unwind->processing is checked. But I think, in theory, without the
smp_mb() before the clearing of the cpu_unwind->procssing that it can
be seen before the unwind_deferred is read.

  CPU 0                                    CPU 1
  -----                                    -----
read event->unwind_deferred

                                        write NULL > event->unwind_deferre
                                        smp_mb() (in rcuwait)

CPU writes 0 > cpu_unwind->processing (re-ordered)

                                        reads cpu_unwind->processing == 0
                                        Starts to free event

Executes perf_event_callchain_deferred()

I'll add another smp_mb() to be safe in v11.

-- Steve