LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: rostedt <rostedt@goodmis.org>
Cc: Joel Fernandes <joelaf@google.com>,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-rt-users <linux-rt-users@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Tom Zanussi <tom.zanussi@linux.intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Boqun Feng <boqun.feng@gmail.com>, fweisbec <fweisbec@gmail.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	kbuild test robot <fengguang.wu@intel.com>,
	baohong liu <baohong.liu@intel.com>,
	vedang patel <vedang.patel@intel.com>,
	kernel-team <kernel-team@lge.com>
Subject: Re: [RFC v4 3/4] irqflags: Avoid unnecessary calls to trace_ if you can
Date: Thu, 26 Apr 2018 12:08:00 -0400 (EDT)	[thread overview]
Message-ID: <749188693.2028.1524758880738.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <1609895968.1947.1524754982656.JavaMail.zimbra@efficios.com>

----- On Apr 26, 2018, at 11:03 AM, Mathieu Desnoyers mathieu.desnoyers@efficios.com wrote:

> ----- On Apr 25, 2018, at 6:51 PM, rostedt rostedt@goodmis.org wrote:
> 
>> On Wed, 25 Apr 2018 17:40:56 -0400 (EDT)
>> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
>> 
>>> One problem with your approach is that you can have multiple callers
>>> for the same tracepoint name, where some could be non-preemptible and
>>> others blocking. Also, there is then no clear way for the callback
>>> registration API to enforce whether the callback expects the tracepoint
>>> to be blocking or non-preemptible. This can introduce hard to diagnose
>>> issues in a kernel without debug options enabled.
>> 
>> I agree that it should not be tied to an implementation name. But
>> "blocking" is confusing. I would say "can_sleep" or some such name that
>> states that the trace point caller is indeed something that can sleep.
> 
> "trace_*event*_{can,might,may}_sleep" are all acceptable candidates for
> me.
> 
>> 
>>> 
>>> Regarding the name, I'm OK with having something along the lines of
>>> trace_*event*_blocking or such. Please don't use "srcu" or other naming
>>> that is explicitly tied to the underlying mechanism used internally
>>> however: what we want to convey is that this specific tracepoint probe
>>> can be preempted and block. The underlying implementation could move to
>>> a different RCU flavor brand in the future, and it should not impact
>>> users of the tracepoint APIs.
>>> 
>>> In order to ensure that probes that may block only register themselves
>>> to tracepoints that allow blocking, we should introduce new tracepoint
>>> declaration/definition *and* registration APIs also contain the
>>> "BLOCKING/blocking" keywords (or such), so we can ensure that a
>>> tracepoint probe being registered to a "blocking" tracepoint is indeed
>>> allowed to block.
>> 
>> I'd really don't want to add more declaration/definitions, as we
>> already have too many as is, and with different meanings and the number
>> is of incarnations is n! in growth.
>> 
>> I'd say we just stick with a trace_<event>_can_sleep() call, and make
>> sure that if that is used that no trace_<event>() call is also used, and
>> enforce this with linker or compiler tricks.
> 
> My main concern is not about having both trace_<event>_can_sleep() mixed
> with trace_<event>() calls. It's more about having a registration API allowing
> modules registering probes that may need to sleep to explicitly declare it,
> and enforce that tracepoint never connects a probe that may need to sleep
> with an instrumentation site which cannot sleep.
> 
> I'm unsure what's the best way to achieve this goal though. We could possibly
> extend the tracepoint_probe_register_* APIs to introduce e.g.
> tracepoint_probe_register_prio_flags() and provide a TRACEPOINT_PROBE_CAN_SLEEP
> as parameter upon registration. If this flag is provided, then we could figure
> out
> an way to iterate on all callers, and ensure they are all "can_sleep" type of
> callers.

Iteration on all callers would require that we add some separate section data
for each caller, which we don't have currently. At the moment, the only data
we need is at the tracepoint definition. If we have tons of callers for a given
tracepoint (which might be the case for lockdep), we'd end up consuming a lot of
useless space.

