From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754627AbdDLPxR (ORCPT ); Wed, 12 Apr 2017 11:53:17 -0400 Received: from mail.kernel.org ([198.145.29.136]:50380 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754538AbdDLPxO (ORCPT ); Wed, 12 Apr 2017 11:53:14 -0400 Date: Wed, 12 Apr 2017 11:53:04 -0400 From: Steven Rostedt To: "Paul E. McKenney" Cc: linux-kernel@vger.kernel.org Subject: Re: There is a Tasks RCU stall warning Message-ID: <20170412115304.3077dbc8@gandalf.local.home> In-Reply-To: <20170412151817.GG3956@linux.vnet.ibm.com> References: <20170411174953.46adbf1e@gandalf.local.home> <20170411215656.GI1600@linux.vnet.ibm.com> <20170411181530.27dc21cc@gandalf.local.home> <20170411230154.GA3956@linux.vnet.ibm.com> <20170411230445.GA25951@linux.vnet.ibm.com> <20170411231138.GB25951@linux.vnet.ibm.com> <20170412032307.GA27011@linux.vnet.ibm.com> <20170412091821.4ad74bb0@gandalf.local.home> <20170412141936.GF3956@linux.vnet.ibm.com> <20170412104255.26bb17d4@gandalf.local.home> <20170412151817.GG3956@linux.vnet.ibm.com> X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 12 Apr 2017 08:18:17 -0700 "Paul E. McKenney" 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: : call 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 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. > > 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. > > 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. > > 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. 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(); -- Steve