All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.ibm.com>
To: Joel Fernandes <joel@joelfernandes.org>
Cc: linux-kernel@vger.kernel.org, Rao Shoaib <rao.shoaib@oracle.com>,
	max.byungchul.park@gmail.com, byungchul.park@lge.com,
	kernel-team@android.com, kernel-team@lge.com,
	Davidlohr Bueso <dave@stgolabs.net>,
	Josh Triplett <josh@joshtriplett.org>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	rcu@vger.kernel.org, Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [PATCH RFC v1 1/2] rcu/tree: Add basic support for kfree_rcu batching
Date: Mon, 12 Aug 2019 07:44:01 -0700	[thread overview]
Message-ID: <20190812144401.GG28441@linux.ibm.com> (raw)
In-Reply-To: <20190812131356.GD27552@google.com>

On Mon, Aug 12, 2019 at 09:13:56AM -0400, Joel Fernandes wrote:
> On Sun, Aug 11, 2019 at 04:35:04PM -0700, Paul E. McKenney wrote:
> > On Sat, Aug 10, 2019 at 10:26:58PM -0400, Joel Fernandes wrote:
> > > On Sat, Aug 10, 2019 at 11:24:46AM -0700, Paul E. McKenney wrote:
> > > > On Sat, Aug 10, 2019 at 12:20:37AM -0400, Joel Fernandes wrote:
> > > > > On Fri, Aug 09, 2019 at 08:38:14PM -0700, Paul E. McKenney wrote:
> > > > > > On Fri, Aug 09, 2019 at 10:42:32PM -0400, Joel Fernandes wrote:
> > > > > > > On Wed, Aug 07, 2019 at 10:52:15AM -0700, Paul E. McKenney wrote:
> > > > > > > [snip] 
> > > > > > > > > > > @@ -3459,6 +3645,8 @@ void __init rcu_init(void)
> > > > > > > > > > >  {
> > > > > > > > > > >  	int cpu;
> > > > > > > > > > >  
> > > > > > > > > > > +	kfree_rcu_batch_init();
> > > > > > > > > > 
> > > > > > > > > > What happens if someone does a kfree_rcu() before this point?  It looks
> > > > > > > > > > like it should work, but have you tested it?
> > > > > > > > > > 
> > > > > > > > > > >  	rcu_early_boot_tests();
> > > > > > > > > > 
> > > > > > > > > > For example, by testing it in rcu_early_boot_tests() and moving the
> > > > > > > > > > call to kfree_rcu_batch_init() here.
> > > > > > > > > 
> > > > > > > > > I have not tried to do the kfree_rcu() this early. I will try it out.
> > > > > > > > 
> > > > > > > > Yeah, well, call_rcu() this early came as a surprise to me back in the
> > > > > > > > day, so...  ;-)
> > > > > > > 
> > > > > > > I actually did get surprised as well!
> > > > > > > 
> > > > > > > It appears the timers are not fully initialized so the really early
> > > > > > > kfree_rcu() call from rcu_init() does cause a splat about an initialized
> > > > > > > timer spinlock (even though future kfree_rcu()s and the system are working
> > > > > > > fine all the way into the torture tests).
> > > > > > > 
> > > > > > > I think to resolve this, we can just not do batching until early_initcall,
> > > > > > > during which I have an initialization function which switches batching on.
> > > > > > > >From that point it is safe.
> > > > > > 
> > > > > > Just go ahead and batch, but don't bother with the timer until
> > > > > > after single-threaded boot is done.  For example, you could check
> > > > > > rcu_scheduler_active similar to how sync_rcu_exp_select_cpus() does.
> > > > > > (See kernel/rcu/tree_exp.h.)
> > > > > 
> > > > > Cool, that works nicely and I tested it. Actually I made it such that we
> > > > > don't need to batch even, before the scheduler is up. I don't see any benefit
> > > > > of that unless we can see a kfree_rcu() flood happening that early at boot
> > > > > which seems highly doubtful as a real world case.
> > > > 
> > > > The benefit is removing the kfree_rcu() special cases from the innards
> > > > of RCU, for example, in rcu_do_batch().  Another benefit is removing the
> > > > current restriction on the position of the rcu_head structure within the
> > > > enclosing data structure.
> > > > 
> > > > So it would be good to avoid the current kfree_rcu() special casing within
> > > > RCU itself.
> > > > 
> > > > Or are you using some trick that avoids both the batching and the current
> > > > kfree_rcu() special casing?
> > > 
> > > Oh. I see what you mean. Would it be Ok with you to have that be a follow up
> > > patch?  I am not getting rid (yet) of the special casing in rcu_do_batch in
> > > this patch, but can do that in another patch.
> > 
> > I am OK having that in another patch, and I will be looking over yours
> > and Byungchul's two patches tomorrow.  If they look OK, I will queue them.
> 
> Ok, some of the code comments are stale as Byungchul pointed, allow me to fix
> them and then you can look at v3 directly, to save you the time.

Works for me, thank you!

							Thanx, Paul

