From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Michael Jeanson <mjeanson@efficios.com>,
linux-kernel <linux-kernel@vger.kernel.org>,
rostedt <rostedt@goodmis.org>,
Peter Zijlstra <peterz@infradead.org>,
Alexei Starovoitov <ast@kernel.org>, Yonghong Song <yhs@fb.com>,
paulmck <paulmck@kernel.org>, Ingo Molnar <mingo@redhat.com>,
acme <acme@kernel.org>, Mark Rutland <mark.rutland@arm.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
"Joel Fernandes, Google" <joel@joelfernandes.org>,
bpf <bpf@vger.kernel.org>
Subject: Re: [RFC PATCH 1/6] tracing: introduce sleepable tracepoints
Date: Tue, 27 Oct 2020 09:37:08 -0400 (EDT) [thread overview]
Message-ID: <1631556114.38532.1603805828748.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <20201026224301.gi4bakmj3pov5zyu@ast-mbp.dhcp.thefacebook.com>
----- On Oct 26, 2020, at 6:43 PM, Alexei Starovoitov alexei.starovoitov@gmail.com wrote:
> On Fri, Oct 23, 2020 at 03:53:47PM -0400, Michael Jeanson wrote:
>> -#define __DO_TRACE(tp, proto, args, cond, rcuidle) \
>> +#define __DO_TRACE(tp, proto, args, cond, rcuidle, tp_flags) \
>> do { \
>> struct tracepoint_func *it_func_ptr; \
>> void *it_func; \
>> void *__data; \
>> int __maybe_unused __idx = 0; \
>> + bool maysleep = (tp_flags) & TRACEPOINT_MAYSLEEP; \
>> \
>> if (!(cond)) \
>> return; \
>> @@ -170,8 +178,13 @@ static inline struct tracepoint
>> *tracepoint_ptr_deref(tracepoint_ptr_t *p)
>> /* srcu can't be used from NMI */ \
>> WARN_ON_ONCE(rcuidle && in_nmi()); \
>> \
>> - /* keep srcu and sched-rcu usage consistent */ \
>> - preempt_disable_notrace(); \
>> + if (maysleep) { \
>> + might_sleep(); \
>
> The main purpose of the patch set is to access user memory in tracepoints,
> right?
Yes, exactly.
> In such case I suggest to use stronger might_fault() here.
> We used might_sleep() in sleepable bpf and it wasn't enough to catch
> a combination where sleepable hook was invoked while mm->mmap_lock was
> taken which may cause a deadlock.
Good point! We will do that for the next round.
By the way, we named this "sleepable" tracepoint (with flag TRACEPOINT_MAYSLEEP),
but we are open to a better name. Would TRACEPOINT_MAYFAULT be more descriptive ?
(a "faultable" tracepoint sounds weird though)
Thanks,
Mathieu
>
>> + rcu_read_lock_trace(); \
>> + } else { \
>> + /* keep srcu and sched-rcu usage consistent */ \
>> + preempt_disable_notrace(); \
> > + } \
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
next prev parent reply other threads:[~2020-10-27 13:37 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-23 19:53 [RFC PATCH 0/6] Sleepable tracepoints Michael Jeanson
2020-10-23 19:53 ` [RFC PATCH 1/6] tracing: introduce sleepable tracepoints Michael Jeanson
2020-10-26 22:43 ` Alexei Starovoitov
2020-10-27 13:37 ` Mathieu Desnoyers [this message]
2020-10-28 21:23 ` Alexei Starovoitov
2021-02-11 19:36 ` Mathieu Desnoyers
2020-11-02 18:51 ` Joel Fernandes
2020-10-23 19:53 ` [RFC PATCH 2/6] tracing: ftrace: add support for " Michael Jeanson
2020-10-23 19:53 ` [RFC PATCH 3/6] tracing: bpf-trace: " Michael Jeanson
2020-10-23 19:53 ` [RFC PATCH 4/6] tracing: perf: " Michael Jeanson
2020-10-23 19:53 ` [RFC PATCH 5/6] tracing: convert sys_enter/exit to " Michael Jeanson
2020-10-23 19:53 ` [RFC PATCH 6/6] tracing: use sched-RCU instead of SRCU for rcuidle tracepoints Michael Jeanson
2020-10-23 21:13 ` Joel Fernandes
2020-10-26 8:20 ` Peter Zijlstra
2020-10-26 14:28 ` Mathieu Desnoyers
2020-10-26 20:44 ` Steven Rostedt
2020-10-27 13:57 ` Mathieu Desnoyers
2020-11-02 18:43 ` Joel Fernandes
2020-10-26 12:05 ` [RFC PATCH 0/6] Sleepable tracepoints peter enderborg
2020-10-26 14:59 ` Mathieu Desnoyers
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=1631556114.38532.1603805828748.JavaMail.zimbra@efficios.com \
--to=mathieu.desnoyers@efficios.com \
--cc=acme@kernel.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=alexei.starovoitov@gmail.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=joel@joelfernandes.org \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=mjeanson@efficios.com \
--cc=namhyung@kernel.org \
--cc=paulmck@kernel.org \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=yhs@fb.com \
/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;
as well as URLs for NNTP newsgroup(s).