Message ID | 20230112005223.2329802-1-joel@joelfernandes.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:4e01:0:0:0:0:0 with SMTP id p1csp3625092wrt; Wed, 11 Jan 2023 17:09:03 -0800 (PST) X-Google-Smtp-Source: AMrXdXvJyviXaSVMyaahijg7njwdfZi3ib7wPq4p8f1XYvALNbi3RsUG5GH2K97x0pbfbZERGS78 X-Received: by 2002:a17:906:8a52:b0:84d:4732:65df with SMTP id gx18-20020a1709068a5200b0084d473265dfmr9893995ejc.61.1673485743040; Wed, 11 Jan 2023 17:09:03 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1673485743; cv=none; d=google.com; s=arc-20160816; b=eo1KvA14/4qE1/o7FFz3qQ5x5UJYZYcb+FV4brEgEc1mHDDoLeQ5bCFXMCQdjJfstS zxBea63VP6dPaad0XaE15flRhyGTWjMVKGC7Vw9I+FCeXaX6+cZV0xiZF8BzBD7eMEff vNal9mCa8sZbq2S6xM0Jj6z695WP0sYrZmAViKlP4jVuB3+1SDQBSdHrox5THERUyn6Z /Kd25EzT/CQQNI0xr5pqNdYHLaqDEGxKs7CAQlJGWYmKDdQ6mD9thWnWX6a0sipHZhgb MXbB0Aun3LunO5nlifBFZlenPXNm96GXo7JDdrrfZyHio1YkBjnVubJCY3nx6El0RPGA XZZQ== 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=Qng87r2afyBWFugVmr/ToSxitn179pDEw8bgUuU7NSI=; b=MCR8R51lffUkZEg08IcrDY0lbWcC/6oPlvKz1+01aFo7Vg+hht6OCKBhZNukUr17gW jpJgMHB8W4BltKupOcHkwyZ4I6exJ/gU5kPP7hhwSovpoFVxM7yyDVreY3zWsy7CNYah 9AcQO6vOqx534holQFaxi5SPPr4/NQKJQgrgBsAdkL3HrgGfOoeQHVcT/N+dhAHum9Sx RIWeNsYwjBQliosZr+PdOJTtQA7xKplsg6zaWIYn6NFbYGuv4LUdLqhp4T+6sJFp7pak UOrLT+DdcbMu7Hq0OksFLwtUWhROm7Y8LjdY0+Nt9ivuNCWMXC6dhxOiqbZ1uoDTzCUJ FWcQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@joelfernandes.org header.s=google header.b=gvNbDNcf; 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 Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id sc3-20020a1709078a0300b0078d20d71475si18446865ejc.413.2023.01.11.17.08.39; Wed, 11 Jan 2023 17:09:03 -0800 (PST) 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=@joelfernandes.org header.s=google header.b=gvNbDNcf; 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 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235863AbjALAwh (ORCPT <rfc822;syz17693488234@gmail.com> + 99 others); Wed, 11 Jan 2023 19:52:37 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39630 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235581AbjALAwf (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 11 Jan 2023 19:52:35 -0500 Received: from mail-qt1-x835.google.com (mail-qt1-x835.google.com [IPv6:2607:f8b0:4864:20::835]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D0D09F584 for <linux-kernel@vger.kernel.org>; Wed, 11 Jan 2023 16:52:34 -0800 (PST) Received: by mail-qt1-x835.google.com with SMTP id fd15so5389228qtb.9 for <linux-kernel@vger.kernel.org>; Wed, 11 Jan 2023 16:52:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=joelfernandes.org; s=google; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=Qng87r2afyBWFugVmr/ToSxitn179pDEw8bgUuU7NSI=; b=gvNbDNcf4RL8zlbzVOgMt/Grt6Xy2W9WrWdryZnQNlhiIQrZvRAvmUKTl360QG43D2 UEUwVYJQJn+v2Tm/K46xHvqw48oryZvzwGWvybOMqLhEao9wJCwaFhfLWrQCHCZlGfAF 9Xz6Si1vL7gFeawCEE+HufH2JKzJWq55Tqr00= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Qng87r2afyBWFugVmr/ToSxitn179pDEw8bgUuU7NSI=; b=1q2yRzmAL+xsgKUbzICn8PyLyKB19L4ZvGdtZTHHHB0PDaYT+euEwiKmkeBrL72B1A Z3vlAhpFvJhNvjTc7u3NswR9miPCNO2IYmGykjNeIekcM3dqNlfZQsdqYAxb1tufKiy6 5Ee6tFiiQQg9VwRI8Vl7PUOydJZWqqlpOdWUh/OOdXF5VyT9bcq6vc3TgW83P7kF9uF9 yTfSuVYvgwCvYx10pVR18uLF6+5b8hpCeVno9eD87uyzooRd5xPyBNypuBs+ZK7PquIm h59HfFv5bZsyKKZzCNl9yoEyVWOMJ1TPecyN3b47OlUE/kEwEs24oSEzp5PMBiMWqf5F sBLg== X-Gm-Message-State: AFqh2koK7ztuaG2O3LWHKaP3lcmQ/3e1YiHM1jpUtACUGitaFkeqCcJm Gm1FxqAIxs+YeHiHfHl0KRC6905H5wqfqGPw X-Received: by 2002:ac8:5710:0:b0:3a9:86dd:3c60 with SMTP id 16-20020ac85710000000b003a986dd3c60mr137977132qtw.47.1673484753191; Wed, 11 Jan 2023 16:52:33 -0800 (PST) Received: from joelboxx.c.googlers.com.com (129.239.188.35.bc.googleusercontent.com. [35.188.239.129]) by smtp.gmail.com with ESMTPSA id z9-20020ac87ca9000000b003a7e2aea23esm8307523qtv.86.2023.01.11.16.52.31 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 11 Jan 2023 16:52:32 -0800 (PST) From: "Joel Fernandes (Google)" <joel@joelfernandes.org> To: linux-kernel@vger.kernel.org Cc: "Joel Fernandes (Google)" <joel@joelfernandes.org>, Josh Triplett <josh@joshtriplett.org>, Lai Jiangshan <jiangshanlai@gmail.com>, Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, "Paul E. McKenney" <paulmck@kernel.org>, rcu@vger.kernel.org, Steven Rostedt <rostedt@goodmis.org>, fweisbec@gmail.com, urezki@gmail.com Subject: [PATCH v2 rcu/dev 1/2] rcu: Track laziness during boot and suspend Date: Thu, 12 Jan 2023 00:52:22 +0000 Message-Id: <20230112005223.2329802-1-joel@joelfernandes.org> X-Mailer: git-send-email 2.39.0.314.g84b9a713c41-goog MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS autolearn=unavailable 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?1754776986340231006?= X-GMAIL-MSGID: =?utf-8?q?1754776986340231006?= |
Series |
[v2,rcu/dev,1/2] rcu: Track laziness during boot and suspend
|
|
Commit Message
Joel Fernandes
Jan. 12, 2023, 12:52 a.m. UTC
During boot and suspend/resume, it is desired to prevent RCU laziness from
effecting performance and in some cases failures like with suspend.
Track whether RCU laziness is to be ignored or not, in kernels with
CONFIG_RCU_LAZY enabled. We do such tracking for expedited-RCU already, however
since Android currently expedites synchronous_rcu() always, we cannot rely on
that. The next patch ignores laziness hints based on this tracking.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
Paul, could we take this and the next one for 6.2 -rc cycle?
I also booted debian Linux and verified the flag is reset correctly after boot
completes. Thanks.
kernel/rcu/rcu.h | 6 ++++++
kernel/rcu/tree.c | 2 ++
kernel/rcu/update.c | 40 +++++++++++++++++++++++++++++++++++++++-
5 files changed, 55 insertions(+), 1 deletion(-)
create mode 100644 cc_list
create mode 100644 to_list
Comments
On Thu, Jan 12, 2023 at 12:52:22AM +0000, Joel Fernandes (Google) wrote: > During boot and suspend/resume, it is desired to prevent RCU laziness from > effecting performance and in some cases failures like with suspend. > > Track whether RCU laziness is to be ignored or not, in kernels with > CONFIG_RCU_LAZY enabled. We do such tracking for expedited-RCU already, however > since Android currently expedites synchronous_rcu() always, we cannot rely on > that. The next patch ignores laziness hints based on this tracking. > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> > --- > Paul, could we take this and the next one for 6.2 -rc cycle? Given how late it is in v6.2, I would need you to make it all depend on CONFIG_RCU_LAZY, so that it is blindingly and immediately obvious that there is no change in behavior for kernels built with CONFIG_RCU_LAZY=n. If you are OK with the v6.3 merge window and backports, what you have likely suffices, modulo review and testing. So, how would you like to proceed? Thanx, Paul > I also booted debian Linux and verified the flag is reset correctly after boot > completes. Thanks. > > kernel/rcu/rcu.h | 6 ++++++ > kernel/rcu/tree.c | 2 ++ > kernel/rcu/update.c | 40 +++++++++++++++++++++++++++++++++++++++- > 5 files changed, 55 insertions(+), 1 deletion(-) > create mode 100644 cc_list > create mode 100644 to_list > > diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h > index 5c8013f7085f..115616ac3bfa 100644 > --- a/kernel/rcu/rcu.h > +++ b/kernel/rcu/rcu.h > @@ -449,14 +449,20 @@ do { \ > /* Tiny RCU doesn't expedite, as its purpose in life is instead to be tiny. */ > static inline bool rcu_gp_is_normal(void) { return true; } > static inline bool rcu_gp_is_expedited(void) { return false; } > +static inline bool rcu_async_should_hurry(void) { return false; } > static inline void rcu_expedite_gp(void) { } > static inline void rcu_unexpedite_gp(void) { } > +static inline void rcu_async_hurry(void) { } > +static inline void rcu_async_relax(void) { } > static inline void rcu_request_urgent_qs_task(struct task_struct *t) { } > #else /* #ifdef CONFIG_TINY_RCU */ > bool rcu_gp_is_normal(void); /* Internal RCU use. */ > bool rcu_gp_is_expedited(void); /* Internal RCU use. */ > +bool rcu_async_should_hurry(void); /* Internal RCU use. */ > void rcu_expedite_gp(void); > void rcu_unexpedite_gp(void); > +void rcu_async_hurry(void); > +void rcu_async_relax(void); > void rcupdate_announce_bootup_oddness(void); > #ifdef CONFIG_TASKS_RCU_GENERIC > void show_rcu_tasks_gp_kthreads(void); > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > index 63545d79da51..78b2e999c904 100644 > --- a/kernel/rcu/tree.c > +++ b/kernel/rcu/tree.c > @@ -4504,11 +4504,13 @@ static int rcu_pm_notify(struct notifier_block *self, > switch (action) { > case PM_HIBERNATION_PREPARE: > case PM_SUSPEND_PREPARE: > + rcu_async_hurry(); > rcu_expedite_gp(); > break; > case PM_POST_HIBERNATION: > case PM_POST_SUSPEND: > rcu_unexpedite_gp(); > + rcu_async_relax(); > break; > default: > break; > diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c > index 3893022f8ed8..19bf6fa3ee6a 100644 > --- a/kernel/rcu/update.c > +++ b/kernel/rcu/update.c > @@ -144,8 +144,45 @@ bool rcu_gp_is_normal(void) > } > EXPORT_SYMBOL_GPL(rcu_gp_is_normal); > > -static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1); > +static atomic_t rcu_async_hurry_nesting = ATOMIC_INIT(1); > +/* > + * Should call_rcu() callbacks be processed with urgency or are > + * they OK being executed with arbitrary delays? > + */ > +bool rcu_async_should_hurry(void) > +{ > + return !IS_ENABLED(CONFIG_RCU_LAZY) || > + atomic_read(&rcu_async_hurry_nesting); > +} > +EXPORT_SYMBOL_GPL(rcu_async_should_hurry); > + > +/** > + * rcu_async_hurry - Make future async RCU callbacks not lazy. > + * > + * After a call to this function, future calls to call_rcu() > + * will be processed in a timely fashion. > + */ > +void rcu_async_hurry(void) > +{ > + if (IS_ENABLED(CONFIG_RCU_LAZY)) > + atomic_inc(&rcu_async_hurry_nesting); > +} > +EXPORT_SYMBOL_GPL(rcu_async_hurry); > > +/** > + * rcu_async_relax - Make future async RCU callbacks lazy. > + * > + * After a call to this function, future calls to call_rcu() > + * will be processed in a lazy fashion. > + */ > +void rcu_async_relax(void) > +{ > + if (IS_ENABLED(CONFIG_RCU_LAZY)) > + atomic_dec(&rcu_async_hurry_nesting); > +} > +EXPORT_SYMBOL_GPL(rcu_async_relax); > + > +static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1); > /* > * Should normal grace-period primitives be expedited? Intended for > * use within RCU. Note that this function takes the rcu_expedited > @@ -195,6 +232,7 @@ static bool rcu_boot_ended __read_mostly; > void rcu_end_inkernel_boot(void) > { > rcu_unexpedite_gp(); > + rcu_async_relax(); > if (rcu_normal_after_boot) > WRITE_ONCE(rcu_normal, 1); > rcu_boot_ended = true; > -- > 2.39.0.314.g84b9a713c41-goog
> On Jan 11, 2023, at 8:29 PM, Paul E. McKenney <paulmck@kernel.org> wrote: > > On Thu, Jan 12, 2023 at 12:52:22AM +0000, Joel Fernandes (Google) wrote: >> During boot and suspend/resume, it is desired to prevent RCU laziness from >> effecting performance and in some cases failures like with suspend. >> >> Track whether RCU laziness is to be ignored or not, in kernels with >> CONFIG_RCU_LAZY enabled. We do such tracking for expedited-RCU already, however >> since Android currently expedites synchronous_rcu() always, we cannot rely on >> that. The next patch ignores laziness hints based on this tracking. >> >> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> >> --- >> Paul, could we take this and the next one for 6.2 -rc cycle? > > Given how late it is in v6.2, I would need you to make it all depend on > CONFIG_RCU_LAZY, so that it is blindingly and immediately obvious that > there is no change in behavior for kernels built with CONFIG_RCU_LAZY=n. > > If you are OK with the v6.3 merge window and backports, what you have > likely suffices, modulo review and testing. > > So, how would you like to proceed? Yes that is fine with me. Though, it is right now fully compile-time dependent on the lazy config, except the definition of the integer which I can’t put behind a config because of ugly ifdefs. I would expect LTO to remove the integer definition automatically. But it has no effect otherwise. I do worry a bit about users who are not using stable and have their own kernel versions with backports. But maybe a fixes tag will get their attention? Android and ChromeOS should both be fine though, given they swear by stable. Thanks, - Joel > > Thanx, Paul > >> I also booted debian Linux and verified the flag is reset correctly after boot >> completes. Thanks. >> >> kernel/rcu/rcu.h | 6 ++++++ >> kernel/rcu/tree.c | 2 ++ >> kernel/rcu/update.c | 40 +++++++++++++++++++++++++++++++++++++++- >> 5 files changed, 55 insertions(+), 1 deletion(-) >> create mode 100644 cc_list >> create mode 100644 to_list >> >> diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h >> index 5c8013f7085f..115616ac3bfa 100644 >> --- a/kernel/rcu/rcu.h >> +++ b/kernel/rcu/rcu.h >> @@ -449,14 +449,20 @@ do { \ >> /* Tiny RCU doesn't expedite, as its purpose in life is instead to be tiny. */ >> static inline bool rcu_gp_is_normal(void) { return true; } >> static inline bool rcu_gp_is_expedited(void) { return false; } >> +static inline bool rcu_async_should_hurry(void) { return false; } >> static inline void rcu_expedite_gp(void) { } >> static inline void rcu_unexpedite_gp(void) { } >> +static inline void rcu_async_hurry(void) { } >> +static inline void rcu_async_relax(void) { } >> static inline void rcu_request_urgent_qs_task(struct task_struct *t) { } >> #else /* #ifdef CONFIG_TINY_RCU */ >> bool rcu_gp_is_normal(void); /* Internal RCU use. */ >> bool rcu_gp_is_expedited(void); /* Internal RCU use. */ >> +bool rcu_async_should_hurry(void); /* Internal RCU use. */ >> void rcu_expedite_gp(void); >> void rcu_unexpedite_gp(void); >> +void rcu_async_hurry(void); >> +void rcu_async_relax(void); >> void rcupdate_announce_bootup_oddness(void); >> #ifdef CONFIG_TASKS_RCU_GENERIC >> void show_rcu_tasks_gp_kthreads(void); >> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c >> index 63545d79da51..78b2e999c904 100644 >> --- a/kernel/rcu/tree.c >> +++ b/kernel/rcu/tree.c >> @@ -4504,11 +4504,13 @@ static int rcu_pm_notify(struct notifier_block *self, >> switch (action) { >> case PM_HIBERNATION_PREPARE: >> case PM_SUSPEND_PREPARE: >> + rcu_async_hurry(); >> rcu_expedite_gp(); >> break; >> case PM_POST_HIBERNATION: >> case PM_POST_SUSPEND: >> rcu_unexpedite_gp(); >> + rcu_async_relax(); >> break; >> default: >> break; >> diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c >> index 3893022f8ed8..19bf6fa3ee6a 100644 >> --- a/kernel/rcu/update.c >> +++ b/kernel/rcu/update.c >> @@ -144,8 +144,45 @@ bool rcu_gp_is_normal(void) >> } >> EXPORT_SYMBOL_GPL(rcu_gp_is_normal); >> >> -static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1); >> +static atomic_t rcu_async_hurry_nesting = ATOMIC_INIT(1); >> +/* >> + * Should call_rcu() callbacks be processed with urgency or are >> + * they OK being executed with arbitrary delays? >> + */ >> +bool rcu_async_should_hurry(void) >> +{ >> + return !IS_ENABLED(CONFIG_RCU_LAZY) || >> + atomic_read(&rcu_async_hurry_nesting); >> +} >> +EXPORT_SYMBOL_GPL(rcu_async_should_hurry); >> + >> +/** >> + * rcu_async_hurry - Make future async RCU callbacks not lazy. >> + * >> + * After a call to this function, future calls to call_rcu() >> + * will be processed in a timely fashion. >> + */ >> +void rcu_async_hurry(void) >> +{ >> + if (IS_ENABLED(CONFIG_RCU_LAZY)) >> + atomic_inc(&rcu_async_hurry_nesting); >> +} >> +EXPORT_SYMBOL_GPL(rcu_async_hurry); >> >> +/** >> + * rcu_async_relax - Make future async RCU callbacks lazy. >> + * >> + * After a call to this function, future calls to call_rcu() >> + * will be processed in a lazy fashion. >> + */ >> +void rcu_async_relax(void) >> +{ >> + if (IS_ENABLED(CONFIG_RCU_LAZY)) >> + atomic_dec(&rcu_async_hurry_nesting); >> +} >> +EXPORT_SYMBOL_GPL(rcu_async_relax); >> + >> +static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1); >> /* >> * Should normal grace-period primitives be expedited? Intended for >> * use within RCU. Note that this function takes the rcu_expedited >> @@ -195,6 +232,7 @@ static bool rcu_boot_ended __read_mostly; >> void rcu_end_inkernel_boot(void) >> { >> rcu_unexpedite_gp(); >> + rcu_async_relax(); >> if (rcu_normal_after_boot) >> WRITE_ONCE(rcu_normal, 1); >> rcu_boot_ended = true; >> -- >> 2.39.0.314.g84b9a713c41-goog
On Thu, Jan 12, 2023 at 12:52:22AM +0000, Joel Fernandes (Google) wrote: > During boot and suspend/resume, it is desired to prevent RCU laziness from > effecting performance and in some cases failures like with suspend. > > Track whether RCU laziness is to be ignored or not, in kernels with > CONFIG_RCU_LAZY enabled. We do such tracking for expedited-RCU already, however > since Android currently expedites synchronous_rcu() always, we cannot rely on > that. The next patch ignores laziness hints based on this tracking. > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> > --- > Paul, could we take this and the next one for 6.2 -rc cycle? > > I also booted debian Linux and verified the flag is reset correctly after boot > completes. Thanks. After going back and forth a bit, I took these two as-is (aside from the usual commit-log wordsmithing). At some point, the state-change-notification from things like boot, suspend/resume, sysrq-t, and panic() should probably be refactored. But I do not believe that we are yet at that point, and there is not much point in half-refactorings in cases such as this one. I added the Fixes tags, and, if testing goes well, I plan to submit them into the upcoming merge window. I have no reason to believe that anyone is hitting this, it is only a few weeks away anyhow, and a good chunk of that would be consumed by testing and review. And if someone does hit it, we do have your fixes to send to them, so thank you for that! (These won't become visible on -rcu until I complete today's rebase, in case you were wondering.) Thanx, Paul > kernel/rcu/rcu.h | 6 ++++++ > kernel/rcu/tree.c | 2 ++ > kernel/rcu/update.c | 40 +++++++++++++++++++++++++++++++++++++++- > 5 files changed, 55 insertions(+), 1 deletion(-) > create mode 100644 cc_list > create mode 100644 to_list > > diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h > index 5c8013f7085f..115616ac3bfa 100644 > --- a/kernel/rcu/rcu.h > +++ b/kernel/rcu/rcu.h > @@ -449,14 +449,20 @@ do { \ > /* Tiny RCU doesn't expedite, as its purpose in life is instead to be tiny. */ > static inline bool rcu_gp_is_normal(void) { return true; } > static inline bool rcu_gp_is_expedited(void) { return false; } > +static inline bool rcu_async_should_hurry(void) { return false; } > static inline void rcu_expedite_gp(void) { } > static inline void rcu_unexpedite_gp(void) { } > +static inline void rcu_async_hurry(void) { } > +static inline void rcu_async_relax(void) { } > static inline void rcu_request_urgent_qs_task(struct task_struct *t) { } > #else /* #ifdef CONFIG_TINY_RCU */ > bool rcu_gp_is_normal(void); /* Internal RCU use. */ > bool rcu_gp_is_expedited(void); /* Internal RCU use. */ > +bool rcu_async_should_hurry(void); /* Internal RCU use. */ > void rcu_expedite_gp(void); > void rcu_unexpedite_gp(void); > +void rcu_async_hurry(void); > +void rcu_async_relax(void); > void rcupdate_announce_bootup_oddness(void); > #ifdef CONFIG_TASKS_RCU_GENERIC > void show_rcu_tasks_gp_kthreads(void); > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > index 63545d79da51..78b2e999c904 100644 > --- a/kernel/rcu/tree.c > +++ b/kernel/rcu/tree.c > @@ -4504,11 +4504,13 @@ static int rcu_pm_notify(struct notifier_block *self, > switch (action) { > case PM_HIBERNATION_PREPARE: > case PM_SUSPEND_PREPARE: > + rcu_async_hurry(); > rcu_expedite_gp(); > break; > case PM_POST_HIBERNATION: > case PM_POST_SUSPEND: > rcu_unexpedite_gp(); > + rcu_async_relax(); > break; > default: > break; > diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c > index 3893022f8ed8..19bf6fa3ee6a 100644 > --- a/kernel/rcu/update.c > +++ b/kernel/rcu/update.c > @@ -144,8 +144,45 @@ bool rcu_gp_is_normal(void) > } > EXPORT_SYMBOL_GPL(rcu_gp_is_normal); > > -static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1); > +static atomic_t rcu_async_hurry_nesting = ATOMIC_INIT(1); > +/* > + * Should call_rcu() callbacks be processed with urgency or are > + * they OK being executed with arbitrary delays? > + */ > +bool rcu_async_should_hurry(void) > +{ > + return !IS_ENABLED(CONFIG_RCU_LAZY) || > + atomic_read(&rcu_async_hurry_nesting); > +} > +EXPORT_SYMBOL_GPL(rcu_async_should_hurry); > + > +/** > + * rcu_async_hurry - Make future async RCU callbacks not lazy. > + * > + * After a call to this function, future calls to call_rcu() > + * will be processed in a timely fashion. > + */ > +void rcu_async_hurry(void) > +{ > + if (IS_ENABLED(CONFIG_RCU_LAZY)) > + atomic_inc(&rcu_async_hurry_nesting); > +} > +EXPORT_SYMBOL_GPL(rcu_async_hurry); > > +/** > + * rcu_async_relax - Make future async RCU callbacks lazy. > + * > + * After a call to this function, future calls to call_rcu() > + * will be processed in a lazy fashion. > + */ > +void rcu_async_relax(void) > +{ > + if (IS_ENABLED(CONFIG_RCU_LAZY)) > + atomic_dec(&rcu_async_hurry_nesting); > +} > +EXPORT_SYMBOL_GPL(rcu_async_relax); > + > +static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1); > /* > * Should normal grace-period primitives be expedited? Intended for > * use within RCU. Note that this function takes the rcu_expedited > @@ -195,6 +232,7 @@ static bool rcu_boot_ended __read_mostly; > void rcu_end_inkernel_boot(void) > { > rcu_unexpedite_gp(); > + rcu_async_relax(); > if (rcu_normal_after_boot) > WRITE_ONCE(rcu_normal, 1); > rcu_boot_ended = true; > -- > 2.39.0.314.g84b9a713c41-goog
> On Jan 12, 2023, at 2:27 PM, Paul E. McKenney <paulmck@kernel.org> wrote: > > On Thu, Jan 12, 2023 at 12:52:22AM +0000, Joel Fernandes (Google) wrote: >> During boot and suspend/resume, it is desired to prevent RCU laziness from >> effecting performance and in some cases failures like with suspend. >> >> Track whether RCU laziness is to be ignored or not, in kernels with >> CONFIG_RCU_LAZY enabled. We do such tracking for expedited-RCU already, however >> since Android currently expedites synchronous_rcu() always, we cannot rely on >> that. The next patch ignores laziness hints based on this tracking. >> >> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> >> --- >> Paul, could we take this and the next one for 6.2 -rc cycle? >> >> I also booted debian Linux and verified the flag is reset correctly after boot >> completes. Thanks. > > After going back and forth a bit, I took these two as-is (aside from > the usual commit-log wordsmithing). > > At some point, the state-change-notification from things like boot, > suspend/resume, sysrq-t, and panic() should probably be refactored. > But I do not believe that we are yet at that point, and there is not > much point in half-refactorings in cases such as this one. > > I added the Fixes tags, and, if testing goes well, I plan to submit > them into the upcoming merge window. I have no reason to believe that > anyone is hitting this, it is only a few weeks away anyhow, and a good > chunk of that would be consumed by testing and review. > > And if someone does hit it, we do have your fixes to send to them, so > thank you for that! > (These won't become visible on -rcu until I complete today's rebase, > in case you were wondering.) Thanks and this sounds good to me. Meanwhile I pulled into our product kernel so all is good. Thanks, - Joel > > Thanx, Paul > >> kernel/rcu/rcu.h | 6 ++++++ >> kernel/rcu/tree.c | 2 ++ >> kernel/rcu/update.c | 40 +++++++++++++++++++++++++++++++++++++++- >> 5 files changed, 55 insertions(+), 1 deletion(-) >> create mode 100644 cc_list >> create mode 100644 to_list >> >> diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h >> index 5c8013f7085f..115616ac3bfa 100644 >> --- a/kernel/rcu/rcu.h >> +++ b/kernel/rcu/rcu.h >> @@ -449,14 +449,20 @@ do { \ >> /* Tiny RCU doesn't expedite, as its purpose in life is instead to be tiny. */ >> static inline bool rcu_gp_is_normal(void) { return true; } >> static inline bool rcu_gp_is_expedited(void) { return false; } >> +static inline bool rcu_async_should_hurry(void) { return false; } >> static inline void rcu_expedite_gp(void) { } >> static inline void rcu_unexpedite_gp(void) { } >> +static inline void rcu_async_hurry(void) { } >> +static inline void rcu_async_relax(void) { } >> static inline void rcu_request_urgent_qs_task(struct task_struct *t) { } >> #else /* #ifdef CONFIG_TINY_RCU */ >> bool rcu_gp_is_normal(void); /* Internal RCU use. */ >> bool rcu_gp_is_expedited(void); /* Internal RCU use. */ >> +bool rcu_async_should_hurry(void); /* Internal RCU use. */ >> void rcu_expedite_gp(void); >> void rcu_unexpedite_gp(void); >> +void rcu_async_hurry(void); >> +void rcu_async_relax(void); >> void rcupdate_announce_bootup_oddness(void); >> #ifdef CONFIG_TASKS_RCU_GENERIC >> void show_rcu_tasks_gp_kthreads(void); >> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c >> index 63545d79da51..78b2e999c904 100644 >> --- a/kernel/rcu/tree.c >> +++ b/kernel/rcu/tree.c >> @@ -4504,11 +4504,13 @@ static int rcu_pm_notify(struct notifier_block *self, >> switch (action) { >> case PM_HIBERNATION_PREPARE: >> case PM_SUSPEND_PREPARE: >> + rcu_async_hurry(); >> rcu_expedite_gp(); >> break; >> case PM_POST_HIBERNATION: >> case PM_POST_SUSPEND: >> rcu_unexpedite_gp(); >> + rcu_async_relax(); >> break; >> default: >> break; >> diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c >> index 3893022f8ed8..19bf6fa3ee6a 100644 >> --- a/kernel/rcu/update.c >> +++ b/kernel/rcu/update.c >> @@ -144,8 +144,45 @@ bool rcu_gp_is_normal(void) >> } >> EXPORT_SYMBOL_GPL(rcu_gp_is_normal); >> >> -static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1); >> +static atomic_t rcu_async_hurry_nesting = ATOMIC_INIT(1); >> +/* >> + * Should call_rcu() callbacks be processed with urgency or are >> + * they OK being executed with arbitrary delays? >> + */ >> +bool rcu_async_should_hurry(void) >> +{ >> + return !IS_ENABLED(CONFIG_RCU_LAZY) || >> + atomic_read(&rcu_async_hurry_nesting); >> +} >> +EXPORT_SYMBOL_GPL(rcu_async_should_hurry); >> + >> +/** >> + * rcu_async_hurry - Make future async RCU callbacks not lazy. >> + * >> + * After a call to this function, future calls to call_rcu() >> + * will be processed in a timely fashion. >> + */ >> +void rcu_async_hurry(void) >> +{ >> + if (IS_ENABLED(CONFIG_RCU_LAZY)) >> + atomic_inc(&rcu_async_hurry_nesting); >> +} >> +EXPORT_SYMBOL_GPL(rcu_async_hurry); >> >> +/** >> + * rcu_async_relax - Make future async RCU callbacks lazy. >> + * >> + * After a call to this function, future calls to call_rcu() >> + * will be processed in a lazy fashion. >> + */ >> +void rcu_async_relax(void) >> +{ >> + if (IS_ENABLED(CONFIG_RCU_LAZY)) >> + atomic_dec(&rcu_async_hurry_nesting); >> +} >> +EXPORT_SYMBOL_GPL(rcu_async_relax); >> + >> +static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1); >> /* >> * Should normal grace-period primitives be expedited? Intended for >> * use within RCU. Note that this function takes the rcu_expedited >> @@ -195,6 +232,7 @@ static bool rcu_boot_ended __read_mostly; >> void rcu_end_inkernel_boot(void) >> { >> rcu_unexpedite_gp(); >> + rcu_async_relax(); >> if (rcu_normal_after_boot) >> WRITE_ONCE(rcu_normal, 1); >> rcu_boot_ended = true; >> -- >> 2.39.0.314.g84b9a713c41-goog
On Thu, Jan 12, 2023 at 05:25:51PM -0500, Joel Fernandes wrote: > > > > On Jan 12, 2023, at 2:27 PM, Paul E. McKenney <paulmck@kernel.org> wrote: > > > > On Thu, Jan 12, 2023 at 12:52:22AM +0000, Joel Fernandes (Google) wrote: > >> During boot and suspend/resume, it is desired to prevent RCU laziness from > >> effecting performance and in some cases failures like with suspend. > >> > >> Track whether RCU laziness is to be ignored or not, in kernels with > >> CONFIG_RCU_LAZY enabled. We do such tracking for expedited-RCU already, however > >> since Android currently expedites synchronous_rcu() always, we cannot rely on > >> that. The next patch ignores laziness hints based on this tracking. > >> > >> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> > >> --- > >> Paul, could we take this and the next one for 6.2 -rc cycle? > >> > >> I also booted debian Linux and verified the flag is reset correctly after boot > >> completes. Thanks. > > > > After going back and forth a bit, I took these two as-is (aside from > > the usual commit-log wordsmithing). > > > > At some point, the state-change-notification from things like boot, > > suspend/resume, sysrq-t, and panic() should probably be refactored. > > But I do not believe that we are yet at that point, and there is not > > much point in half-refactorings in cases such as this one. > > > > I added the Fixes tags, and, if testing goes well, I plan to submit > > them into the upcoming merge window. I have no reason to believe that > > anyone is hitting this, it is only a few weeks away anyhow, and a good > > chunk of that would be consumed by testing and review. > > > > And if someone does hit it, we do have your fixes to send to them, so > > thank you for that! > > (These won't become visible on -rcu until I complete today's rebase, > > in case you were wondering.) > > Thanks and this sounds good to me. Meanwhile I pulled into our product kernel so all is good. Very good! Please let me know how it goes! Thanx, Paul > Thanks, > > - Joel > > > > > > Thanx, Paul > > > >> kernel/rcu/rcu.h | 6 ++++++ > >> kernel/rcu/tree.c | 2 ++ > >> kernel/rcu/update.c | 40 +++++++++++++++++++++++++++++++++++++++- > >> 5 files changed, 55 insertions(+), 1 deletion(-) > >> create mode 100644 cc_list > >> create mode 100644 to_list > >> > >> diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h > >> index 5c8013f7085f..115616ac3bfa 100644 > >> --- a/kernel/rcu/rcu.h > >> +++ b/kernel/rcu/rcu.h > >> @@ -449,14 +449,20 @@ do { \ > >> /* Tiny RCU doesn't expedite, as its purpose in life is instead to be tiny. */ > >> static inline bool rcu_gp_is_normal(void) { return true; } > >> static inline bool rcu_gp_is_expedited(void) { return false; } > >> +static inline bool rcu_async_should_hurry(void) { return false; } > >> static inline void rcu_expedite_gp(void) { } > >> static inline void rcu_unexpedite_gp(void) { } > >> +static inline void rcu_async_hurry(void) { } > >> +static inline void rcu_async_relax(void) { } > >> static inline void rcu_request_urgent_qs_task(struct task_struct *t) { } > >> #else /* #ifdef CONFIG_TINY_RCU */ > >> bool rcu_gp_is_normal(void); /* Internal RCU use. */ > >> bool rcu_gp_is_expedited(void); /* Internal RCU use. */ > >> +bool rcu_async_should_hurry(void); /* Internal RCU use. */ > >> void rcu_expedite_gp(void); > >> void rcu_unexpedite_gp(void); > >> +void rcu_async_hurry(void); > >> +void rcu_async_relax(void); > >> void rcupdate_announce_bootup_oddness(void); > >> #ifdef CONFIG_TASKS_RCU_GENERIC > >> void show_rcu_tasks_gp_kthreads(void); > >> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c > >> index 63545d79da51..78b2e999c904 100644 > >> --- a/kernel/rcu/tree.c > >> +++ b/kernel/rcu/tree.c > >> @@ -4504,11 +4504,13 @@ static int rcu_pm_notify(struct notifier_block *self, > >> switch (action) { > >> case PM_HIBERNATION_PREPARE: > >> case PM_SUSPEND_PREPARE: > >> + rcu_async_hurry(); > >> rcu_expedite_gp(); > >> break; > >> case PM_POST_HIBERNATION: > >> case PM_POST_SUSPEND: > >> rcu_unexpedite_gp(); > >> + rcu_async_relax(); > >> break; > >> default: > >> break; > >> diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c > >> index 3893022f8ed8..19bf6fa3ee6a 100644 > >> --- a/kernel/rcu/update.c > >> +++ b/kernel/rcu/update.c > >> @@ -144,8 +144,45 @@ bool rcu_gp_is_normal(void) > >> } > >> EXPORT_SYMBOL_GPL(rcu_gp_is_normal); > >> > >> -static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1); > >> +static atomic_t rcu_async_hurry_nesting = ATOMIC_INIT(1); > >> +/* > >> + * Should call_rcu() callbacks be processed with urgency or are > >> + * they OK being executed with arbitrary delays? > >> + */ > >> +bool rcu_async_should_hurry(void) > >> +{ > >> + return !IS_ENABLED(CONFIG_RCU_LAZY) || > >> + atomic_read(&rcu_async_hurry_nesting); > >> +} > >> +EXPORT_SYMBOL_GPL(rcu_async_should_hurry); > >> + > >> +/** > >> + * rcu_async_hurry - Make future async RCU callbacks not lazy. > >> + * > >> + * After a call to this function, future calls to call_rcu() > >> + * will be processed in a timely fashion. > >> + */ > >> +void rcu_async_hurry(void) > >> +{ > >> + if (IS_ENABLED(CONFIG_RCU_LAZY)) > >> + atomic_inc(&rcu_async_hurry_nesting); > >> +} > >> +EXPORT_SYMBOL_GPL(rcu_async_hurry); > >> > >> +/** > >> + * rcu_async_relax - Make future async RCU callbacks lazy. > >> + * > >> + * After a call to this function, future calls to call_rcu() > >> + * will be processed in a lazy fashion. > >> + */ > >> +void rcu_async_relax(void) > >> +{ > >> + if (IS_ENABLED(CONFIG_RCU_LAZY)) > >> + atomic_dec(&rcu_async_hurry_nesting); > >> +} > >> +EXPORT_SYMBOL_GPL(rcu_async_relax); > >> + > >> +static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1); > >> /* > >> * Should normal grace-period primitives be expedited? Intended for > >> * use within RCU. Note that this function takes the rcu_expedited > >> @@ -195,6 +232,7 @@ static bool rcu_boot_ended __read_mostly; > >> void rcu_end_inkernel_boot(void) > >> { > >> rcu_unexpedite_gp(); > >> + rcu_async_relax(); > >> if (rcu_normal_after_boot) > >> WRITE_ONCE(rcu_normal, 1); > >> rcu_boot_ended = true; > >> -- > >> 2.39.0.314.g84b9a713c41-goog
On Thu, 12 Jan 2023 00:52:22 +0000 "Joel Fernandes (Google)" <joel@joelfernandes.org> wrote: > -- a/kernel/rcu/update.c > +++ b/kernel/rcu/update.c > @@ -144,8 +144,45 @@ bool rcu_gp_is_normal(void) > } > EXPORT_SYMBOL_GPL(rcu_gp_is_normal); > > -static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1); > +static atomic_t rcu_async_hurry_nesting = ATOMIC_INIT(1); > +/* > + * Should call_rcu() callbacks be processed with urgency or are > + * they OK being executed with arbitrary delays? > + */ > +bool rcu_async_should_hurry(void) > +{ > + return !IS_ENABLED(CONFIG_RCU_LAZY) || > + atomic_read(&rcu_async_hurry_nesting); > +} > +EXPORT_SYMBOL_GPL(rcu_async_should_hurry); > + > +/** > + * rcu_async_hurry - Make future async RCU callbacks not lazy. > + * > + * After a call to this function, future calls to call_rcu() > + * will be processed in a timely fashion. > + */ > +void rcu_async_hurry(void) > +{ > + if (IS_ENABLED(CONFIG_RCU_LAZY)) > + atomic_inc(&rcu_async_hurry_nesting); > +} > +EXPORT_SYMBOL_GPL(rcu_async_hurry); > Where do you plan on calling these externally, as they are being marked exported? If you allow random drivers to enable this, I can see something enabling it and hitting an error path that causes it to never disable it. I wouldn't have EXPORT_SYMBOL_GPL() unless you really know that it is needed externally. In fact, is this really needed outside of the RCU subsystem? -- Steve > +/** > + * rcu_async_relax - Make future async RCU callbacks lazy. > + * > + * After a call to this function, future calls to call_rcu() > + * will be processed in a lazy fashion. > + */ > +void rcu_async_relax(void) > +{ > + if (IS_ENABLED(CONFIG_RCU_LAZY)) > + atomic_dec(&rcu_async_hurry_nesting); > +} > +EXPORT_SYMBOL_GPL(rcu_async_relax); > + > +static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1);
On Sun, Jan 15, 2023 at 4:25 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Thu, 12 Jan 2023 00:52:22 +0000 > "Joel Fernandes (Google)" <joel@joelfernandes.org> wrote: > > > -- a/kernel/rcu/update.c > > +++ b/kernel/rcu/update.c > > @@ -144,8 +144,45 @@ bool rcu_gp_is_normal(void) > > } > > EXPORT_SYMBOL_GPL(rcu_gp_is_normal); > > > > -static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1); > > +static atomic_t rcu_async_hurry_nesting = ATOMIC_INIT(1); > > +/* > > + * Should call_rcu() callbacks be processed with urgency or are > > + * they OK being executed with arbitrary delays? > > + */ > > +bool rcu_async_should_hurry(void) > > +{ > > + return !IS_ENABLED(CONFIG_RCU_LAZY) || > > + atomic_read(&rcu_async_hurry_nesting); > > +} > > +EXPORT_SYMBOL_GPL(rcu_async_should_hurry); > > + > > +/** > > + * rcu_async_hurry - Make future async RCU callbacks not lazy. > > + * > > + * After a call to this function, future calls to call_rcu() > > + * will be processed in a timely fashion. > > + */ > > +void rcu_async_hurry(void) > > +{ > > + if (IS_ENABLED(CONFIG_RCU_LAZY)) > > + atomic_inc(&rcu_async_hurry_nesting); > > +} > > +EXPORT_SYMBOL_GPL(rcu_async_hurry); > > > > Where do you plan on calling these externally, as they are being > marked exported? > > If you allow random drivers to enable this, I can see something > enabling it and hitting an error path that causes it to never disable > it. You mean, just like rcu_expedite_gp() ? > I wouldn't have EXPORT_SYMBOL_GPL() unless you really know that it is > needed externally. At the moment it is not called externally but in the future, it could be from rcutorture. If you see rcu_expedite_gp(), that is exported too. I was just modeling it around that API. thanks, - Joel > > -- Steve > > > > +/** > > + * rcu_async_relax - Make future async RCU callbacks lazy. > > + * > > + * After a call to this function, future calls to call_rcu() > > + * will be processed in a lazy fashion. > > + */ > > +void rcu_async_relax(void) > > +{ > > + if (IS_ENABLED(CONFIG_RCU_LAZY)) > > + atomic_dec(&rcu_async_hurry_nesting); > > +} > > +EXPORT_SYMBOL_GPL(rcu_async_relax); > > + > > +static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1);
On Sun, Jan 15, 2023 at 04:34:58PM -0500, Joel Fernandes wrote: > On Sun, Jan 15, 2023 at 4:25 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > > > On Thu, 12 Jan 2023 00:52:22 +0000 > > "Joel Fernandes (Google)" <joel@joelfernandes.org> wrote: > > > > > -- a/kernel/rcu/update.c > > > +++ b/kernel/rcu/update.c > > > @@ -144,8 +144,45 @@ bool rcu_gp_is_normal(void) > > > } > > > EXPORT_SYMBOL_GPL(rcu_gp_is_normal); > > > > > > -static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1); > > > +static atomic_t rcu_async_hurry_nesting = ATOMIC_INIT(1); > > > +/* > > > + * Should call_rcu() callbacks be processed with urgency or are > > > + * they OK being executed with arbitrary delays? > > > + */ > > > +bool rcu_async_should_hurry(void) > > > +{ > > > + return !IS_ENABLED(CONFIG_RCU_LAZY) || > > > + atomic_read(&rcu_async_hurry_nesting); > > > +} > > > +EXPORT_SYMBOL_GPL(rcu_async_should_hurry); > > > + > > > +/** > > > + * rcu_async_hurry - Make future async RCU callbacks not lazy. > > > + * > > > + * After a call to this function, future calls to call_rcu() > > > + * will be processed in a timely fashion. > > > + */ > > > +void rcu_async_hurry(void) > > > +{ > > > + if (IS_ENABLED(CONFIG_RCU_LAZY)) > > > + atomic_inc(&rcu_async_hurry_nesting); > > > +} > > > +EXPORT_SYMBOL_GPL(rcu_async_hurry); > > > > > > > Where do you plan on calling these externally, as they are being > > marked exported? > > > > If you allow random drivers to enable this, I can see something > > enabling it and hitting an error path that causes it to never disable > > it. > > You mean, just like rcu_expedite_gp() ? > > > I wouldn't have EXPORT_SYMBOL_GPL() unless you really know that it is > > needed externally. > > At the moment it is not called externally but in the future, it could > be from rcutorture. If you see rcu_expedite_gp(), that is exported > too. I was just modeling it around that API. It really should be invoked from rcutorture for testing purposes. In current -rcu, TREE01 enables LAZY_RCU, so we are finally getting coverage (aside from my manual enablement on part of the test grid that I use). So we need that export. On the other hand, wasn't there some talk recently of targeted exports? If that comes to pass, this could be exported to only rcutorture and rcuscale. Thanx, Paul > thanks, > > - Joel > > > > > -- Steve > > > > > > > +/** > > > + * rcu_async_relax - Make future async RCU callbacks lazy. > > > + * > > > + * After a call to this function, future calls to call_rcu() > > > + * will be processed in a lazy fashion. > > > + */ > > > +void rcu_async_relax(void) > > > +{ > > > + if (IS_ENABLED(CONFIG_RCU_LAZY)) > > > + atomic_dec(&rcu_async_hurry_nesting); > > > +} > > > +EXPORT_SYMBOL_GPL(rcu_async_relax); > > > + > > > +static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1);
On Sun, 15 Jan 2023 16:34:58 -0500 Joel Fernandes <joel@joelfernandes.org> wrote: > > > +EXPORT_SYMBOL_GPL(rcu_async_hurry); > > > > > > > Where do you plan on calling these externally, as they are being > > marked exported? > > > > If you allow random drivers to enable this, I can see something > > enabling it and hitting an error path that causes it to never disable > > it. > > You mean, just like rcu_expedite_gp() ? > > > I wouldn't have EXPORT_SYMBOL_GPL() unless you really know that it is > > needed externally. > > At the moment it is not called externally but in the future, it could > be from rcutorture. If you see rcu_expedite_gp(), that is exported > too. I was just modeling it around that API. The reason for the export should have been mentioned in the change log if the patch is not obvious to why it is being exported. Thanks, -- Steve
On Tue, Jan 17, 2023 at 02:32:24PM -0500, Steven Rostedt wrote: > On Sun, 15 Jan 2023 16:34:58 -0500 > Joel Fernandes <joel@joelfernandes.org> wrote: > > > > > +EXPORT_SYMBOL_GPL(rcu_async_hurry); > > > > > > > > > > Where do you plan on calling these externally, as they are being > > > marked exported? > > > > > > If you allow random drivers to enable this, I can see something > > > enabling it and hitting an error path that causes it to never disable > > > it. > > > > You mean, just like rcu_expedite_gp() ? > > > > > I wouldn't have EXPORT_SYMBOL_GPL() unless you really know that it is > > > needed externally. > > > > At the moment it is not called externally but in the future, it could > > be from rcutorture. If you see rcu_expedite_gp(), that is exported > > too. I was just modeling it around that API. > > The reason for the export should have been mentioned in the change log if > the patch is not obvious to why it is being exported. Would something like this suffice? With attribution, of course. Export rcu_async_should_hurry(), rcu_async_hurry(), and rcu_async_relax() for later use by rcutorture. Thanx, Paul
On Tue, Jan 17, 2023 at 7:37 PM Paul E. McKenney <paulmck@kernel.org> wrote: > > On Tue, Jan 17, 2023 at 02:32:24PM -0500, Steven Rostedt wrote: > > On Sun, 15 Jan 2023 16:34:58 -0500 > > Joel Fernandes <joel@joelfernandes.org> wrote: > > > > > > > +EXPORT_SYMBOL_GPL(rcu_async_hurry); > > > > > > > > > > > > > Where do you plan on calling these externally, as they are being > > > > marked exported? > > > > > > > > If you allow random drivers to enable this, I can see something > > > > enabling it and hitting an error path that causes it to never disable > > > > it. > > > > > > You mean, just like rcu_expedite_gp() ? > > > > > > > I wouldn't have EXPORT_SYMBOL_GPL() unless you really know that it is > > > > needed externally. > > > > > > At the moment it is not called externally but in the future, it could > > > be from rcutorture. If you see rcu_expedite_gp(), that is exported > > > too. I was just modeling it around that API. > > > > The reason for the export should have been mentioned in the change log if > > the patch is not obvious to why it is being exported. > > Would something like this suffice? With attribution, of course. > > Export rcu_async_should_hurry(), rcu_async_hurry(), and > rcu_async_relax() for later use by rcutorture. Looks good to me Paul, and thanks for the suggestion Steven! - Joel
On Tue, 17 Jan 2023 11:37:34 -0800 "Paul E. McKenney" <paulmck@kernel.org> wrote: > > The reason for the export should have been mentioned in the change log if > > the patch is not obvious to why it is being exported. > > Would something like this suffice? With attribution, of course. > > Export rcu_async_should_hurry(), rcu_async_hurry(), and > rcu_async_relax() for later use by rcutorture. Yes thanks. That way, at least a git blame will give some rationale for the export. -- Steve
On Tue, Jan 17, 2023 at 04:40:29PM -0500, Steven Rostedt wrote: > On Tue, 17 Jan 2023 11:37:34 -0800 > "Paul E. McKenney" <paulmck@kernel.org> wrote: > > > > The reason for the export should have been mentioned in the change log if > > > the patch is not obvious to why it is being exported. > > > > Would something like this suffice? With attribution, of course. > > > > Export rcu_async_should_hurry(), rcu_async_hurry(), and > > rcu_async_relax() for later use by rcutorture. > > Yes thanks. That way, at least a git blame will give some rationale for the > export. Very well, I will apply this on my next rebase. Thanx, Paul
diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h index 5c8013f7085f..115616ac3bfa 100644 --- a/kernel/rcu/rcu.h +++ b/kernel/rcu/rcu.h @@ -449,14 +449,20 @@ do { \ /* Tiny RCU doesn't expedite, as its purpose in life is instead to be tiny. */ static inline bool rcu_gp_is_normal(void) { return true; } static inline bool rcu_gp_is_expedited(void) { return false; } +static inline bool rcu_async_should_hurry(void) { return false; } static inline void rcu_expedite_gp(void) { } static inline void rcu_unexpedite_gp(void) { } +static inline void rcu_async_hurry(void) { } +static inline void rcu_async_relax(void) { } static inline void rcu_request_urgent_qs_task(struct task_struct *t) { } #else /* #ifdef CONFIG_TINY_RCU */ bool rcu_gp_is_normal(void); /* Internal RCU use. */ bool rcu_gp_is_expedited(void); /* Internal RCU use. */ +bool rcu_async_should_hurry(void); /* Internal RCU use. */ void rcu_expedite_gp(void); void rcu_unexpedite_gp(void); +void rcu_async_hurry(void); +void rcu_async_relax(void); void rcupdate_announce_bootup_oddness(void); #ifdef CONFIG_TASKS_RCU_GENERIC void show_rcu_tasks_gp_kthreads(void); diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c index 63545d79da51..78b2e999c904 100644 --- a/kernel/rcu/tree.c +++ b/kernel/rcu/tree.c @@ -4504,11 +4504,13 @@ static int rcu_pm_notify(struct notifier_block *self, switch (action) { case PM_HIBERNATION_PREPARE: case PM_SUSPEND_PREPARE: + rcu_async_hurry(); rcu_expedite_gp(); break; case PM_POST_HIBERNATION: case PM_POST_SUSPEND: rcu_unexpedite_gp(); + rcu_async_relax(); break; default: break; diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c index 3893022f8ed8..19bf6fa3ee6a 100644 --- a/kernel/rcu/update.c +++ b/kernel/rcu/update.c @@ -144,8 +144,45 @@ bool rcu_gp_is_normal(void) } EXPORT_SYMBOL_GPL(rcu_gp_is_normal); -static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1); +static atomic_t rcu_async_hurry_nesting = ATOMIC_INIT(1); +/* + * Should call_rcu() callbacks be processed with urgency or are + * they OK being executed with arbitrary delays? + */ +bool rcu_async_should_hurry(void) +{ + return !IS_ENABLED(CONFIG_RCU_LAZY) || + atomic_read(&rcu_async_hurry_nesting); +} +EXPORT_SYMBOL_GPL(rcu_async_should_hurry); + +/** + * rcu_async_hurry - Make future async RCU callbacks not lazy. + * + * After a call to this function, future calls to call_rcu() + * will be processed in a timely fashion. + */ +void rcu_async_hurry(void) +{ + if (IS_ENABLED(CONFIG_RCU_LAZY)) + atomic_inc(&rcu_async_hurry_nesting); +} +EXPORT_SYMBOL_GPL(rcu_async_hurry); +/** + * rcu_async_relax - Make future async RCU callbacks lazy. + * + * After a call to this function, future calls to call_rcu() + * will be processed in a lazy fashion. + */ +void rcu_async_relax(void) +{ + if (IS_ENABLED(CONFIG_RCU_LAZY)) + atomic_dec(&rcu_async_hurry_nesting); +} +EXPORT_SYMBOL_GPL(rcu_async_relax); + +static atomic_t rcu_expedited_nesting = ATOMIC_INIT(1); /* * Should normal grace-period primitives be expedited? Intended for * use within RCU. Note that this function takes the rcu_expedited @@ -195,6 +232,7 @@ static bool rcu_boot_ended __read_mostly; void rcu_end_inkernel_boot(void) { rcu_unexpedite_gp(); + rcu_async_relax(); if (rcu_normal_after_boot) WRITE_ONCE(rcu_normal, 1); rcu_boot_ended = true;