Message ID | 20221019104045.160283-1-qiang1.zhang@intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4ac7:0:0:0:0:0 with SMTP id y7csp263514wrs; Wed, 19 Oct 2022 04:18:12 -0700 (PDT) X-Google-Smtp-Source: AMsMyM5IyaZ4Aw8KJ+Bk+0j2HjMR5BEu4XixlpKSNh3CS9wzkY/JZUb5tisiUkjfvqQD4aRCg27s X-Received: by 2002:a17:907:2c74:b0:78d:b3ae:8408 with SMTP id ib20-20020a1709072c7400b0078db3ae8408mr6355123ejc.172.1666178292325; Wed, 19 Oct 2022 04:18:12 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666178292; cv=none; d=google.com; s=arc-20160816; b=WtLnMq7gCpYDIw0CkgiC5c7HzOvOT3rL1WFyP4iPDfF2dpIAzorkL9ItxHhi/R/0fS e/rFxpMhWaex8QGkgQvcBmF+UA5i4BgdN0dr9d9JzQU16iKEybekcSxP/u4w4+BNVQgR t6jszN81bfuoa9OXCwW0/xdwg/wMIDswaCBV08zmnUddHXA8ECMs02+kihAv7GAGXL47 lOuzwIwqI0Aov6sd/L0KmoCz5iU49FLXGk5W3NucO2NxJI5diuJQHqgaU/CSJrhVlxK1 pxasMuhWRd/aBKeTRSL5KlQ1tnuP+hTinhWzpcPJ1B9dnO01sxm1Wi9e3Vf8KGMGqDRF j2vw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=GvPLAXPK1lZYdFAwvJPJ8nGtMarJjTfWozz6yp5XgYw=; b=NNAKuzu9gRC3VD9FuyVxzCeQIGlOkPomSh/9oySFSLkkrpPikwsgf396djeu0DeB+C 0b+4EPqjoFMeK0v8QZVtQYLcLR1tKPeLdrnkVm0O2kgiCi4akVf8IBVGJKOIddYubMqR HFlGh1Uti5voQxOM4CM9s3fwOn7PJynJxMNTYJ3GnEgiuLPIZMWZH4FF/yRMOky7fzrP Qu/C8Kd9F7JH4yc7yrZrMpqYWUThasuXZHzroSrxFCbCtbzNDRM6jEtHVbH43mgHcsGv VkWtPKDr5lte4vaBATjPCVybeK2/XHHvgRrABO2pZTZuvEirYIOL2IpjXTgZFNrbOa93 zgcQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=lYoVsCXt; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id y4-20020a056402440400b0045c2174074asi17055911eda.350.2022.10.19.04.17.44; Wed, 19 Oct 2022 04:18:12 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=lYoVsCXt; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232326AbiJSLHK (ORCPT <rfc822;samuel.l.nystrom@gmail.com> + 99 others); Wed, 19 Oct 2022 07:07:10 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33200 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234339AbiJSLGm (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 19 Oct 2022 07:06:42 -0400 Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E9917102DE8; Wed, 19 Oct 2022 03:35:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1666175742; x=1697711742; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=+Z1UQaZoC71MDvVH2UIbpGIxKwdpGYtiyrhpruzz4ZI=; b=lYoVsCXtzewdeWgEtzSywYxcbIEq/prA64yAzTp3JbD2qxKgkJwUpsmq bCq7MuEW8FGZMJ6NUuKFqXHVGksys/+9yGLZ0kaKq20YIv3t9j6iK49j5 PAIxhXoqDs9u67xNuMILQm+1/pwOtvP0Exa1Y8cZSOdDoVnYWdqrAb4cT AY64h+swjm/Sh6djBO5WuLqnqO9iWzxoSDtKpiFDu4o3OJLk3GKrT3vs2 l1fxUZxVKEmp9vkQfbgl2GHt2Q/A6mpp8sUtbaT3iUuuPL43SozA3bS7x J1AH1uMkNQBA7Njj7fJZl44DK/O+NtPp3HQ1zGtUXeMJPfJdnc1U4x8vE A==; X-IronPort-AV: E=McAfee;i="6500,9779,10504"; a="303989043" X-IronPort-AV: E=Sophos;i="5.95,196,1661842800"; d="scan'208";a="303989043" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Oct 2022 03:34:31 -0700 X-IronPort-AV: E=McAfee;i="6500,9779,10504"; a="607034094" X-IronPort-AV: E=Sophos;i="5.95,196,1661842800"; d="scan'208";a="607034094" Received: from zq-optiplex-7090.bj.intel.com ([10.238.156.129]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 19 Oct 2022 03:34:29 -0700 From: Zqiang <qiang1.zhang@intel.com> To: paulmck@kernel.org, frederic@kernel.org, joel@joelfernandes.org Cc: rcu@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] rcu: Make call_rcu() lazy only when CONFIG_RCU_LAZY is enabled Date: Wed, 19 Oct 2022 18:40:45 +0800 Message-Id: <20221019104045.160283-1-qiang1.zhang@intel.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI, SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1747114569253419471?= X-GMAIL-MSGID: =?utf-8?q?1747114569253419471?= |
Series |
rcu: Make call_rcu() lazy only when CONFIG_RCU_LAZY is enabled
|
|
Commit Message
Zqiang
Oct. 19, 2022, 10:40 a.m. UTC
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.
Signed-off-by: Zqiang <qiang1.zhang@intel.com>
---
kernel/rcu/tree.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
Comments
> On Oct 19, 2022, at 6:34 AM, Zqiang <qiang1.zhang@intel.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. > > Signed-off-by: Zqiang <qiang1.zhang@intel.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. 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? 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 >
> On Oct 19, 2022, at 8:10 AM, Joel Fernandes <joel@joelfernandes.org> wrote: > > > >> On Oct 19, 2022, at 6:34 AM, Zqiang <qiang1.zhang@intel.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. >> >> Signed-off-by: Zqiang <qiang1.zhang@intel.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. > > 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. 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 >>
On Wed, Oct 19, 2022 at 08:12:30AM -0400, Joel Fernandes wrote: > > On Oct 19, 2022, at 8:10 AM, Joel Fernandes <joel@joelfernandes.org> wrote: > >> On Oct 19, 2022, at 6:34 AM, Zqiang <qiang1.zhang@intel.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@intel.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 > >>
> On Oct 19, 2022, at 1:45 PM, Paul E. McKenney <paulmck@kernel.org> wrote: > > On Wed, Oct 19, 2022 at 08:12:30AM -0400, Joel Fernandes wrote: >>> On Oct 19, 2022, at 8:10 AM, Joel Fernandes <joel@joelfernandes.org> wrote: >>>>> On Oct 19, 2022, at 6:34 AM, Zqiang <qiang1.zhang@intel.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@intel.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. Sounds good. Zqiang, do you mind updating your patch along these lines? That way you get the proper attribution. More comments below: > >>> 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. It’s a trade off, I thought that’s why we wanted to have the binary search stuff. If no one reports issue on Linux-next, then that code won’t be put to use in the near future at least. > I do very strongly encourage the ChromeOS and Android folks to test this > very severely, however. Agreed. Yes that will happen, though I have to make a note for Android folks other than Vlad, to backports these (and enable the config option), carefully! Especially on pre-5.15 kernels. Luckily I had to do this (not so trivial) exercise myself. Thanks! - Joel > > 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 >>>>
> On Oct 19, 2022, at 1:45 PM, Paul E. McKenney <paulmck@kernel.org> wrote: > > On Wed, Oct 19, 2022 at 08:12:30AM -0400, Joel Fernandes wrote: >>> On Oct 19, 2022, at 8:10 AM, Joel Fernandes <joel@joelfernandes.org> wrote: >>>>> On Oct 19, 2022, at 6:34 AM, Zqiang <qiang1.zhang@intel.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@intel.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. > >Sounds good. Zqiang, do you mind updating your patch along these lines? That way you get the proper attribution. > Thanks Joel and Paul review, I will update my patch and resend. Thanks Zqiang >More comments below: > >>> 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. > >It’s a trade off, I thought that’s why we wanted to have the binary search stuff. If no one reports issue on Linux-next, then that code won’t be put to use in the near future at least. > > I do very strongly encourage the ChromeOS and Android folks to test this > very severely, however. > >Agreed. Yes that will happen, though I have to make a note for Android folks other than Vlad, to backports these (and enable the config option), carefully! Especially on pre-5.15 kernels. Luckily I had to do this (not so trivial) exercise myself. > >Thanks! > > - Joel > > > 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 >>>>
On Wed, Oct 19, 2022 at 02:25:29PM -0400, Joel Fernandes wrote: > > > > On Oct 19, 2022, at 1:45 PM, Paul E. McKenney <paulmck@kernel.org> wrote: > > > > On Wed, Oct 19, 2022 at 08:12:30AM -0400, Joel Fernandes wrote: > >>> On Oct 19, 2022, at 8:10 AM, Joel Fernandes <joel@joelfernandes.org> wrote: > >>>>> On Oct 19, 2022, at 6:34 AM, Zqiang <qiang1.zhang@intel.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@intel.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. > > Sounds good. Zqiang, do you mind updating your patch along these lines? That way you get the proper attribution. > > More comments below: > > > >>> 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. > > It’s a trade off, I thought that’s why we wanted to have the binary search stuff. If no one reports issue on Linux-next, then that code won’t be put to use in the near future at least. Well, not to put too fine a point on it, but we currently really are exposing -next to lazy call_rcu(). ;-) > > I do very strongly encourage the ChromeOS and Android folks to test this > > very severely, however. > > Agreed. Yes that will happen, though I have to make a note for Android folks other than Vlad, to backports these (and enable the config option), carefully! Especially on pre-5.15 kernels. Luckily I had to do this (not so trivial) exercise myself. And this is another situation in which the binary search stuff may prove extremely useful. Thanx, Paul > Thanks! > > - Joel > > > > > 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 > >>>>
> On Oct 19, 2022, at 7:34 PM, Paul E. McKenney <paulmck@kernel.org> wrote: > > On Wed, Oct 19, 2022 at 02:25:29PM -0400, Joel Fernandes wrote: >> >> >>>> On Oct 19, 2022, at 1:45 PM, Paul E. McKenney <paulmck@kernel.org> wrote: >>> >>> On Wed, Oct 19, 2022 at 08:12:30AM -0400, Joel Fernandes wrote: >>>>> On Oct 19, 2022, at 8:10 AM, Joel Fernandes <joel@joelfernandes.org> wrote: >>>>>>> On Oct 19, 2022, at 6:34 AM, Zqiang <qiang1.zhang@intel.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!! Indeed. >>>>>>> Signed-off-by: Zqiang <qiang1.zhang@intel.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. >> >> Sounds good. Zqiang, do you mind updating your patch along these lines? That way you get the proper attribution. Acked that patch. >> More comments below: >>> >>>>> 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. >> >> It’s a trade off, I thought that’s why we wanted to have the binary search stuff. If no one reports issue on Linux-next, then that code won’t be put to use in the near future at least. > > Well, not to put too fine a point on it, but we currently really are > exposing -next to lazy call_rcu(). ;-) This is true. I think I assumed nobody will enable a default off config option but I probably meant a smaller percentage will. >>> I do very strongly encourage the ChromeOS and Android folks to test this >>> very severely, however. >> >> Agreed. Yes that will happen, though I have to make a note for Android folks other than Vlad, to backports these (and enable the config option), carefully! Especially on pre-5.15 kernels. Luckily I had to do this (not so trivial) exercise myself. > > And this is another situation in which the binary search stuff may prove > extremely useful. Agreed. Thanks. Very least I owe per-rdp splitting of the hashtable, to that code. Steven and me talked today that probably the hashtable can go into the rcu_segcblist itself, and protect it by the nocb lock. Thanks, - Joel > Thanx, Paul > >> Thanks! >> >> - Joel >> >>> >>> 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 >>>>>>
On Thu, Oct 20, 2022 at 04:42:05AM -0400, Joel Fernandes wrote: > > On Oct 19, 2022, at 7:34 PM, Paul E. McKenney <paulmck@kernel.org> wrote: > > > > On Wed, Oct 19, 2022 at 02:25:29PM -0400, Joel Fernandes wrote: > >> > >> > >>>> On Oct 19, 2022, at 1:45 PM, Paul E. McKenney <paulmck@kernel.org> wrote: > >>> > >>> On Wed, Oct 19, 2022 at 08:12:30AM -0400, Joel Fernandes wrote: > >>>>> On Oct 19, 2022, at 8:10 AM, Joel Fernandes <joel@joelfernandes.org> wrote: > >>>>>>> On Oct 19, 2022, at 6:34 AM, Zqiang <qiang1.zhang@intel.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!! > > Indeed. > > >>>>>>> Signed-off-by: Zqiang <qiang1.zhang@intel.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. > >> > >> Sounds good. Zqiang, do you mind updating your patch along these lines? That way you get the proper attribution. > > Acked that patch. > > >> More comments below: > >>> > >>>>> 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. > >> > >> It’s a trade off, I thought that’s why we wanted to have the binary search stuff. If no one reports issue on Linux-next, then that code won’t be put to use in the near future at least. > > > > Well, not to put too fine a point on it, but we currently really are > > exposing -next to lazy call_rcu(). ;-) > > This is true. I think I assumed nobody will enable a default off config option but I probably meant a smaller percentage will. > > >>> I do very strongly encourage the ChromeOS and Android folks to test this > >>> very severely, however. > >> > >> Agreed. Yes that will happen, though I have to make a note for Android folks other than Vlad, to backports these (and enable the config option), carefully! Especially on pre-5.15 kernels. Luckily I had to do this (not so trivial) exercise myself. > > > > And this is another situation in which the binary search stuff may prove > > extremely useful. > > Agreed. Thanks. Very least I owe per-rdp splitting of the hashtable, to that code. Steven and me talked today that probably the hashtable can go into the rcu_segcblist itself, and protect it by the nocb lock. I have to ask... How does this fit in with CPU-hotplug and callback migration? More to the point, what events would cause us to decide that this is required? For example, shouldn't we give your current binary-search code at least a few chances to save the day? Thanx, Paul > >>>>>> +} > >>>>>> +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 > >>>>>>
> On Oct 20, 2022, at 2:39 PM, Paul E. McKenney <paulmck@kernel.org> wrote: > > On Thu, Oct 20, 2022 at 04:42:05AM -0400, Joel Fernandes wrote: >>>> On Oct 19, 2022, at 7:34 PM, Paul E. McKenney <paulmck@kernel.org> wrote: >>> >>> On Wed, Oct 19, 2022 at 02:25:29PM -0400, Joel Fernandes wrote: >>>> >>>> >>>>>> On Oct 19, 2022, at 1:45 PM, Paul E. McKenney <paulmck@kernel.org> wrote: >>>>> >>>>> On Wed, Oct 19, 2022 at 08:12:30AM -0400, Joel Fernandes wrote: >>>>>>> On Oct 19, 2022, at 8:10 AM, Joel Fernandes <joel@joelfernandes.org> wrote: >>>>>>>>> On Oct 19, 2022, at 6:34 AM, Zqiang <qiang1.zhang@intel.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!! >> >> Indeed. >> >>>>>>>>> Signed-off-by: Zqiang <qiang1.zhang@intel.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. >>>> >>>> Sounds good. Zqiang, do you mind updating your patch along these lines? That way you get the proper attribution. >> >> Acked that patch. >> >>>> More comments below: >>>>> >>>>>>> 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. >>>> >>>> It’s a trade off, I thought that’s why we wanted to have the binary search stuff. If no one reports issue on Linux-next, then that code won’t be put to use in the near future at least. >>> >>> Well, not to put too fine a point on it, but we currently really are >>> exposing -next to lazy call_rcu(). ;-) >> >> This is true. I think I assumed nobody will enable a default off config option but I probably meant a smaller percentage will. >> >>>>> I do very strongly encourage the ChromeOS and Android folks to test this >>>>> very severely, however. >>>> >>>> Agreed. Yes that will happen, though I have to make a note for Android folks other than Vlad, to backports these (and enable the config option), carefully! Especially on pre-5.15 kernels. Luckily I had to do this (not so trivial) exercise myself. >>> >>> And this is another situation in which the binary search stuff may prove >>> extremely useful. >> >> Agreed. Thanks. Very least I owe per-rdp splitting of the hashtable, to that code. Steven and me talked today that probably the hashtable can go into the rcu_segcblist itself, and protect it by the nocb lock. > > I have to ask... > > How does this fit in with CPU-hotplug and callback migration? Yes it will require change and I already thought of that, have to update the hashtable on all such events. > More to the point, what events would cause us to decide that this is > required? For example, shouldn't we give your current binary-search > code at least a few chances to save the day? Totally, if you’re taking the patch as is, I would be very happy. And I’ll continue to improve it with the above. But I was not sure yet if you’re taking it. I think it’s a worthwhile to take it for mainline in the current state and I’ll also add more data about callbacks to it in future (queuing time of callback, etc) — basically all the stuff I wanted to add to rcu_head. One reason for the above proposal is I also want to keep it turned on in production, and the current solution cannot be, due to the global locking and is not expected to be kept on in production. But is still a worthwhile addition for debug kernels IMO. Thanks, - Joel > Thanx, Paul > >>>>>>>> +} >>>>>>>> +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 >>>>>>>>
> On Oct 20, 2022, at 2:46 PM, Joel Fernandes <joel@joelfernandes.org> wrote: >>> >>>>> More comments below: >>>>>> >>>>>>>> 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. >>>>> >>>>> It’s a trade off, I thought that’s why we wanted to have the binary search stuff. If no one reports issue on Linux-next, then that code won’t be put to use in the near future at least. >>>> >>>> Well, not to put too fine a point on it, but we currently really are >>>> exposing -next to lazy call_rcu(). ;-) >>> >>> This is true. I think I assumed nobody will enable a default off config option but I probably meant a smaller percentage will. >>> >>>>>> I do very strongly encourage the ChromeOS and Android folks to test this >>>>>> very severely, however. >>>>> >>>>> Agreed. Yes that will happen, though I have to make a note for Android folks other than Vlad, to backports these (and enable the config option), carefully! Especially on pre-5.15 kernels. Luckily I had to do this (not so trivial) exercise myself. >>>> >>>> And this is another situation in which the binary search stuff may prove >>>> extremely useful. >>> >>> Agreed. Thanks. Very least I owe per-rdp splitting of the hashtable, to that code. Steven and me talked today that probably the hashtable can go into the rcu_segcblist itself, and protect it by the nocb lock. >> >> I have to ask... >> >> How does this fit in with CPU-hotplug and callback migration? > > Yes it will require change and I already thought of that, have to update the hashtable on all such events. > >> More to the point, what events would cause us to decide that this is >> required? For example, shouldn't we give your current binary-search >> code at least a few chances to save the day? > > Totally, if you’re taking the patch as is, I would be very happy. And I’ll continue to improve it with the above. But I was not sure yet if you’re taking it. > > I think it’s a worthwhile to take it for mainline in the current state and I’ll also add more data about callbacks to it in future (queuing time of callback, etc) — basically all the stuff I wanted to add to rcu_head. > > One reason for the above proposal is I also want to keep it turned on in production, and the current solution cannot be, due to the global locking and is not expected to be kept on in production. But is still a worthwhile addition for debug kernels IMO. I realized while talking to Steve that the hashtable has to be per CPU if we are to store more than a lazy flag, such as queuing timestamps. This is because you can have multiple callbacks of the same function pointer queued on multiple CPUs. So you can have multiple timestamps to store. Same thing if we stored automata. It’s per callback instance, not per callback function. Thanks, - Joel > > Thanks, > > - Joel > > >> Thanx, Paul >> >>>>>>>>> +} >>>>>>>>> +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 >>>>>>>>>
On Thu, Oct 20, 2022 at 05:33:37PM -0400, Joel Fernandes wrote: > > > > On Oct 20, 2022, at 2:46 PM, Joel Fernandes <joel@joelfernandes.org> wrote: > >>> > >>>>> More comments below: > >>>>>> > >>>>>>>> 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. > >>>>> > >>>>> It’s a trade off, I thought that’s why we wanted to have the binary search stuff. If no one reports issue on Linux-next, then that code won’t be put to use in the near future at least. > >>>> > >>>> Well, not to put too fine a point on it, but we currently really are > >>>> exposing -next to lazy call_rcu(). ;-) > >>> > >>> This is true. I think I assumed nobody will enable a default off config option but I probably meant a smaller percentage will. > >>> > >>>>>> I do very strongly encourage the ChromeOS and Android folks to test this > >>>>>> very severely, however. > >>>>> > >>>>> Agreed. Yes that will happen, though I have to make a note for Android folks other than Vlad, to backports these (and enable the config option), carefully! Especially on pre-5.15 kernels. Luckily I had to do this (not so trivial) exercise myself. > >>>> > >>>> And this is another situation in which the binary search stuff may prove > >>>> extremely useful. > >>> > >>> Agreed. Thanks. Very least I owe per-rdp splitting of the hashtable, to that code. Steven and me talked today that probably the hashtable can go into the rcu_segcblist itself, and protect it by the nocb lock. > >> > >> I have to ask... > >> > >> How does this fit in with CPU-hotplug and callback migration? > > > > Yes it will require change and I already thought of that, have to update the hashtable on all such events. > > > >> More to the point, what events would cause us to decide that this is > >> required? For example, shouldn't we give your current binary-search > >> code at least a few chances to save the day? > > > > Totally, if you’re taking the patch as is, I would be very happy. And I’ll continue to improve it with the above. But I was not sure yet if you’re taking it. > > > > I think it’s a worthwhile to take it for mainline in the current state and I’ll also add more data about callbacks to it in future (queuing time of callback, etc) — basically all the stuff I wanted to add to rcu_head. > > > > One reason for the above proposal is I also want to keep it turned on in production, and the current solution cannot be, due to the global locking and is not expected to be kept on in production. But is still a worthwhile addition for debug kernels IMO. > > I realized while talking to Steve that the hashtable has to be per CPU if we are to store more than a lazy flag, such as queuing timestamps. This is because you can have multiple callbacks of the same function pointer queued on multiple CPUs. So you can have multiple timestamps to store. Same thing if we stored automata. It’s per callback instance, not per callback function. Agreed, to be useful, this must be per callback instance. Thanx, Paul > Thanks, > > - Joel > > > > > > Thanks, > > > > - Joel > > > > > >> Thanx, Paul > >> > >>>>>>>>> +} > >>>>>>>>> +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 > >>>>>>>>>
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); +} +EXPORT_SYMBOL_GPL(call_rcu); +#endif /* Maximum number of jiffies to wait before draining a batch. */ #define KFREE_DRAIN_JIFFIES (5 * HZ)