From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752827AbbBKQgu (ORCPT ); Wed, 11 Feb 2015 11:36:50 -0500 Received: from mx1.redhat.com ([209.132.183.28]:62261 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751421AbbBKQgt (ORCPT ); Wed, 11 Feb 2015 11:36:49 -0500 Date: Wed, 11 Feb 2015 16:48:24 +0000 From: Alexander Gordeev To: "Paul E. McKenney" Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/3] rcu: Call trace_rcu_batch_start() with enabled interrupts Message-ID: <20150211164823.GA23101@agordeev.usersys.redhat.com> References: <20150211161330.GH4166@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150211161330.GH4166@linux.vnet.ibm.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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(). > 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