| 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
| ||
|
Message-ID: <20180727122825.281a3a67@gandalf.local.home>
Date: Fri, 27 Jul 2018 12:28:25 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Joel Fernandes <joel@...lfernandes.org>
Cc: linux-kernel@...r.kernel.org, kernel-team@...roid.com,
Boqun Feng <boqun.feng@...il.com>,
Byungchul Park <byungchul.park@....com>,
Erick Reyes <erickreyes@...gle.com>,
Ingo Molnar <mingo@...hat.com>,
Julia Cartwright <julia@...com>,
Masami Hiramatsu <mhiramat@...nel.org>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
Namhyung Kim <namhyung@...nel.org>,
Paul McKenney <paulmck@...ux.vnet.ibm.com>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Glexiner <tglx@...utronix.de>,
Todd Kjos <tkjos@...gle.com>,
Tom Zanussi <tom.zanussi@...ux.intel.com>,
Will Deacon <will.deacon@....com>
Subject: Re: [PATCH v11 3/3] tracing: Centralize preemptirq tracepoints and
unify their usage
On Thu, 26 Jul 2018 16:50:44 -0700
Joel Fernandes <joel@...lfernandes.org> wrote:
> From: "Joel Fernandes (Google)" <joel@...lfernandes.org>
>
> This patch detaches the preemptirq tracepoints from the tracers and
> keeps it separate.
>
> Advantages:
> * Lockdep and irqsoff event can now run in parallel since they no longer
> have their own calls.
>
> * This unifies the usecase of adding hooks to an irqsoff and irqson
> event, and a preemptoff and preempton event.
> 3 users of the events exist:
> - Lockdep
> - irqsoff and preemptoff tracers
> - irqs and preempt trace events
>
> The unification cleans up several ifdefs and makes the code in preempt
> tracer and irqsoff tracers simpler. It gets rid of all the horrific
> ifdeferry around PROVE_LOCKING and makes configuration of the different
> users of the tracepoints more easy and understandable. It also gets rid
> of the time_* function calls from the lockdep hooks used to call into
> the preemptirq tracer which is not needed anymore. The negative delta in
> lines of code in this patch is quite large too.
>
> In the patch we introduce a new CONFIG option PREEMPTIRQ_TRACEPOINTS
> as a single point for registering probes onto the tracepoints. With
> this,
> the web of config options for preempt/irq toggle tracepoints and its
> users becomes:
>
> PREEMPT_TRACER PREEMPTIRQ_EVENTS IRQSOFF_TRACER PROVE_LOCKING
> | | \ | |
> \ (selects) / \ \ (selects) /
> TRACE_PREEMPT_TOGGLE ----> TRACE_IRQFLAGS
> \ /
> \ (depends on) /
> PREEMPTIRQ_TRACEPOINTS
>
> One note, I have to check for lockdep recursion in the code that calls
> the trace events API and bail out if we're in lockdep recursion
> protection to prevent something like the following case: a spin_lock is
> taken. Then lockdep_acquired is called. That does a raw_local_irq_save
> and then sets lockdep_recursion, and then calls __lockdep_acquired. In
> this function, a call to get_lock_stats happens which calls
> preempt_disable, which calls trace IRQS off somewhere which enters my
> tracepoint code and sets the tracing_irq_cpu flag to prevent recursion.
> This flag is then never cleared causing lockdep paths to never be
> entered and thus causing splats and other bad things.
>
> Other than the performance tests mentioned in the previous patch, I also
> ran the locking API test suite. I verified that all tests cases are
> passing.
>
> I also injected issues by not registering lockdep probes onto the
> tracepoints and I see failures to confirm that the probes are indeed
> working.
>
> This series + lockdep probes not registered (just to inject errors):
> [ 0.000000] hard-irqs-on + irq-safe-A/21: ok | ok | ok |
> [ 0.000000] soft-irqs-on + irq-safe-A/21: ok | ok | ok |
> [ 0.000000] sirq-safe-A => hirqs-on/12:FAILED|FAILED| ok |
> [ 0.000000] sirq-safe-A => hirqs-on/21:FAILED|FAILED| ok |
> [ 0.000000] hard-safe-A + irqs-on/12:FAILED|FAILED| ok |
> [ 0.000000] soft-safe-A + irqs-on/12:FAILED|FAILED| ok |
> [ 0.000000] hard-safe-A + irqs-on/21:FAILED|FAILED| ok |
> [ 0.000000] soft-safe-A + irqs-on/21:FAILED|FAILED| ok |
> [ 0.000000] hard-safe-A + unsafe-B #1/123: ok | ok | ok |
> [ 0.000000] soft-safe-A + unsafe-B #1/123: ok | ok | ok |
>
> With this series + lockdep probes registered, all locking tests pass:
>
> [ 0.000000] hard-irqs-on + irq-safe-A/21: ok | ok | ok |
> [ 0.000000] soft-irqs-on + irq-safe-A/21: ok | ok | ok |
> [ 0.000000] sirq-safe-A => hirqs-on/12: ok | ok | ok |
> [ 0.000000] sirq-safe-A => hirqs-on/21: ok | ok | ok |
> [ 0.000000] hard-safe-A + irqs-on/12: ok | ok | ok |
> [ 0.000000] soft-safe-A + irqs-on/12: ok | ok | ok |
> [ 0.000000] hard-safe-A + irqs-on/21: ok | ok | ok |
> [ 0.000000] soft-safe-A + irqs-on/21: ok | ok | ok |
> [ 0.000000] hard-safe-A + unsafe-B #1/123: ok | ok | ok |
> [ 0.000000] soft-safe-A + unsafe-B #1/123: ok | ok | ok |
>
> Reviewed-by: Namhyung Kim <namhyung@...nel.org>
> Signed-off-by: Joel Fernandes (Google) <joel@...lfernandes.org>
> ---
> include/linux/ftrace.h | 11 +-
> include/linux/irqflags.h | 11 +-
> include/linux/lockdep.h | 8 +-
> include/linux/preempt.h | 2 +-
> include/trace/events/preemptirq.h | 23 +--
> init/main.c | 5 +-
> kernel/locking/lockdep.c | 35 ++---
Did Peter ever give an Acked-by for this patch?
-- Steve
> kernel/sched/core.c | 2 +-
> kernel/trace/Kconfig | 22 ++-
> kernel/trace/Makefile | 2 +-
> kernel/trace/trace_irqsoff.c | 231 ++++++++----------------------
> kernel/trace/trace_preemptirq.c | 72 ++++++++++
> 12 files changed, 195 insertions(+), 229 deletions(-)
> create mode 100644 kernel/trace/trace_preemptirq.c
>
>
Powered by blists - more mailing lists