Re: [PATCH v5 13/17] perf: Support deferred user callchains
From: Namhyung Kim
Date: Fri Apr 25 2025 - 11:24:58 EST
Hello,
On Thu, Apr 24, 2025 at 12:25:42PM -0400, Steven Rostedt wrote:
> From: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
>
> Use the new unwind_deferred_trace() interface (if available) to defer
> unwinds to task context. This will allow the use of .sframe (when it
> becomes available) and also prevents duplicate userspace unwinds.
>
> Suggested-by: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Co-developed-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
> Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
> ---
[SNIP]
> +/*
> + * Returns:
> +* > 0 : if already queued.
> + * 0 : if it performed the queuing
> + * < 0 : if it did not get queued.
> + */
> +static int deferred_request(struct perf_event *event)
> +{
> + struct callback_head *work = &event->pending_unwind_work;
> + int pending;
> + int ret;
I'm not sure if it works for per-CPU events. The event is shared so any
task can request the deferred callchains. Does it handle if task A
requests one and scheduled out before going to the user mode, and task B
on the CPU also requests another after that? I'm afraid not..
> +
> + if (!current->mm || !user_mode(task_pt_regs(current)))
> + return -EINVAL;
Does it mean it cannot use deferred callstack when it's in the kernel
mode like during a syscall?
Thanks,
Namhyung
> +
> + if (in_nmi())
> + return deferred_request_nmi(event);
> +
> + guard(irqsave)();
> +
> + /* callback already pending? */
> + pending = READ_ONCE(event->pending_unwind_callback);
> + if (pending)
> + return 1;
> +
> + /* Claim the work unless an NMI just now swooped in to do so. */
> + if (!try_cmpxchg(&event->pending_unwind_callback, &pending, 1))
> + return 1;
> +
> + /* The work has been claimed, now schedule it. */
> + ret = task_work_add(current, work, TWA_RESUME);
> + if (WARN_ON_ONCE(ret)) {
> + WRITE_ONCE(event->pending_unwind_callback, 0);
> + return ret;
> + }
> + return 0;
> +}