On Thu, May 12, 2022 at 05:16:22PM -0700, Paul E. McKenney wrote:
> On Thu, May 12, 2022 at 03:04:42AM +0000, Joel Fernandes (Google) wrote:
> > Add sysctl knobs just for easier debugging/testing, to tune the maximum
> > batch size, maximum time to wait before flush, and turning off the
> > feature entirely.
> >
> > Signed-off-by: Joel Fernandes (Google) <joel@xxxxxxxxxxxxxxxxx>
>
> This is good, and might also be needed longer term.
>
> One thought below.
>
> Thanx, Paul
>
> > ---
> > include/linux/sched/sysctl.h | 4 ++++
> > kernel/rcu/lazy.c | 12 ++++++++++--
> > kernel/sysctl.c | 23 +++++++++++++++++++++++
> > 3 files changed, 37 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/sched/sysctl.h b/include/linux/sched/sysctl.h
> > index c19dd5a2c05c..55ffc61beed1 100644
> > --- a/include/linux/sched/sysctl.h
> > +++ b/include/linux/sched/sysctl.h
> > @@ -16,6 +16,10 @@ enum { sysctl_hung_task_timeout_secs = 0 };
> >
> > extern unsigned int sysctl_sched_child_runs_first;
> >
> > +extern unsigned int sysctl_rcu_lazy;
> > +extern unsigned int sysctl_rcu_lazy_batch;
> > +extern unsigned int sysctl_rcu_lazy_jiffies;
> > +
> > enum sched_tunable_scaling {
> > SCHED_TUNABLESCALING_NONE,
> > SCHED_TUNABLESCALING_LOG,
> > diff --git a/kernel/rcu/lazy.c b/kernel/rcu/lazy.c
> > index 55e406cfc528..0af9fb67c92b 100644
> > --- a/kernel/rcu/lazy.c
> > +++ b/kernel/rcu/lazy.c
> > @@ -12,6 +12,10 @@
> > // How much to wait before flushing?
> > #define MAX_LAZY_JIFFIES 10000
> >
> > +unsigned int sysctl_rcu_lazy_batch = MAX_LAZY_BATCH;
> > +unsigned int sysctl_rcu_lazy_jiffies = MAX_LAZY_JIFFIES;
> > +unsigned int sysctl_rcu_lazy = 1;
> > +
> > // We cast lazy_rcu_head to rcu_head and back. This keeps the API simple while
> > // allowing us to use lockless list node in the head. Also, we use BUILD_BUG_ON
> > // later to ensure that rcu_head and lazy_rcu_head are of the same size.
> > @@ -49,6 +53,10 @@ void call_rcu_lazy(struct rcu_head *head_rcu, rcu_callback_t func)
> > struct lazy_rcu_head *head = (struct lazy_rcu_head *)head_rcu;
> > struct rcu_lazy_pcp *rlp;
> >
> > + if (!sysctl_rcu_lazy) {
>
> This is the place to check for early boot use. Or, alternatively,
> initialize sysctl_rcu_lazy to zero and set it to one once boot is far
> enough along to allow all the pieces to work reasonably.
Sure, I was also thinking perhaps to set this to a static branch. That way we
don't have to pay the cost of the branch after it is setup on boot.
But I think as you mentioned on IRC, if we were to promote this to a
non-DEBUG patch, we would need to handle conditions similar to Frederick's
work on toggling offloading.
thanks,
- Joel