> > However, I won't send them upstream without a follow-on patch that gets
> > rid of the kfree_rcu() special casing within rcu_do_batch() and perhaps
> > elsewhere.  This follow-on patch would of course also need to change rcuperf
> > appropriately.
> 
> Sounds good.
> 
> > > For now I am just doing something like the following in kfree_call_rcu(). I
> > > was almost about to hit send on the v1 and I have been testing this a lot so
> > > I'll post it anyway; and we can discuss more about this point on that.
> > > 
> > > +void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
> > > +{
> > > +       unsigned long flags;
> > > +       struct kfree_rcu_cpu *krcp;
> > > +       bool monitor_todo;
> > > +
> > > +       /* kfree_call_rcu() batching requires timers to be up. If the scheduler
> > > +        * is not yet up, just skip batching and do non-batched kfree_call_rcu().
> > > +        */
> > > +       if (rcu_scheduler_active != RCU_SCHEDULER_RUNNING)
> > > +               return kfree_call_rcu_nobatch(head, func);
> > > +
> > 
> > As a stopgap until the follow-on patch, this looks fine.
> 
> Cool, thanks!
> 
> - Joel
> 

  reply	other threads:[~2019-08-12 14:44 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-06 21:20 [PATCH RFC v1 1/2] rcu/tree: Add basic support for kfree_rcu batching Joel Fernandes (Google)
2019-08-06 21:20 ` [PATCH RFC v1 2/2] rcuperf: Add kfree_rcu performance Tests Joel Fernandes (Google)
2019-08-07  0:29   ` Paul E. McKenney
2019-08-07 10:22     ` Joel Fernandes
2019-08-07 17:56       ` Paul E. McKenney
2019-08-09 16:01         ` Joel Fernandes
2019-08-11  2:01     ` Joel Fernandes
2019-08-11 23:42       ` Paul E. McKenney
2019-08-06 23:56 ` [PATCH RFC v1 1/2] rcu/tree: Add basic support for kfree_rcu batching Paul E. McKenney
2019-08-07  9:45   ` Joel Fernandes
2019-08-07 17:52     ` Paul E. McKenney
2019-08-08  9:52       ` Byungchul Park
2019-08-08 12:56         ` Joel Fernandes
2019-08-08 14:23           ` Byungchul Park
2019-08-08 18:09             ` Paul E. McKenney
2019-08-11  8:36               ` Byungchul Park
2019-08-11  8:49                 ` Byungchul Park
2019-08-11 23:49                   ` Paul E. McKenney
2019-08-12 10:10                     ` Byungchul Park
2019-08-12 13:12                       ` Joel Fernandes
2019-08-13  5:29                         ` Byungchul Park
2019-08-13 15:41                           ` Paul E. McKenney
2019-08-14  0:11                             ` Byungchul Park
2019-08-14  2:53                               ` Paul E. McKenney
2019-08-14  3:43                                 ` Byungchul Park
2019-08-14 16:59                                   ` Paul E. McKenney
2019-08-11 10:37                 ` Byungchul Park
2019-08-08 23:30           ` Joel Fernandes
2019-08-09 15:16             ` Paul E. McKenney
2019-08-09 15:39               ` Joel Fernandes
2019-08-09 16:33                 ` Paul E. McKenney
2019-08-09 20:22                   ` Joel Fernandes
2019-08-09 20:26                     ` Joel Fernandes
2019-08-09 21:25                       ` Joel Fernandes
2019-08-10  3:38                         ` Paul E. McKenney
2019-08-09 20:29                     ` Joel Fernandes
2019-08-09 20:42                     ` Paul E. McKenney
2019-08-09 21:36                       ` Joel Fernandes
2019-08-10  3:40                         ` Paul E. McKenney
2019-08-10  3:52                           ` Joel Fernandes
2019-08-10  2:42       ` Joel Fernandes
2019-08-10  3:38         ` Paul E. McKenney
2019-08-10  4:20           ` Joel Fernandes
2019-08-10 18:24             ` Paul E. McKenney
2019-08-11  2:26               ` Joel Fernandes
2019-08-11 23:35                 ` Paul E. McKenney
2019-08-12 13:13                   ` Joel Fernandes
2019-08-12 14:44                     ` Paul E. McKenney [this message]
2019-08-08 10:26     ` Byungchul Park
2019-08-08 18:11       ` Paul E. McKenney
2019-08-08 20:13         ` Joel Fernandes
2019-08-08 20:51           ` Paul E. McKenney
2019-08-08 22:34             ` Joel Fernandes
2019-08-08 22:37               ` Paul E. McKenney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190812144401.GG28441@linux.ibm.com \
    --to=paulmck@linux.ibm.com \
    --cc=byungchul.park@lge.com \
    --cc=dave@stgolabs.net \
    --cc=jiangshanlai@gmail.com \
    --cc=joel@joelfernandes.org \
    --cc=josh@joshtriplett.org \
    --cc=kernel-team@android.com \
    --cc=kernel-team@lge.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=max.byungchul.park@gmail.com \
    --cc=rao.shoaib@oracle.com \
    --cc=rcu@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.