mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@kernel.org>
To: rcu@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, kernel-team@meta.com,
	rostedt@goodmis.org, Zqiang <qiang1.zhang@intel.com>,
	"Paul E . McKenney" <paulmck@kernel.org>
Subject: [PATCH rcu 5/6] rcu-tasks: Make rude RCU-Tasks work well with CPU hotplug
Date: Wed,  4 Jan 2023 16:44:58 -0800	[thread overview]
Message-ID: <20230105004501.1771332-10-paulmck@kernel.org> (raw)
In-Reply-To: <20230105004454.GA1771168@paulmck-ThinkPad-P17-Gen-1>

From: Zqiang <qiang1.zhang@intel.com>

The synchronize_rcu_tasks_rude() function invokes rcu_tasks_rude_wait_gp()
to wait one rude RCU-tasks grace period.  The rcu_tasks_rude_wait_gp()
function in turn checks if there is only a single online CPU.  If so, it
will immediately return, because a call to synchronize_rcu_tasks_rude()
is by definition a grace period on a single-CPU system.  (We could
have blocked!)

Unfortunately, this check uses num_online_cpus() without synchronization,
which can result in too-short grace periods.  To see this, consider the
following scenario:

        CPU0                                   CPU1 (going offline)
                                          migration/1 task:
                                      cpu_stopper_thread
                                       -> take_cpu_down
                                          -> _cpu_disable
                                           (dec __num_online_cpus)
                                          ->cpuhp_invoke_callback
                                                preempt_disable
                                                access old_data0
           task1
 del old_data0                                  .....
 synchronize_rcu_tasks_rude()
 task1 schedule out
 ....
 task2 schedule in
 rcu_tasks_rude_wait_gp()
     ->__num_online_cpus == 1
       ->return
 ....
 task1 schedule in
 ->free old_data0
                                                preempt_enable

When CPU1 decrements __num_online_cpus, its value becomes 1.  However,
CPU1 has not finished going offline, and will take one last trip through
the scheduler and the idle loop before it actually stops executing
instructions.  Because synchronize_rcu_tasks_rude() is mostly used for
tracing, and because both the scheduler and the idle loop can be traced,
this means that CPU0's prematurely ended grace period might disrupt the
tracing on CPU1.  Given that this disruption might include CPU1 executing
instructions in memory that was just now freed (and maybe reallocated),
this is a matter of some concern.

This commit therefore removes that problematic single-CPU check from the
rcu_tasks_rude_wait_gp() function.  This dispenses with the single-CPU
optimization, but there is no evidence indicating that this optimization
is important.  In addition, synchronize_rcu_tasks_generic() contains a
similar optimization (albeit only for early boot), which also splats.
(As in exactly why are you invoking synchronize_rcu_tasks_rude() so
early in boot, anyway???)

It is OK for the synchronize_rcu_tasks_rude() function's check to be
unsynchronized because the only times that this check can evaluate to
true is when there is only a single CPU running with preemption
disabled.

While in the area, this commit also fixes a minor bug in which a
call to synchronize_rcu_tasks_rude() would instead be attributed to
synchronize_rcu_tasks().

[ paulmck: Add "synchronize_" prefix and "()" suffix. ]

Signed-off-by: Zqiang <qiang1.zhang@intel.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 kernel/rcu/tasks.h | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
index 5de61f12a1645..eee38b0d362a8 100644
--- a/kernel/rcu/tasks.h
+++ b/kernel/rcu/tasks.h
@@ -560,8 +560,9 @@ static int __noreturn rcu_tasks_kthread(void *arg)
 static void synchronize_rcu_tasks_generic(struct rcu_tasks *rtp)
 {
 	/* Complain if the scheduler has not started.  */
-	WARN_ONCE(rcu_scheduler_active == RCU_SCHEDULER_INACTIVE,
-			 "synchronize_rcu_tasks called too soon");
+	if (WARN_ONCE(rcu_scheduler_active == RCU_SCHEDULER_INACTIVE,
+			 "synchronize_%s() called too soon", rtp->name))
+		return;
 
 	// If the grace-period kthread is running, use it.
 	if (READ_ONCE(rtp->kthread_ptr)) {
@@ -1064,9 +1065,6 @@ static void rcu_tasks_be_rude(struct work_struct *work)
 // Wait for one rude RCU-tasks grace period.
 static void rcu_tasks_rude_wait_gp(struct rcu_tasks *rtp)
 {
-	if (num_online_cpus() <= 1)
-		return;	// Fastpath for only one CPU.
-
 	rtp->n_ipis += cpumask_weight(cpu_online_mask);
 	schedule_on_each_cpu(rcu_tasks_be_rude);
 }
-- 
2.31.1.189.g2e36527f23


  parent reply	other threads:[~2023-01-05  0:50 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-05  0:44 [PATCH rcu 0/7] Torture-test updates for v6.3 Paul E. McKenney
2023-01-05  0:44 ` [PATCH rcu 1/6] rcu-tasks: Use accurate runstart time for RCU Tasks boot-time testing Paul E. McKenney
2023-01-05  0:44 ` [PATCH rcu 1/7] torture: Seed torture_random_state on CPU Paul E. McKenney
2023-01-05  0:44 ` [PATCH rcu 2/6] rcu-tasks: Improve comments explaining tasks_rcu_exit_srcu purpose Paul E. McKenney
2023-01-05  0:44 ` [PATCH rcu 2/7] refscale: Provide for initialization failure Paul E. McKenney
2023-01-05  0:44 ` [PATCH rcu 3/6] rcu-tasks: Remove preemption disablement around srcu_read_[un]lock() calls Paul E. McKenney
2023-01-05  0:44 ` [PATCH rcu 3/7] refscale: Add tests using SLAB_TYPESAFE_BY_RCU Paul E. McKenney
2023-01-05  0:44 ` [PATCH rcu 4/7] locktorture: Allow non-rtmutex lock types to be boosted Paul E. McKenney
2023-01-05  0:44 ` [PATCH rcu 4/6] rcu-tasks: Fix synchronize_rcu_tasks() VS zap_pid_ns_processes() Paul E. McKenney
2023-01-05  0:44 ` [PATCH rcu 5/7] locktorture: Make the rt_boost factor a tunable Paul E. McKenney
2023-01-05  0:44 ` Paul E. McKenney [this message]
2023-01-05  0:44 ` [PATCH rcu 6/6] rcu-tasks: Handle queue-shrink/callback-enqueue race condition Paul E. McKenney
2023-01-05  0:45 ` [PATCH rcu 6/7] rcutorture: Drop sparse lock-acquisition annotations Paul E. McKenney
2023-01-05  0:45 ` [PATCH rcu 7/7] torture: Fix hang during kthread shutdown phase 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=20230105004501.1771332-10-paulmck@kernel.org \
    --to=paulmck@kernel.org \
    --cc=kernel-team@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=qiang1.zhang@intel.com \
    --cc=rcu@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    /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

all inboxes | Powered by JetHome®