| Message ID | 20230112005223.2329802-2-joel@joelfernandes.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0b8ddef9f5c69f7193ebf1a3c38c279ee173722c |
| Headers | show |
| Series | [v2,rcu/dev,1/2] rcu: Track laziness during boot and suspend | expand |
On Thu, 12 Jan 2023 00:52:23 +0000 "Joel Fernandes (Google)" <joel@joelfernandes.org> wrote: > > static void > -__call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy) > +__call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy_in) > { > static atomic_t doublefrees; > unsigned long flags; > struct rcu_data *rdp; > - bool was_alldone; > + bool was_alldone, lazy; I'm curious to why the the extra variable. > > /* Misaligned rcu_head! */ > WARN_ON_ONCE((unsigned long)head & (sizeof(void *) - 1)); > @@ -2622,6 +2622,7 @@ __call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy) > kasan_record_aux_stack_noalloc(head); > local_irq_save(flags); > rdp = this_cpu_ptr(&rcu_data); > + lazy = lazy_in && !rcu_async_should_hurry(); Wouldn't just having: lazy = lazy && !rcu_async_should_hurry(); be sufficient? -- Steve > > /* Add the callback to our list. */ > if (unlikely(!rcu_segcblist_is_enabled(&rdp->cblist))) { > --
On Sun, Jan 15, 2023 at 3:55 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Thu, 12 Jan 2023 00:52:23 +0000 > "Joel Fernandes (Google)" <joel@joelfernandes.org> wrote: > > > > > static void > > -__call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy) > > +__call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy_in) > > { > > static atomic_t doublefrees; > > unsigned long flags; > > struct rcu_data *rdp; > > - bool was_alldone; > > + bool was_alldone, lazy; > > I'm curious to why the the extra variable. > > > > > /* Misaligned rcu_head! */ > > WARN_ON_ONCE((unsigned long)head & (sizeof(void *) - 1)); > > @@ -2622,6 +2622,7 @@ __call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy) > > kasan_record_aux_stack_noalloc(head); > > local_irq_save(flags); > > rdp = this_cpu_ptr(&rcu_data); > > + lazy = lazy_in && !rcu_async_should_hurry(); > > Wouldn't just having: > > lazy = lazy && !rcu_async_should_hurry(); > > be sufficient? I prefer to not overwrite function arguments, it makes debugging harder IMHO. - Joel > > -- Steve > > > > > /* Add the callback to our list. */ > > if (unlikely(!rcu_segcblist_is_enabled(&rdp->cblist))) { > > --
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index 78b2e999c904..c71e38d7815a 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -2594,12 +2594,12 @@ static void check_cb_ovld(struct rcu_data *rdp) } static void -__call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy) +__call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy_in) { static atomic_t doublefrees; unsigned long flags; struct rcu_data *rdp; - bool was_alldone; + bool was_alldone, lazy; /* Misaligned rcu_head! */ WARN_ON_ONCE((unsigned long)head & (sizeof(void *) - 1)); @@ -2622,6 +2622,7 @@ __call_rcu_common(struct rcu_head *head, rcu_callback_t func, bool lazy) kasan_record_aux_stack_noalloc(head); local_irq_save(flags); rdp = this_cpu_ptr(&rcu_data); + lazy = lazy_in && !rcu_async_should_hurry(); /* Add the callback to our list. */ if (unlikely(!rcu_segcblist_is_enabled(&rdp->cblist))) {
During suspend, we see failures to suspend 1 in 300-500 suspends. Looking closer, it appears that asynchronous RCU callbacks are being queued as lazy even though synchronous callbacks are expedited. These delays appear to not be very welcome by the suspend/resume code as evidenced by these occasional suspend failures. This commit modifies call_rcu() to check if rcu_async_should_hurry(), which will return true if we are in suspend or in-kernel boot. Ignoring the lazy hint makes the 3000 suspend/resume cycles pass reliably on a 12th gen 12-core Intel CPU, and there is some evidence that it also slightly speeds up boot performance. Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> --- kernel/rcu/tree.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)