Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753529AbbBKRU4 (ORCPT ); Wed, 11 Feb 2015 12:20:56 -0500 Received: from e37.co.us.ibm.com ([32.97.110.158]:52867 "EHLO e37.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752642AbbBKRUz (ORCPT ); Wed, 11 Feb 2015 12:20:55 -0500 Date: Wed, 11 Feb 2015 09:20:49 -0800 From: "Paul E. McKenney" To: Alexander Gordeev Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/3] rcu: Call trace_rcu_batch_start() with enabled interrupts Message-ID: <20150211172049.GJ4166@linux.vnet.ibm.com> Reply-To: paulmck@linux.vnet.ibm.com References: <20150211161330.GH4166@linux.vnet.ibm.com> <20150211164823.GA23101@agordeev.usersys.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150211164823.GA23101@agordeev.usersys.redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15021117-0025-0000-0000-0000087D2E93 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2877 Lines: 77 On Wed, Feb 11, 2015 at 04:48:24PM +0000, Alexander Gordeev wrote: > On Wed, Feb 11, 2015 at 08:13:30AM -0800, Paul E. McKenney wrote: > > On Wed, Feb 11, 2015 at 03:42:39PM +0100, Alexander Gordeev wrote: > > > Currently trace_rcu_batch_start() is called with local > > > interrupts disabled. Yet, there is no reason to do so. > > > > > > Cc: "Paul E. McKenney" > > > Signed-off-by: Alexander Gordeev > > > > Hmmm... I am not seeing this one. As you noted in the commit log for > > your earlier patch, the purpose of Tiny RCU is to be tiny, not to be > > all that fast. This commit increases the size a bit (admittedly only > > when CONFIG_RCU_TRACE=y), and also increases complexity a bit. > > > > So it does not look to me to be something we want for Tiny RCU. > > > > So what am I missing here? > > The benefit - "heavy" trace_rcu_batch_start() is called while interrupts > are enabled. Which is normally a priority, but in this case - still a > good tradeoff IMHO. > > And I do not agree :) The code reads better with the loop tightly "enclosed" > with trace_rcu_batch_start()/trace_rcu_batch_end(). Sorry, but I am still not seeing this one as being worth the change. I did take the other two, and they are passing light rcutorture testing, so we are good on that front, at least. Thanx, Paul > > > --- > > > kernel/rcu/tiny.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c > > > index 069742d..01e80ac 100644 > > > --- a/kernel/rcu/tiny.c > > > +++ b/kernel/rcu/tiny.c > > > @@ -166,11 +166,12 @@ static void __rcu_process_callbacks(struct rcu_ctrlblk *rcp) > > > const char *rn = NULL; > > > struct rcu_head *next, *list; > > > unsigned long flags; > > > + RCU_TRACE(long qlen); > > > RCU_TRACE(int cb_count = 0); > > > > > > /* Move the ready-to-invoke callbacks to a local list. */ > > > local_irq_save(flags); > > > - RCU_TRACE(trace_rcu_batch_start(rcp->name, 0, rcp->qlen, -1)); > > > + RCU_TRACE(qlen = rcp->qlen); > > > list = rcp->rcucblist; > > > rcp->rcucblist = *rcp->donetail; > > > *rcp->donetail = NULL; > > > @@ -180,6 +181,7 @@ static void __rcu_process_callbacks(struct rcu_ctrlblk *rcp) > > > local_irq_restore(flags); > > > > > > /* Invoke the callbacks on the local list. */ > > > + RCU_TRACE(trace_rcu_batch_start(rcp->name, 0, qlen, -1)); > > > RCU_TRACE(rn = rcp->name); > > > while (list) { > > > next = list->next; > > > -- > > > 1.8.3.1 > > > > > > > -- > Regards, > Alexander Gordeev > agordeev@redhat.com > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/