mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Aleksa Sarai <cyphar@cyphar.com>
To: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Robert Richter <rric@kernel.org>,
	Brendan Gregg <bgregg@netflix.com>
Cc: linux-kernel@vger.kernel.org, oprofile-list@lists.sf.net,
	cyphar@cyphar.com
Subject: [RFC] Merging ftrace_stack, perf_callchain, oprofile->backtrace and stack_trace
Date: Sun, 21 Oct 2018 20:31:06 +1100	[thread overview]
Message-ID: <20181021093106.mhjpb2rnx6kstjki@ryuk> (raw)

[-- Attachment #1: Type: text/plain, Size: 2320 bytes --]

Hi all,

I'm currently working on a patchset to make kretprobes produce
reasonable stack traces[1], and it appears this is a generic problem
across the entire kernel -- you can see the same kretprobe_trampoline()
issue when using ftrace just as much as bpf_trace.

However, in working on this patch, I've noticed that there appear to be
several different implementations of "get the stack trace from this
pt_regs" which all appear quite similar. Namely:

  * struct ftrace_stack;
  * struct perf_callchain_entry; [**]
  * struct stack_trace;

  * oprofile_operations->backtrace [This is not related to the kretprobe
	problem, very tangential, but since it's usage is not very
	complicated -- logging to dmesg -- it wouldn't be too bad to
	refactor this one too].

Would there be a strong objection to me trying to merge together these
so that they all use 'struct stack_trace', and no longer have
arch-specific code that is doing (what appears to be) the same unwind
code? Or is this something that was intentionally avoided because there
are some differences that I'm not seeing?

The reason I ask is because the kretprobes patch would require saving
the stacktrace during pre_handler_kretprobe() -- and so in order for all
of the tracing subsystems to take advantage of it they'd need to be able
to use that saved stack trace.

The only other option I can see would be to implement some sort of
translation from 'struct stack_trace' to the others. This wouldn't be
too bad, but I imagine it would be uglier than refactoring them all to
use the same struct.

[**] perf_callchain_entry has the concept of "marking" a context in the
     stack trace. But I wonder whether this is something that we could
	 do with 'struct stack_trace' -- after all it's just magic ->ip
	 values. *But* then the question is what is the purpose of
	 sysctl_perf_event_max_contexts_per_stack? It limits the number of
	 contexts, but isn't that already implicitly limited by the number
	 of stack entries? We could also implement this with 'struct
	 stack_trace' but it would require wrapping 'struct stack_trace' to
	 make it efficient.

[1]: https://github.com/iovisor/bpftrace/issues/101

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

             reply	other threads:[~2018-10-21  9:31 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-21  9:31 Aleksa Sarai [this message]
2018-10-21  9:41 ` Aleksa Sarai
2018-10-21 20:13 ` Peter Zijlstra
2018-10-22 15:26   ` Aleksa Sarai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181021093106.mhjpb2rnx6kstjki@ryuk \
    --to=cyphar@cyphar.com \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=bgregg@netflix.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=oprofile-list@lists.sf.net \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=rric@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®