From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org
Subject: Re: There is a Tasks RCU stall warning
Date: Wed, 12 Apr 2017 09:26:10 -0700 [thread overview]
Message-ID: <20170412162610.GI3956@linux.vnet.ibm.com> (raw)
In-Reply-To: <20170412115304.3077dbc8@gandalf.local.home>
On Wed, Apr 12, 2017 at 11:53:04AM -0400, Steven Rostedt wrote:
> On Wed, 12 Apr 2017 08:18:17 -0700
> "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
>
>
> > > Well the trampolines pretty much can, but they are removed before
> > > calling synchronize_rcu_tasks(), and nothing can enter the trampoline
> > > when that is called.
> >
> > Color me confused...
> >
> > So you can have an arbitrary function call within a trampoline?
>
> Sorta.
>
> When you do register_ftrace_function(ops), where ops has ops->func that
> points to a function you want to have called when a function is traced,
> the following happens (if there's no other ops registered). Let's use
> an example where ops is filtered on just the schedule() function call:
>
>
> <schedule>:
> call trampoline ---+
> [..] |
> +--> <trampoline>:
> push regs
> call ops->func
> pop regs
> ret
>
> But that ops->func() must be very limited in what it can do. Although,
> it may actually call an rcu_read_lock()! But if that's the case, it
> must either check if rcu is watching (which perf does), or enable rcu
> via the rcu_irq_enter() with a check on rcu_irq_enter_disabled(), which
> my stack tracer does.
>
> Now this can be called even from NMI context! Thus what ops->func does
> must be aware of that. The stack tracer func has an:
>
> if (in_nmi())
> return;
>
> Because it needs to grab spin locks.
But preemption is enabled within the trampoline? If so, then if
CONFIG_RCU_BOOST is set, rcu_read_unlock() can call rt_mutex_unlock().
Which looks OK to me, but I thought I should mention it.
> But one thing an op->func() is never allowed to do, is to call
> schedule() directly, or even a cond_resched(). It may be preempted if
> preemption was enabled when the trampoline was hit, but it must not
> assume that it can do a voluntary schedule. That would break the
> rcu_tasks as well if it did.
OK, so it can call functions, but it is not permitted to call functions
that voluntarily block. That should work. (Fingers firmly crossed.)
> > If not, agreed, no problem. Otherwise, it seems like we have a big
> > problem remaining. Unless the functions called from a trampoline are
> > guaranteed never to do a context switch.
>
> Well, they can be preempted, but they should never do a voluntary
> schedule. If they did, that would be bad.
OK, feeeling better now. ;-)
> > So what exactly is the trampoline code allowed to do? ;-)
>
> Well, it must be able to work in an NMI context, or bail otherwise. And
> it should never schedule on its own.
Good.
> > My problem is that I have no idea what can and cannot be included in
> > trampoline code. In absence of that information, my RCU-honed reflexes
> > jump immediately to the worst case that I can think of. ;-)
>
> Lets just say that it can't voluntarily sleep. Would that be good
> enough? If someday in the future I decide to let it do so, I would add
> a flag and force that ops not to be able to use a dynamic trampoline.
That would work. Again, feeling much better now. ;-)
> Currently, without the synchronize_rcu_tasks(), when a dynamic ops is
> registered, the functions will point to a non dynamic trampoline. That
> is, one that is never freed. It simply does:
>
> preempt_disable_notrace();
>
> do_for_each_ftrace_op(op, ftrace_ops_list) {
> /*
> * Check the following for each ops before calling their func:
> * if RCU flag is set, then rcu_is_watching() must be true
> * if PER_CPU is set, then ftrace_function_local_disable()
> * must be false
> * Otherwise test if the ip matches the ops filter
> *
> * If any of the above fails then the op->func() is not executed.
> */
> if ((!(op->flags & FTRACE_OPS_FL_RCU) || rcu_is_watching()) &&
> (!(op->flags & FTRACE_OPS_FL_PER_CPU) ||
> !ftrace_function_local_disabled(op)) &&
> ftrace_ops_test(op, ip, regs)) {
>
> if (FTRACE_WARN_ON(!op->func)) {
> pr_warn("op=%p %pS\n", op, op);
> goto out;
> }
> op->func(ip, parent_ip, op, regs);
> }
> } while_for_each_ftrace_op(op);
> out:
> preempt_enable_notrace();
Makes sense!
Thanx, Paul
next prev parent reply other threads:[~2017-04-12 16:27 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-11 21:18 There is a Tasks RCU stall warning Paul E. McKenney
2017-04-11 21:21 ` Steven Rostedt
2017-04-11 21:32 ` Paul E. McKenney
2017-04-11 21:31 ` Steven Rostedt
2017-04-11 21:34 ` Steven Rostedt
2017-04-11 21:39 ` Steven Rostedt
2017-04-11 21:44 ` Paul E. McKenney
2017-04-11 21:49 ` Steven Rostedt
2017-04-11 21:56 ` Paul E. McKenney
2017-04-11 22:15 ` Steven Rostedt
2017-04-11 23:01 ` Paul E. McKenney
2017-04-11 23:04 ` Paul E. McKenney
2017-04-11 23:11 ` Paul E. McKenney
2017-04-12 3:23 ` Paul E. McKenney
2017-04-12 13:18 ` Steven Rostedt
2017-04-12 14:19 ` Paul E. McKenney
2017-04-12 14:42 ` Steven Rostedt
2017-04-12 15:18 ` Paul E. McKenney
2017-04-12 15:53 ` Steven Rostedt
2017-04-12 16:26 ` Paul E. McKenney [this message]
2017-04-12 16:49 ` Steven Rostedt
2017-04-12 14:48 ` Paul E. McKenney
2017-04-12 14:59 ` Steven Rostedt
2017-04-12 16:27 ` Paul E. McKenney
2017-04-12 16:57 ` Steven Rostedt
2017-04-12 17:07 ` Paul E. McKenney
2017-04-12 17:13 ` Steven Rostedt
2017-04-12 20:02 ` Paul E. McKenney
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=20170412162610.GI3956@linux.vnet.ibm.com \
--to=paulmck@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=rostedt@goodmis.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;
as well as URLs for NNTP newsgroup(s).