Message-ID: <20220414213156.GZ4285@paulmck-ThinkPad-P17-Gen-1>
Date:   Thu, 14 Apr 2022 14:31:56 -0700
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Joel Fernandes <joel@...lfernandes.org>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Josh Triplett <josh@...htriplett.org>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        rcu <rcu@...r.kernel.org>, Steven Rostedt <rostedt@...dmis.org>,
        Frederic Weisbecker <frederic@...nel.org>
Subject: Re: [PATCH RFC] rcu/nocb: Provide default all-CPUs mask for
 RCU_NOCB_CPU=y

On Thu, Apr 14, 2022 at 09:14:05PM +0000, Joel Fernandes wrote:
> On Thu, Apr 14, 2022 at 02:09:33PM -0700, Paul E. McKenney wrote:
> > On Thu, Apr 14, 2022 at 03:49:16PM -0400, Joel Fernandes wrote:
> > > On Thu, Apr 14, 2022 at 3:42 PM Paul E. McKenney <paulmck@...nel.org> wrote:
> > > >
> > > > On Thu, Apr 14, 2022 at 07:19:48PM +0000, Joel Fernandes wrote:
> > > > > On Mon, Apr 11, 2022 at 08:41:09AM -0700, Paul E. McKenney wrote:
> > > > > [..]
> > > > > > > > [4]     All CPUs are offloaded at boot, and any CPU can be de-offloaded
> > > > > > > >         and offloaded at runtime.  This is the same behavior that
> > > > > > > >         you would currently get with CONFIG_RCU_NOCB_CPU_ALL=n and
> > > > > > > >         rcu_nocbs=0-N.
> > > > > > >
> > > > > > > Yes, this is the behavior I intend. So then there would not be a need
> > > > > > > to pass a mask (and I suspect for a large number of users, it
> > > > > > > simplifies boot params).
> > > > > >
> > > > > > Very good, and from what I can see, this should work for everyone.
> > > > >
> > > > > Just to clarify, what I am going to do is, if this new option =y, then
> > > > > rcu_nocbs effectively wont do anything. i.e. All CPUs are offloaded at boot.
> > > > > Let me know if we are not on the same page about it though. I do feel that is
> > > > > a sensible choice given =y. If we are on same page, please ignore my comment.
> > > >
> > > > I was assuming that the rcu_nocbs=??? for non-empty "???" would override
> > > > the CONFIG_RCU_NOCB_CPU_ALL=y.  If you choose not to do that, shouldn't
> > > > you at least issue some sort of diagnostic?  After all, the sysadmin
> > > > gave a kernel-boot parameter asking the code to do something and the
> > > > code is choosing not to do that something.
> > > >
> > > > Of course, such a sysadmin might want the CONFIG_RCU_NOCB_CPU_ALL=y
> > > > Kconfig option to affect only the default, that is, when no rcu_nocbs
> > > > kernel boot parameter is specified.  This would change the second "[4]"
> > > > in my original table to "[2]".
> > > >
> > > > Thoughts?
> > > 
> > > I thought about that. I feel that since we are defaulting the new
> > > config option to =n , it is a conscious choice by the distro to set it
> > > to =y.  In such a case, they should be Ok with offloading all CPUs. If
> > > they decide to selectively offload some CPUs in the future, then they
> > > could revisit the config option at that time.
> > > 
> > > I feel the kernel config should override the boot parameter behavior.
> > > It is the same effect as a sysadmin passing kernel parameter X
> > > assuming the kernel does something but the CONFIG option might not
> > > even build code corresponding to X.
> > > 
> > > I feel to address your concern, we can document in kernel command line
> > > documentation that rcu_nocbs= does not have an effect if
> > > CONFIG_RCU_NOCB_CPU_ALL=y, would that work for you?
> > 
> > Not me so much, because I would just set CONFIG_RCU_NOCB_CPU_ALL=n so
> > as to not worry about it.
> > 
> > But I am not at all looking forward to complaints about rcu_nocbs not
> > working the way people expect.  So let's take some time to think more
> > carefully about this.
> 
> That's a fair concern. But we are defaulting it to 'n' so I think if it is
> unconsciously enabled without someone reading documentation, then that's a
> slightly different issue.

Suppose that one group decides to change to CONFIG_RCU_NOCB_CPU_ALL=y,
and some other group on some other continent happens to be using the
rcu_nocbs boot parameter (having read the documentation).  And suppose
that the level of communication between the two groups is typical,
that is to say, nonexistent.

Sure, we can argue that groups should communicate, but our making that
argument won't necessarily prevent the group using rcu_nocbs from taking
us to task in the course of their debugging effort.

> On the other hand, I can also make it such that if rcu_nocbs= is passed, then
> the CONFIG does not take effect. That's quite a bit weird/quirky IMHO.

Not at all.  We can simply say that CONFIG_RCU_NOCB_CPU_ALL controls
only the default situation, that is, when rcu_nocbs is not specified.

							Thanx, Paul

> thanks,
> 
>  - Joel
> 
> 
> 
> > 							Thanx, Paul
> > 
> > > Thanks,
> > > 
> > > - Joel
> > > 
> > > >
> > > > > > > > I believe that Steve Rostedt's review would carry weight for ChromeOS,
> > > > > > > > however, I am suffering a senior moment on the right person for Android.
> > > > > > >
> > > > > > > I think for Android, Kalesh Singh is in the kernel team and Tim Murray
> > > > > > > is the performance lead. They could appropriately represent their RCU
> > > > > > > needs.
> > > > > >
> > > > > > Sounds good!  Please collect a Reviewed-by from one or both of them.
> > > > >
> > > > > Ok.
> > > >
> > > >                                                         Thanx, Paul

Powered by blists - more mailing lists