linux-kernel - Re: [PATCH v5 13/17] perf: Support deferred user callchains

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aA_oJ7tgGv-H4ocX@google.com>
Date: Mon, 28 Apr 2025 13:42:15 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
	Masami Hiramatsu <mhiramat@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Josh Poimboeuf <jpoimboe@...nel.org>, x86@...nel.org,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Indu Bhagat <indu.bhagat@...cle.com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	linux-perf-users@...r.kernel.org, Mark Brown <broonie@...nel.org>,
	linux-toolchains@...r.kernel.org, Jordan Rome <jordalgo@...a.com>,
	Sam James <sam@...too.org>,
	Andrii Nakryiko <andrii.nakryiko@...il.com>,
	Jens Remus <jremus@...ux.ibm.com>,
	Florian Weimer <fweimer@...hat.com>,
	Andy Lutomirski <luto@...nel.org>, Weinan Liu <wnliu@...gle.com>,
	Blake Jones <blakejones@...gle.com>,
	Beau Belgrave <beaub@...ux.microsoft.com>,
	"Jose E. Marchesi" <jemarch@....org>
Subject: Re: [PATCH v5 13/17] perf: Support deferred user callchains

On Fri, Apr 25, 2025 at 12:58:15PM -0400, Steven Rostedt wrote:
> On Fri, 25 Apr 2025 08:24:47 -0700
> Namhyung Kim <namhyung@...nel.org> wrote:
> > > +/*
> > > + * 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..
> 
> I was afraid of that.
> 
> This is no different that what Josh did in his last set in v4. I'm guessing
> the issue is running with "-a", correct?

Yes.

> 
> Could we just not use deferred when running with "-a" for now? Or could we
> possibly just make the deferred stacktrace its own event? Have it be
> possible that perf just registers a signal instance with the deferred
> unwinding logic, and then perf can handle where to write the information. I
> don't know perf well enough to implement that.

Even if it excludes per-CPU events, per-task events also can attach to a
CPU and that's the default behavior of the perf record IIRC.  In that
case, it needs to be careful when it accesses the event since the task
can migrate to another CPU.  So I'm not sure if it's a good idea to
track event that requested the deferred callchains.

Also it doesn't need to emit duplicate deferred callchains if a task
has multiple events and they are requesting callchains.  Unfortunately,
the kernel cannot know which events are related or profiled together.

Hmm.. maybe we can add a cookie to the event itself (by ioctl or
something) in order to group events in a profiling session and then use
that for deferred callchains?  Task should maintain a list of active
cookies (or sessions) somehow but then perf can check if the current CPU
has events with matching cookies and emit a deferred callchain.

> 
> Josh's code had it call the unwind_deferred_init() and just used its own
> event to callback to and that was called on hundreds of events when I ran:
> 
>   perf record -g <whatever>
> 
> Same if I added the "-a" option.
> 
> The above function return values came from Josh's code:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git/tree/kernel/unwind/deferred.c?h=sframe#n173
> 
> I just moved it out of deferred.c and into perf itself, and removed the
> cookie logic.
> 
> > 
> > > +
> > > +	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?
> 
> task_pt_regs(current) will return the regs from when the task entered the
> kernel. So the answer is no, it will still trace if an interrupt happened
> while a task is in a system call.

Ok, thanks for the explanation.  The the user_mode check was for kernel
threads, right?

Thanks,
Namhyung