rcu.vger.kernel.org archive mirror
 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: Wed, 7 Aug 2019 10:52:15 -0700	[thread overview]
Message-ID: <20190807175215.GE28441@linux.ibm.com> (raw)
In-Reply-To: <20190807094504.GB169551@google.com>

On Wed, Aug 07, 2019 at 05:45:04AM -0400, Joel Fernandes wrote:
> On Tue, Aug 06, 2019 at 04:56:31PM -0700, Paul E. McKenney wrote:
> > On Tue, Aug 06, 2019 at 05:20:40PM -0400, Joel Fernandes (Google) wrote:

[ . . . ]

> > > +	for (; head; head = next) {
> > > +		next = head->next;
> > > +		head->next = NULL;
> > > +		__call_rcu(head, head->func, -1, 1);
> > 
> > We need at least a cond_resched() here.  200,000 times through this loop
> > in a PREEMPT=n kernel might not always be pretty.  Except that this is
> > invoked directly from kfree_rcu() which might be invoked with interrupts
> > disabled, which precludes calls to cond_resched().  So the realtime guys
> > are not going to be at all happy with this loop.
> 
> Ok, will add this here.
> 
> > And this loop could be avoided entirely by having a third rcu_head list
> > in the kfree_rcu_cpu structure.  Yes, some of the batches would exceed
> > KFREE_MAX_BATCH, but given that they are invoked from a workqueue, that
> > should be OK, or at least more OK than queuing 200,000 callbacks with
> > interrupts disabled.  (If it turns out not to be OK, an array of rcu_head
> > pointers can be used to reduce the probability of oversized batches.)
> > This would also mean that the equality comparisons with KFREE_MAX_BATCH
> > need to become greater-or-equal comparisons or some such.
> 
> Yes, certainly we can do these kinds of improvements after this patch, and
> then add more tests to validate the improvements.

Out of pity for people bisecting, we need this fixed up front.

My suggestion is to just allow ->head to grow until ->head_free becomes
available.  That way you are looping with interrupts and preemption
enabled in workqueue context, which is much less damaging than doing so
with interrupts disabled, and possibly even from hard-irq context.

But please feel free to come up with a better solution!

[ . . . ]

> > > @@ -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...  ;-)

							Thanx, Paul

  reply	other threads:[~2019-08-07 17:53 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 [this message]
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
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=20190807175215.GE28441@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).