lkml.org 
[lkml]   [2015]   [Feb]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 3/3] rcu: Call trace_rcu_batch_start() with enabled interrupts
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" <paulmck@linux.vnet.ibm.com>
> > > Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
> >
> > 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
>



\
 
 \ /
  Last update: 2015-02-11 18:41    [W:0.044 / U:3.884 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site