linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Stultz <john.stultz@linaro.org>
To: Paul McKenney <paulmck@linux.vnet.ibm.com>
Cc: Tejun Heo <tj@kernel.org>, Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	lkml <linux-kernel@vger.kernel.org>,
	Dmitry Shmidt <dimitrysh@google.com>,
	Rom Lemarchand <romlem@google.com>,
	Colin Cross <ccross@google.com>, Todd Kjos <tkjos@google.com>,
	Oleg Nesterov <oleg@redhat.com>
Subject: Re: Severe performance regression w/ 4.4+ on Android due to cgroup locking changes
Date: Wed, 13 Jul 2016 15:25:33 -0700	[thread overview]
Message-ID: <CALAqxLXqGqkzmE+Ck0V+4KhFO1f+q8Dk0DSbOpbEG0SEk19Pog@mail.gmail.com> (raw)
In-Reply-To: <20160713214238.GA15996@linux.vnet.ibm.com>

On Wed, Jul 13, 2016 at 2:42 PM, Paul E. McKenney
<paulmck@linux.vnet.ibm.com> wrote:
> On Wed, Jul 13, 2016 at 02:18:41PM -0700, Paul E. McKenney wrote:
>> On Wed, Jul 13, 2016 at 05:05:26PM -0400, Tejun Heo wrote:
>> > On Wed, Jul 13, 2016 at 02:03:15PM -0700, Paul E. McKenney wrote:
>> > > Take the patch that I just sent out and make the choice of normal
>> > > vs. expedited depend on CONFIG_PREEMPT_RT or whatever the -rt guys are
>> > > calling it these days.  Is there a low-latency Kconfig option other
>> > > than CONFIG_NO_HZ_FULL?
>> >
>> > Sounds like a plan to me.
>>
>> I like the way we like each other's idea.  Mutually assured laziness?  ;-)
>
> But here is what mine might look like.  Untested, probably does
> not even build.  Note that the default is -no- expediting, use the
> rcusync.expedited kernel parameter to enable it.
>
>                                                         Thanx, Paul
>
> ------------------------------------------------------------------------
>
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 82b42c958d1c..b8bc9854e548 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -3229,6 +3229,11 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>                         energy efficiency by requiring that the kthreads
>                         periodically wake up to do the polling.
>
> +       rcusync.expedited       [KNL]
> +                       Specify that the rcusync mechanism use expedited
> +                       grace periods.  As of mid-2016, this affects
> +                       per-CPU rwsems.
> +
>         rcutree.blimit= [KNL]
>                         Set maximum number of finished RCU callbacks to
>                         process in one batch.
> diff --git a/kernel/rcu/sync.c b/kernel/rcu/sync.c
> index be922c9f3d37..5bc5bef2e00a 100644
> --- a/kernel/rcu/sync.c
> +++ b/kernel/rcu/sync.c
> @@ -22,6 +22,14 @@
>
>  #include <linux/rcu_sync.h>
>  #include <linux/sched.h>
> +#include <linux/moduleparam.h>
> +#include <linux/module.h>
> +
> +MODULE_ALIAS("rcusync");
> +#ifdef MODULE_PARAM_PREFIX
> +#undef MODULE_PARAM_PREFIX
> +#endif
> +#define MODULE_PARAM_PREFIX "rcusync."
>
>  #ifdef CONFIG_PROVE_RCU
>  #define __INIT_HELD(func)      .held = func,
> @@ -29,7 +37,7 @@
>  #define __INIT_HELD(func)
>  #endif
>
> -static const struct {
> +static struct {
>         void (*sync)(void);
>         void (*call)(struct rcu_head *, void (*)(struct rcu_head *));
>         void (*wait)(void);
> @@ -62,6 +70,20 @@ enum { CB_IDLE = 0, CB_PENDING, CB_REPLAY };
>
>  #define        rss_lock        gp_wait.lock
>
> +static bool expedited;
> +module_param(expedited, bool, 0444);
> +
> +static int __init rcu_sync_early_init(void)
> +{
> +       if (expedited) {
> +               gp_ops[RCU_SYNC].sync = synchronize_rcu_expedited;
> +               gp_ops[RCU_SCHED_SYNC].sync = synchronize_sched_expedited;
> +               gp_ops[RCU_BH_SYNC].sync = synchronize_rcu_bh_expedited;
> +       }

So one minor nit here, with the config based default, you might want
to put some sort of informative message specifying that expidited was
used. This would help narrow down if it was or wasn't enabled when
folks see problems, since it wouldn't be otherwise obvious from a
dmesg log.

thanks
-john

  parent reply	other threads:[~2016-07-13 22:25 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-13  0:00 Severe performance regression w/ 4.4+ on Android due to cgroup locking changes John Stultz
2016-07-13  8:21 ` Peter Zijlstra
2016-07-13 14:42   ` Paul E. McKenney
2016-07-13 18:13     ` Dmitry Shmidt
2016-07-13 18:32       ` Paul E. McKenney
2016-07-13 18:21 ` Tejun Heo
2016-07-13 18:33   ` Tejun Heo
2016-07-13 20:13     ` John Stultz
2016-07-13 20:18       ` Tejun Heo
2016-07-13 20:26         ` Peter Zijlstra
2016-07-13 20:39           ` Tejun Heo
2016-07-13 20:51             ` Peter Zijlstra
2016-07-13 21:01               ` Tejun Heo
2016-07-13 21:03               ` Paul E. McKenney
2016-07-13 21:05                 ` Tejun Heo
2016-07-13 21:18                   ` Paul E. McKenney
2016-07-13 21:42                     ` Paul E. McKenney
2016-07-13 21:46                       ` John Stultz
2016-07-13 22:17                         ` Paul E. McKenney
2016-07-13 22:39                           ` John Stultz
2016-07-13 23:02                             ` Paul E. McKenney
2016-07-13 23:04                               ` Paul E. McKenney
2016-07-14 11:35                                 ` Tejun Heo
2016-07-14 12:04                                   ` Peter Zijlstra
2016-07-14 12:08                                     ` Tejun Heo
2016-07-14 12:20                                       ` Peter Zijlstra
2016-07-14 15:07                                         ` Tejun Heo
2016-07-14 15:24                                           ` Tejun Heo
2016-07-14 16:32                                           ` Peter Zijlstra
2016-07-14 17:34                                             ` Oleg Nesterov
2016-07-14 16:54                               ` John Stultz
2016-07-13 22:25                       ` John Stultz [this message]
2016-07-13 22:01                     ` Tejun Heo
2016-07-13 22:33                       ` Paul E. McKenney
2016-07-14  6:49                       ` Peter Zijlstra
2016-07-14 11:20                         ` Tejun Heo
2016-07-14 12:11                           ` Peter Zijlstra
2016-07-14 15:14                             ` Tejun Heo
2016-07-14 13:18               ` Peter Zijlstra
2016-07-14 14:14                 ` Peter Zijlstra
2016-07-14 14:58                 ` Oleg Nesterov
2016-07-14 16:14                   ` Peter Zijlstra
2016-07-14 16:37                   ` Peter Zijlstra
2016-07-14 17:05                     ` Oleg Nesterov
2016-07-14 16:23                 ` Paul E. McKenney
2016-07-14 16:45                   ` Peter Zijlstra
2016-07-14 17:15                     ` Paul E. McKenney
2016-07-14 16:43                 ` John Stultz
2016-07-14 16:49                   ` Peter Zijlstra
2016-07-14 17:02                     ` John Stultz
2016-07-14 17:13                       ` Oleg Nesterov
2016-07-14 17:30                         ` John Stultz
2016-07-14 17:41                           ` Oleg Nesterov
2016-07-14 17:51                             ` John Stultz
2016-07-14 18:09                 ` Oleg Nesterov
2016-07-14 18:36                   ` Peter Zijlstra
2016-07-14 19:35                     ` Peter Zijlstra
2016-07-13 20:57             ` John Stultz
2016-07-13 20:52           ` Paul E. McKenney
2016-07-13 20:57             ` Peter Zijlstra
2016-07-13 21:08               ` Paul E. McKenney
2016-07-13 21:01             ` Dmitry Shmidt
2016-07-13 21:03               ` John Stultz
2016-07-13 21:05               ` Paul E. McKenney
2016-07-13 20:31     ` Dmitry Shmidt
2016-07-13 20:44   ` Colin Cross
2016-07-13 20:54     ` Tejun Heo

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=CALAqxLXqGqkzmE+Ck0V+4KhFO1f+q8Dk0DSbOpbEG0SEk19Pog@mail.gmail.com \
    --to=john.stultz@linaro.org \
    --cc=ccross@google.com \
    --cc=dimitrysh@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=romlem@google.com \
    --cc=tj@kernel.org \
    --cc=tkjos@google.com \
    /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).