From: "Paul E. McKenney" <paulmck@kernel.org>
To: Joel Fernandes <joel@joelfernandes.org>
Cc: rcu@vger.kernel.org, linux-kernel@vger.kernel.org,
rushikesh.s.kadam@intel.com, urezki@gmail.com,
neeraj.iitr10@gmail.com, frederic@kernel.org,
rostedt@goodmis.org, vineeth@bitbyteword.org
Subject: Re: [PATCH v2 1/8] rcu: Introduce call_rcu_lazy() API implementation
Date: Fri, 8 Jul 2022 16:10:13 -0700 [thread overview]
Message-ID: <20220708231013.GD1790663@paulmck-ThinkPad-P17-Gen-1> (raw)
In-Reply-To: <Ysh6yWThHu6GAfJM@google.com>
On Fri, Jul 08, 2022 at 06:43:21PM +0000, Joel Fernandes wrote:
> On Sat, Jun 25, 2022 at 09:00:19PM -0700, Paul E. McKenney wrote:
> > On Wed, Jun 22, 2022 at 10:50:55PM +0000, Joel Fernandes (Google) wrote:
> > > Implement timer-based RCU lazy callback batching. The batch is flushed
> > > whenever a certain amount of time has passed, or the batch on a
> > > particular CPU grows too big. Also memory pressure will flush it in a
> > > future patch.
> > >
> > > To handle several corner cases automagically (such as rcu_barrier() and
> > > hotplug), we re-use bypass lists to handle lazy CBs. The bypass list
> > > length has the lazy CB length included in it. A separate lazy CB length
> > > counter is also introduced to keep track of the number of lazy CBs.
> > >
> > > Suggested-by: Paul McKenney <paulmck@kernel.org>
> > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> >
> > Not bad, but some questions and comments below.
>
> Thanks a lot for these, real helpful and I replied below:
>
> > > diff --git a/include/linux/rcu_segcblist.h b/include/linux/rcu_segcblist.h
> > > index 659d13a7ddaa..9a992707917b 100644
> > > --- a/include/linux/rcu_segcblist.h
> > > +++ b/include/linux/rcu_segcblist.h
> > > @@ -22,6 +22,7 @@ struct rcu_cblist {
> > > struct rcu_head *head;
> > > struct rcu_head **tail;
> > > long len;
> > > + long lazy_len;
> > > };
> > >
> > > #define RCU_CBLIST_INITIALIZER(n) { .head = NULL, .tail = &n.head }
> > > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> > > index 1a32036c918c..9191a3d88087 100644
> > > --- a/include/linux/rcupdate.h
> > > +++ b/include/linux/rcupdate.h
> > > @@ -82,6 +82,12 @@ static inline int rcu_preempt_depth(void)
> > >
> > > #endif /* #else #ifdef CONFIG_PREEMPT_RCU */
> > >
> > > +#ifdef CONFIG_RCU_LAZY
> > > +void call_rcu_lazy(struct rcu_head *head, rcu_callback_t func);
> > > +#else
> > > +#define call_rcu_lazy(head, func) call_rcu(head, func)
> > > +#endif
> > > +
> > > /* Internal to kernel */
> > > void rcu_init(void);
> > > extern int rcu_scheduler_active;
> > > diff --git a/kernel/rcu/Kconfig b/kernel/rcu/Kconfig
> > > index 27aab870ae4c..0bffa992fdc4 100644
> > > --- a/kernel/rcu/Kconfig
> > > +++ b/kernel/rcu/Kconfig
> > > @@ -293,4 +293,12 @@ config TASKS_TRACE_RCU_READ_MB
> > > Say N here if you hate read-side memory barriers.
> > > Take the default if you are unsure.
> > >
> > > +config RCU_LAZY
> > > + bool "RCU callback lazy invocation functionality"
> > > + depends on RCU_NOCB_CPU
> > > + default n
> > > + help
> > > + To save power, batch RCU callbacks and flush after delay, memory
> > > + pressure or callback list growing too big.
> >
> > Spaces vs. tabs.
>
> Fixed, thanks.
>
> > The checkpatch warning is unhelpful ("please write a help paragraph that
> > fully describes the config symbol")
>
> Good old checkpatch :D
;-) ;-) ;-)
> > > endmenu # "RCU Subsystem"
> > > diff --git a/kernel/rcu/rcu_segcblist.c b/kernel/rcu/rcu_segcblist.c
> > > index c54ea2b6a36b..627a3218a372 100644
> > > --- a/kernel/rcu/rcu_segcblist.c
> > > +++ b/kernel/rcu/rcu_segcblist.c
> > > @@ -20,6 +20,7 @@ void rcu_cblist_init(struct rcu_cblist *rclp)
> > > rclp->head = NULL;
> > > rclp->tail = &rclp->head;
> > > rclp->len = 0;
> > > + rclp->lazy_len = 0;
> > > }
> > >
> > > /*
> > > @@ -32,6 +33,15 @@ void rcu_cblist_enqueue(struct rcu_cblist *rclp, struct rcu_head *rhp)
> > > WRITE_ONCE(rclp->len, rclp->len + 1);
> > > }
> > >
> > > +/*
> > > + * Enqueue an rcu_head structure onto the specified callback list.
> >
> > Please also note the fact that it is enqueuing lazily.
>
> Sorry, done.
>
> > > + */
> > > +void rcu_cblist_enqueue_lazy(struct rcu_cblist *rclp, struct rcu_head *rhp)
> > > +{
> > > + rcu_cblist_enqueue(rclp, rhp);
> > > + WRITE_ONCE(rclp->lazy_len, rclp->lazy_len + 1);
> >
> > Except... Why not just add a "lazy" parameter to rcu_cblist_enqueue()?
> > IS_ENABLED() can make it fast.
>
> Yeah good idea, it simplifies the code too. Thank you!
>
> So you mean I should add in this function so that the branch gets optimized:
> if (lazy && IS_ENABLE(CONFIG_RCU_LAZY)) {
> ...
> }
>
> That makes total sense considering the compiler may otherwise not be able to
> optimize the function viewing just the individual translation unit. I fixed
> it.
Or the other way around:
if (IS_ENABLE(CONFIG_RCU_LAZY) && lazy) {
Just in case the compiler is stumbling over its boolean logic. Or in
case the human reader is. ;-)
> The 6 month old baby and wife are calling my attention now. I will continue
> to reply to the other parts of this and other emails this evening and thanks
> for your help!
Ah, for those who believe that SIGCHLD can be ignored in real life! ;-)
Thanx, Paul
next prev parent reply other threads:[~2022-07-08 23:10 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-22 22:50 [PATCH v2 0/8] Implement call_rcu_lazy() and miscellaneous fixes Joel Fernandes (Google)
2022-06-22 22:50 ` [PATCH v2 1/1] context_tracking: Use arch_atomic_read() in __ct_state for KASAN Joel Fernandes (Google)
2022-06-22 22:58 ` Joel Fernandes
2022-06-22 22:50 ` [PATCH v2 1/8] rcu: Introduce call_rcu_lazy() API implementation Joel Fernandes (Google)
2022-06-22 23:18 ` Joel Fernandes
2022-06-26 4:00 ` Paul E. McKenney
2022-06-23 1:38 ` kernel test robot
2022-06-26 4:00 ` Paul E. McKenney
2022-07-08 18:43 ` Joel Fernandes
2022-07-08 23:10 ` Paul E. McKenney [this message]
2022-07-10 2:26 ` Joel Fernandes
2022-07-10 16:03 ` Paul E. McKenney
2022-07-12 20:53 ` Joel Fernandes
2022-07-12 21:04 ` Paul E. McKenney
2022-07-12 21:10 ` Joel Fernandes
2022-07-12 22:41 ` Paul E. McKenney
2022-06-29 11:53 ` Frederic Weisbecker
2022-06-29 17:05 ` Paul E. McKenney
2022-06-29 20:29 ` Joel Fernandes
2022-06-29 22:01 ` Frederic Weisbecker
2022-06-30 14:08 ` Joel Fernandes
2022-06-22 22:50 ` [PATCH v2 2/8] rcu: shrinker for lazy rcu Joel Fernandes (Google)
2022-06-22 22:50 ` [PATCH v2 3/8] fs: Move call_rcu() to call_rcu_lazy() in some paths Joel Fernandes (Google)
2022-06-22 22:50 ` [PATCH v2 4/8] rcu/nocb: Add option to force all call_rcu() to lazy Joel Fernandes (Google)
2022-06-22 22:50 ` [PATCH v2 5/8] rcu/nocb: Wake up gp thread when flushing Joel Fernandes (Google)
2022-06-26 4:06 ` Paul E. McKenney
2022-06-26 13:45 ` Joel Fernandes
2022-06-26 13:52 ` Paul E. McKenney
2022-06-26 14:37 ` Joel Fernandes
2022-06-22 22:51 ` [PATCH v2 6/8] rcuscale: Add test for using call_rcu_lazy() to emulate kfree_rcu() Joel Fernandes (Google)
2022-06-23 2:09 ` kernel test robot
2022-06-23 3:00 ` kernel test robot
2022-06-23 8:10 ` kernel test robot
2022-06-26 4:13 ` Paul E. McKenney
2022-07-08 4:25 ` Joel Fernandes
2022-07-08 23:06 ` Paul E. McKenney
2022-07-12 20:27 ` Joel Fernandes
2022-07-12 20:58 ` Paul E. McKenney
2022-07-12 21:15 ` Joel Fernandes
2022-07-12 22:41 ` Paul E. McKenney
2022-06-22 22:51 ` [PATCH v2 7/8] rcu/nocb: Rewrite deferred wake up logic to be more clean Joel Fernandes (Google)
2022-06-22 22:51 ` [PATCH v2 8/8] rcu/kfree: Fix kfree_rcu_shrink_count() return value Joel Fernandes (Google)
2022-06-26 4:17 ` Paul E. McKenney
2022-06-27 18:56 ` Uladzislau Rezki
2022-06-27 20:59 ` Paul E. McKenney
2022-06-27 21:18 ` Joel Fernandes
2022-06-27 21:43 ` Paul E. McKenney
2022-06-28 16:56 ` Joel Fernandes
2022-06-28 21:13 ` Joel Fernandes
2022-06-29 16:56 ` Paul E. McKenney
2022-06-29 19:47 ` Joel Fernandes
2022-06-29 21:07 ` Paul E. McKenney
2022-06-30 14:25 ` Joel Fernandes
2022-06-30 15:29 ` Paul E. McKenney
2022-06-29 16:52 ` Paul E. McKenney
2022-06-26 3:12 ` [PATCH v2 0/8] Implement call_rcu_lazy() and miscellaneous fixes Paul E. McKenney
2022-07-08 4:17 ` Joel Fernandes
2022-07-08 22:45 ` Paul E. McKenney
2022-07-10 1:38 ` Joel Fernandes
2022-07-10 15:47 ` 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=20220708231013.GD1790663@paulmck-ThinkPad-P17-Gen-1 \
--to=paulmck@kernel.org \
--cc=frederic@kernel.org \
--cc=joel@joelfernandes.org \
--cc=linux-kernel@vger.kernel.org \
--cc=neeraj.iitr10@gmail.com \
--cc=rcu@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=rushikesh.s.kadam@intel.com \
--cc=urezki@gmail.com \
--cc=vineeth@bitbyteword.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).