linux-kernel - Re: [PATCH v2 00/11] unwind, perf: sframe user space unwinding, deferred perf 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: <20240916153953.7fq5fmch5uqg7tjj@treble>
Date: Mon, 16 Sep 2024 17:39:53 +0200
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Steven Rostedt <rostedt@...dmis.org>, x86@...nel.org,
	Ingo Molnar <mingo@...nel.org>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	linux-kernel@...r.kernel.org, Indu Bhagat <indu.bhagat@...cle.com>,
	Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>, Namhyung Kim <namhyung@...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>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Subject: Re: [PATCH v2 00/11] unwind, perf: sframe user space unwinding,
 deferred perf callchains

On Mon, Sep 16, 2024 at 04:08:56PM +0200, Peter Zijlstra wrote:
> On Sun, Sep 15, 2024 at 01:11:11PM +0200, Josh Poimboeuf wrote:
> > On Sat, Sep 14, 2024 at 08:12:46AM -0400, Steven Rostedt wrote:
> > > I think the unwinder should have an interface itself that provides the
> > > deferred unwinding, instead of having all the tracers to implement
> > > their own.
> > > 
> > > The user space unwinder (regardless of sframe or not) should provide a
> > > way to say "I want a user space stack trace for here, but don't do it
> > > yet. Just give me a cookie and then call my callback function with the
> > > stack and cookie before going back to user space".
> > 
> > We (Steven, Mathieu and I) have been discussing this at GNU Cauldron and
> > I think we're in basic agreement on this.
> > 
> > I think the biggest tweak we decided on is that the context id (aka
> > "cookie") would be percpu.  Its initial value is (cpuid << 48).  It gets
> > incremented for every entry from user space.
> 
> Why? What's the purpose of the cookie?

The cookie is incremented once per entry from userspace, when needed.

It's a unique id used by the tracers which is emitted with both kernel
and user stacktrace samples so the userspace tool can stitch them back
together.  Even if you have multiple tracers triggering at the same time
they can all share a single user trace.

Completely untested patch here that I hacked up today:

 git://git.kernel.org/pub/scm/linux/kernel/git/jpoimboe/linux.git sframe-2.1

To avoid races, unwind_user_deferred() can't be called from NMI.  The
tracers will need to trigger irq_work to call it.

> This scheme seems unsound

It may not be communicated well in this thread.  We had much discussion
at Cauldron the last few days.  The patch may be the best way to
communicate it at this point, though I'm sure I missed some details.

> pin yourself on CPU0 and trigger 1<<48 unwinds while keeping CPU1 idle.

Hm???

> > > That is, we should have an interface like:
> > > 
> > > typedef void (unwinder_callback_t)(struct user_space_stack *, u64 cookie);
> 
> Just make it a void* and let the consumer figure it out.

I think it makes sense to manage the cookie in the deferred unwinder
since the need to correlate the stack traces is an inherent part of the
deferral.

-- 
Josh