This is one reason why I would prefer to have separate tracepoint definitions
for each of rcuidle, can_sleep, and nonpreemptible (nmi-safe) tracepoints.

Thanks,

Mathieu


> 
> Thoughts ?
> 
> Thanks,
> 
> Mathieu
> 
> 
> 
>> 
>> -- Steve
> 
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

  reply	other threads:[~2018-04-26 16:08 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-17  4:07 [RFC v4 0/4] Centralize and unify usage of preempt/irq tracepoints Joel Fernandes
2018-04-17  4:07 ` [RFC v4 1/4] tracepoint: Add API to not do lockdep checks during RCU ops Joel Fernandes
2018-04-17  4:07 ` [RFC v4 2/4] softirq: reorder trace_softirqs_on to prevent lockdep splat Joel Fernandes
2018-04-17  4:07 ` [RFC v4 3/4] irqflags: Avoid unnecessary calls to trace_ if you can Joel Fernandes
2018-04-18  9:02   ` Masami Hiramatsu
2018-04-19  5:43     ` Namhyung Kim
2018-04-20  7:07       ` Joel Fernandes
2018-04-23  1:14         ` Joel Fernandes
2018-04-23  3:19           ` Paul E. McKenney
2018-04-23 14:31             ` Mathieu Desnoyers
2018-04-23 14:53               ` Steven Rostedt
2018-04-23 14:59                 ` Mathieu Desnoyers
2018-04-23 15:12                   ` Paul E. McKenney
2018-04-23 16:18                   ` Steven Rostedt
2018-04-23 17:12                     ` Mathieu Desnoyers
2018-04-23 17:24                       ` Joel Fernandes
2018-04-23 21:22                       ` Steven Rostedt
2018-04-24 15:56                         ` Paul E. McKenney
2018-04-24 16:01                           ` Joel Fernandes
2018-04-24 17:26                             ` Paul E. McKenney
2018-04-24 18:23                               ` Paul E. McKenney
2018-04-24 18:26                                 ` Paul E. McKenney
2018-04-24 18:59                                   ` Joel Fernandes
2018-04-24 19:01                                     ` Joel Fernandes
2018-04-24 19:09                                     ` Paul E. McKenney
2018-04-24 19:16                                       ` Joel Fernandes
2018-04-24 23:21                                     ` Mathieu Desnoyers
2018-04-24 23:46                                       ` Joel Fernandes
2018-04-25  0:10                                         ` Paul E. McKenney
2018-04-25  4:20                                           ` Paul E. McKenney
2018-04-25 21:27                                             ` Joel Fernandes
2018-04-25 21:35                                               ` Paul E. McKenney
2018-04-25 21:40                                               ` Mathieu Desnoyers
2018-04-25 22:51                                                 ` Steven Rostedt
2018-04-26 15:03                                                   ` Mathieu Desnoyers
2018-04-26 16:08                                                     ` Mathieu Desnoyers [this message]
2018-04-25 23:13                                                 ` Joel Fernandes
2018-04-26 15:13                                                   ` Mathieu Desnoyers
2018-04-26 15:20                                                     ` Joel Fernandes
2018-04-26 15:49                                                     ` Paul E. McKenney
2018-04-23 15:49                 ` Joel Fernandes
2018-04-26  2:18             ` Joel Fernandes
2018-05-01  1:18     ` Joel Fernandes
2018-04-17  4:07 ` [RFC v4 4/4] tracing: Centralize preemptirq tracepoints and unify their usage Joel Fernandes

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=749188693.2028.1524758880738.JavaMail.zimbra@efficios.com \
    --to=mathieu.desnoyers@efficios.com \
    --cc=baohong.liu@intel.com \
    --cc=boqun.feng@gmail.com \
    --cc=fengguang.wu@intel.com \
    --cc=fweisbec@gmail.com \
    --cc=joelaf@google.com \
    --cc=kernel-team@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-users@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=rdunlap@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=tom.zanussi@linux.intel.com \
    --cc=vedang.patel@intel.com \
    --subject='Re: [RFC v4 3/4] irqflags: Avoid unnecessary calls to trace_ if you can' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).