linux-kernel - Re: [PATCH] rcu: Make call_rcu() lazy only when CONFIG_RCU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221019174458.GD5600@paulmck-ThinkPad-P17-Gen-1>
Date:   Wed, 19 Oct 2022 10:44:58 -0700
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Joel Fernandes <joel@...lfernandes.org>
Cc:     Zqiang <qiang1.zhang@...el.com>, frederic@...nel.org,
        rcu@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] rcu: Make call_rcu() lazy only when CONFIG_RCU_LAZY is
 enabled

On Wed, Oct 19, 2022 at 08:12:30AM -0400, Joel Fernandes wrote:
> > On Oct 19, 2022, at 8:10 AM, Joel Fernandes <joel@...lfernandes.org> wrote:
> >> On Oct 19, 2022, at 6:34 AM, Zqiang <qiang1.zhang@...el.com> wrote:
> >> 
> >> Currently, regardless of whether the CONFIG_RCU_LAZY is enabled,
> >> invoke the call_rcu() is always lazy, it also means that when
> >> CONFIG_RCU_LAZY is disabled, invoke the call_rcu_flush() is also
> >> lazy. therefore, this commit make call_rcu() lazy only when
> >> CONFIG_RCU_LAZY is enabled.

First, good eyes!  Thank you for spotting this!!!

> >> Signed-off-by: Zqiang <qiang1.zhang@...el.com>
> >> ---
> >> kernel/rcu/tree.c | 8 +++++++-
> >> 1 file changed, 7 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> >> index abc615808b6e..97ef602da3d5 100644
> >> --- a/kernel/rcu/tree.c
> >> +++ b/kernel/rcu/tree.c
> >> @@ -2839,7 +2839,6 @@ void call_rcu_flush(struct rcu_head *head, rcu_callback_t func)
> >>   return __call_rcu_common(head, func, false);
> >> }
> >> EXPORT_SYMBOL_GPL(call_rcu_flush);
> >> -#endif
> >> 
> >> /**
> >> * call_rcu() - Queue an RCU callback for invocation after a grace period.
> >> @@ -2890,6 +2889,13 @@ void call_rcu(struct rcu_head *head, rcu_callback_t func)
> >>   return __call_rcu_common(head, func, true);
> >> }
> >> EXPORT_SYMBOL_GPL(call_rcu);
> >> +#else
> >> +void call_rcu(struct rcu_head *head, rcu_callback_t func)
> >> +{
> >> +    return __call_rcu_common(head, func, false);
> > 
> > Thanks. Instead of adding new function, you can also pass IS_ENABLED(CONFIG…) to the existing function of the same name.

I do like this approach better -- less code, more obvious what is going on.

> > Looks like though I made every one test the patch without having to enable the config option ;-). Hey, I’m a half glass full kind of guy, why do you ask?
> > 
> > Paul, I’ll take a closer look once I’m at the desk, but would you prefer to squash a diff into the existing patch, or want a new patch altogether?
> 
> On the other hand, what I’d want is to nuke the config option altogether or make it default y, we want to catch issues sooner than later.

That might be what we do at some point, but one thing at a time.  Let's
not penalize innocent bystanders, at least not just yet.

I do very strongly encourage the ChromeOS and Android folks to test this
very severely, however.

							Thanx, Paul

> Thanks.
> 
> > 
> > Thanks.
> > 
> > - Joel
> > 
> > 
> >> +}
> >> +EXPORT_SYMBOL_GPL(call_rcu);
> >> +#endif
> >> 
> >> /* Maximum number of jiffies to wait before draining a batch. */
> >> #define KFREE_DRAIN_JIFFIES (5 * HZ)
> >> -- 
> >> 2.25.1
> >>