From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752555AbdG1RN2 (ORCPT ); Fri, 28 Jul 2017 13:13:28 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:56395 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752204AbdG1RN0 (ORCPT ); Fri, 28 Jul 2017 13:13:26 -0400 Date: Fri, 28 Jul 2017 10:13:21 -0700 From: "Paul E. McKenney" To: Steven Rostedt Cc: linux-kernel@vger.kernel.org, mingo@kernel.org, jiangshanlai@gmail.com, dipankar@in.ibm.com, akpm@linux-foundation.org, mathieu.desnoyers@efficios.com, josh@joshtriplett.org, tglx@linutronix.de, peterz@infradead.org, dhowells@redhat.com, edumazet@google.com, fweisbec@gmail.com, oleg@redhat.com Subject: Re: [PATCH tip/core/rcu 07/15] rcu: Add event tracing to ->gp_tasks update at GP start Reply-To: paulmck@linux.vnet.ibm.com References: <20170724214425.GA9665@linux.vnet.ibm.com> <1500932684-10469-7-git-send-email-paulmck@linux.vnet.ibm.com> <20170727213810.2a1c03d1@vmware.local.home> <20170728032232.GA3730@linux.vnet.ibm.com> <20170728081818.357a3612@vmware.local.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170728081818.357a3612@vmware.local.home> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-GCONF: 00 x-cbid: 17072817-0036-0000-0000-000002502D76 X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00007442; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000214; SDB=6.00894202; UDB=6.00447116; IPR=6.00674362; BA=6.00005496; NDR=6.00000001; ZLA=6.00000005; ZF=6.00000009; ZB=6.00000000; ZP=6.00000000; ZH=6.00000000; ZU=6.00000002; MB=3.00016427; XFM=3.00000015; UTC=2017-07-28 17:13:24 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17072817-0037-0000-0000-000041401191 Message-Id: <20170728171321.GG3730@linux.vnet.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-07-28_08:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1706020000 definitions=main-1707280275 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jul 28, 2017 at 08:18:18AM -0400, Steven Rostedt wrote: > On Thu, 27 Jul 2017 20:22:32 -0700 > "Paul E. McKenney" wrote: > > > On Thu, Jul 27, 2017 at 09:38:10PM -0400, Steven Rostedt wrote: > > > On Mon, 24 Jul 2017 14:44:36 -0700 > > > "Paul E. McKenney" wrote: > > > > > > > There is currently event tracing to track when a task is preempted > > > > within a preemptible RCU read-side critical section, and also when that > > > > task subsequently reaches its outermost rcu_read_unlock(), but none > > > > indicating when a new grace period starts when that grace period must > > > > wait on pre-existing readers that have been been preempted at least once > > > > since the beginning of their current RCU read-side critical sections. > > > > > > > > This commit therefore adds an event trace at grace-period start in > > > > the case where there are such readers. Note that only the first > > > > reader in the list is traced. > > > > > > > > Signed-off-by: Paul E. McKenney > > > > --- > > > > kernel/rcu/tree_plugin.h | 9 ++++++++- > > > > 1 file changed, 8 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h > > > > index 14ba496a13cd..3e3f92e981a1 100644 > > > > --- a/kernel/rcu/tree_plugin.h > > > > +++ b/kernel/rcu/tree_plugin.h > > > > @@ -636,10 +636,17 @@ static int rcu_print_task_exp_stall(struct rcu_node *rnp) > > > > */ > > > > static void rcu_preempt_check_blocked_tasks(struct rcu_node *rnp) > > > > { > > > > + struct task_struct *t; > > > > + > > > > RCU_LOCKDEP_WARN(preemptible(), "rcu_preempt_check_blocked_tasks() invoked with preemption enabled!!!\n"); > > > > WARN_ON_ONCE(rcu_preempt_blocked_readers_cgp(rnp)); > > > > - if (rcu_preempt_has_tasks(rnp)) > > > > + if (rcu_preempt_has_tasks(rnp)) { > > > > > > The only function of this if block is to fill the content of the > > > trace event, correct? > > > > > > What about doing: > > > > > > if (trace_rcu_unlock_preempted_task_enabled() && > > > rcu_preempt_has_tasks(rnp)) { > > > > > > instead? The trace_rcu_unlock_preempted_task_enabled() is a static > > > branch (aka jump_label), which would make the above a constant branch > > > when tracing is not enabled, and would keep this from adding any extra > > > overhead. > > > > > > -- Steve > > > > > > > rnp->gp_tasks = rnp->blkd_tasks.next; > > > > The trace_rcu_unlock_preempted_task_enabled() call is a new one on me, > > thank you! > > > > Unfortunately, the above assignment to rnp->gp_tasks is required even > > if tracing is disabled. The reason is that the newly started grace > > period needs to wait on all tasks that have been preempted within their > > current RCU read-side critical section, and rnp->gp_tasks records the > > point in the rnp->blkd_tasks list beyond which all preempted tasks block > > this new grace period. > > > > If this assignment is omitted, we get too-short grace periods, and the > > tasks on this list might still be holding references to stuff that gets > > freed at the end of this new grace period. > > > > I applied your two acks, thank you! > > > > And with you answer about the block not just being for tracing, you can > add my acked-by here too ;-) Added, thank you! Thanx, Paul