From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Joel Fernandes <joel@joelfernandes.org>
Cc: linux-kernel@vger.kernel.org,
Josh Triplett <josh@joshtriplett.org>,
Steven Rostedt <rostedt@goodmis.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Lai Jiangshan <jiangshanlai@gmail.com>,
tglx@linutronix.de,
"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
Mike Galbraith <bitbucket@online.de>,
rcu@vger.kernel.org
Subject: Re: [PATCH] rcu: Allow to eliminate softirq processing from rcutree
Date: Wed, 20 Mar 2019 12:28:35 +0100 [thread overview]
Message-ID: <20190320112835.prq22vsto3ecckff@linutronix.de> (raw)
In-Reply-To: <20190320002613.GA129907@google.com>
On 2019-03-19 20:26:13 [-0400], Joel Fernandes wrote:
> > @@ -2769,19 +2782,121 @@ static void invoke_rcu_callbacks(struct rcu_data *rdp)
> > {
> > if (unlikely(!READ_ONCE(rcu_scheduler_fully_active)))
> > return;
> > - if (likely(!rcu_state.boost)) {
> > - rcu_do_batch(rdp);
> > - return;
> > - }
> > - invoke_rcu_callbacks_kthread();
> > + rcu_do_batch(rdp);
>
> Looks like a nice change, but one question...
>
> Consider the case where rcunosoftirq boot option is not passed.
>
> Before, if RCU_BOOST=y, then callbacks would be invoked in rcuc threads if
> possible, by those threads being woken up from within the softirq context
> (in invoke_rcu_callbacks).
>
> Now, if RCU_BOOST=y, then callbacks would only be invoked in softirq context
> and not in the threads at all. Because rcu_softirq_enabled = false, so the
> path executes:
> rcu_read_unlock_special() ->
> raise_softirq_irqsoff() ->
> rcu_process_callbacks_si() ->
> rcu_process_callbacks() ->
> invoke_rcu_callbacks() ->
> rcu_do_batch()
>
> This seems like a behavioral change to me. This makes the callbacks always
> execute from the softirq context and not the threads when boosting is
> configured. IMO in the very least, such behavioral change should be
> documented in the change.
>
> One way to fix this I think could be, if boosting is enabled, then set
> rcu_softirq_enabled to false by default so the callbacks are still executed
> in the rcuc threads.
>
> Did I miss something? Sorry if I did, thanks!
So with all the swaps and reorder we talking about this change:
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 0a719f726e149..82810483bfc6c 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2306,20 +2306,6 @@ static void rcu_core_si(struct softirq_action *h)
rcu_core();
}
-/*
- * Schedule RCU callback invocation. If the running implementation of RCU
- * does not support RCU priority boosting, just do a direct call, otherwise
- * wake up the per-CPU kernel kthread. Note that because we are running
- * on the current CPU with softirqs disabled, the rcu_cpu_kthread_task
- * cannot disappear out from under us.
- */
-static void invoke_rcu_callbacks(struct rcu_data *rdp)
-{
- if (unlikely(!READ_ONCE(rcu_scheduler_fully_active)))
- return;
- rcu_do_batch(rdp);
-}
-
static void rcu_wake_cond(struct task_struct *t, int status)
{
/*
@@ -2330,6 +2316,19 @@ static void rcu_wake_cond(struct task_struct *t, int status)
wake_up_process(t);
}
+static void invoke_rcu_core_kthread(void)
+{
+ struct task_struct *t;
+ unsigned long flags;
+
+ local_irq_save(flags);
+ __this_cpu_write(rcu_data.rcu_cpu_has_work, 1);
+ t = __this_cpu_read(rcu_data.rcu_cpu_kthread_task);
+ if (t != NULL && t != current)
+ rcu_wake_cond(t, __this_cpu_read(rcu_data.rcu_cpu_kthread_status));
+ local_irq_restore(flags);
+}
+
static bool rcu_softirq_enabled = true;
static int __init rcunosoftirq_setup(char *str)
@@ -2339,26 +2338,33 @@ static int __init rcunosoftirq_setup(char *str)
}
__setup("rcunosoftirq", rcunosoftirq_setup);
+/*
+ * Schedule RCU callback invocation. If the running implementation of RCU
+ * does not support RCU priority boosting, just do a direct call, otherwise
+ * wake up the per-CPU kernel kthread. Note that because we are running
+ * on the current CPU with softirqs disabled, the rcu_cpu_kthread_task
+ * cannot disappear out from under us.
+ */
+static void invoke_rcu_callbacks(struct rcu_data *rdp)
+{
+ if (unlikely(!READ_ONCE(rcu_scheduler_fully_active)))
+ return;
+ if (rcu_state.boost || rcu_softirq_enabled)
+ invoke_rcu_core_kthread();
+ rcu_do_batch(rdp);
+}
+
/*
* Wake up this CPU's rcuc kthread to do RCU core processing.
*/
static void invoke_rcu_core(void)
{
- unsigned long flags;
- struct task_struct *t;
-
if (!cpu_online(smp_processor_id()))
return;
- if (rcu_softirq_enabled) {
+ if (rcu_softirq_enabled)
raise_softirq(RCU_SOFTIRQ);
- } else {
- local_irq_save(flags);
- __this_cpu_write(rcu_data.rcu_cpu_has_work, 1);
- t = __this_cpu_read(rcu_data.rcu_cpu_kthread_task);
- if (t != NULL && t != current)
- rcu_wake_cond(t, __this_cpu_read(rcu_data.rcu_cpu_kthread_status));
- local_irq_restore(flags);
- }
+ else
+ invoke_rcu_core_kthread();
}
static void rcu_cpu_kthread_park(unsigned int cpu)
@@ -2426,7 +2432,8 @@ static int __init rcu_spawn_core_kthreads(void)
per_cpu(rcu_data.rcu_cpu_has_work, cpu) = 0;
if (!IS_ENABLED(CONFIG_RCU_BOOST) && !rcu_softirq_enabled)
return 0;
- WARN_ONCE(smpboot_register_percpu_thread(&rcu_cpu_thread_spec), "%s: Could not start rcub kthread, OOM is now expected behavior\n", __func__);
+ WARN_ONCE(smpboot_register_percpu_thread(&rcu_cpu_thread_spec),
+ "%s: Could not start rcuc kthread, OOM is now expected behavior\n", __func__);
return 0;
}
early_initcall(rcu_spawn_core_kthreads);
--
2.20.1
> - Joel
Sebastian
next prev parent reply other threads:[~2019-03-20 11:28 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-15 11:11 [PATCH] rcu: Allow to eliminate softirq processing from rcutree Sebastian Andrzej Siewior
2019-03-15 13:35 ` Steven Rostedt
2019-03-15 13:57 ` Sebastian Andrzej Siewior
2019-03-18 2:24 ` Paul E. McKenney
2019-03-19 11:44 ` [PATCH v2] " Sebastian Andrzej Siewior
2019-03-19 15:59 ` Paul E. McKenney
2019-03-19 16:24 ` Sebastian Andrzej Siewior
2019-03-19 16:50 ` Paul E. McKenney
2019-03-19 17:02 ` Sebastian Andrzej Siewior
2019-03-20 11:32 ` Sebastian Andrzej Siewior
2019-03-20 15:21 ` Paul E. McKenney
2019-03-20 15:44 ` Paul E. McKenney
2019-03-20 16:05 ` Sebastian Andrzej Siewior
2019-03-20 16:15 ` Paul E. McKenney
2019-03-20 16:35 ` Sebastian Andrzej Siewior
2019-03-20 17:30 ` Paul E. McKenney
2019-03-20 17:59 ` Sebastian Andrzej Siewior
2019-03-20 18:12 ` Paul E. McKenney
2019-03-20 18:14 ` Sebastian Andrzej Siewior
2019-03-20 21:13 ` [PATCH v3] " Sebastian Andrzej Siewior
2019-03-20 23:46 ` Paul E. McKenney
2019-03-21 8:27 ` Sebastian Andrzej Siewior
2019-03-21 13:26 ` Paul E. McKenney
2019-03-21 23:32 ` Paul E. McKenney
2019-03-22 7:35 ` Paul E. McKenney
2019-03-22 12:43 ` Paul E. McKenney
2019-03-22 13:42 ` Joel Fernandes
2019-03-22 14:58 ` Paul E. McKenney
2019-03-22 15:50 ` Joel Fernandes
2019-03-22 16:26 ` Paul E. McKenney
2019-03-22 18:07 ` Paul E. McKenney
2019-03-22 23:48 ` Joel Fernandes
2019-03-23 0:25 ` Paul E. McKenney
2019-03-23 1:04 ` Joel Fernandes
2019-03-23 16:10 ` Paul E. McKenney
2019-03-24 23:42 ` Paul E. McKenney
2019-03-25 13:41 ` Joel Fernandes
2019-03-25 15:08 ` Paul E. McKenney
2019-03-25 15:52 ` Paul E. McKenney
2019-03-20 0:26 ` [PATCH] " Joel Fernandes
2019-03-20 11:28 ` Sebastian Andrzej Siewior [this message]
2019-03-21 12:06 ` Joel Fernandes
2019-03-21 13:52 ` Paul E. McKenney
2019-03-20 15:24 ` Paul E. McKenney
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190320112835.prq22vsto3ecckff@linutronix.de \
--to=bigeasy@linutronix.de \
--cc=bitbucket@online.de \
--cc=jiangshanlai@gmail.com \
--cc=joel@joelfernandes.org \
--cc=josh@joshtriplett.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=paulmck@linux.vnet.ibm.com \
--cc=rcu@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).