rcu/nocb: Last prep work before cpuset interface v2

nocb_gp_wait() iterates all CPUs within the rcuog's group even if they are have been de-offloaded. This is suboptimal if only few CPUs are offloaded within the group. And this will become even more a problem when a nocb kthread will be created for all possible CPUs in the future.

Therefore use a standard double linked list to link all the offloaded rdps and safely add/del their nodes as we (de-)offloaded them.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

Cc Uladzislau Rezki <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Joel Fernandes <[email protected]>

---

  kernel/rcu/tree.h      |  7 +++++--
  kernel/rcu/tree_nocb.h | 37 ++++++++++++++++++++++++++++++-------
  2 files changed, 35 insertions(+), 9 deletions(-)

kernel/rcu/tree.h +5 -2

index deeaf2fee714..486fc901bd08 100644
@@ -221,8 +221,11 @@ struct rcu_data {
  	struct swait_queue_head nocb_gp_wq; /* For nocb kthreads to sleep on. */
  	bool nocb_cb_sleep;		/* Is the nocb CB thread asleep? */
  	struct task_struct *nocb_cb_kthread;
-	struct rcu_data *nocb_next_cb_rdp;
-					/* Next rcu_data in wakeup chain. */
+	struct list_head nocb_head_rdp; /*
+					 * Head of rcu_data list in wakeup chain,
+					 * if rdp_gp.
+					 */
+	struct list_head nocb_entry_rdp; /* rcu_data node in wakeup chain. */
 
  	/* The following fields are used by CB kthread, hence new cacheline. */
  	struct rcu_data *nocb_gp_rdp ____cacheline_internodealigned_in_smp;

kernel/rcu/tree_nocb.h +0 -0

index 2461fe8d0c23..cc1165559177 100644
@@ -625,7 +625,15 @@ static void nocb_gp_wait(struct rcu_data *my_rdp)
  	 * and the global grace-period kthread are awakened if needed.
  	 */
  	WARN_ON_ONCE(my_rdp->nocb_gp_rdp != my_rdp);

kernel/rcu/tree_nocb.h +21 -6

@@ -1003,6 +1011,8 @@ static long rcu_nocb_rdp_deoffload(void *arg)
  	swait_event_exclusive(rdp->nocb_state_wq,
  			      !rcu_segcblist_test_flags(cblist, SEGCBLIST_KTHREAD_CB |
  							SEGCBLIST_KTHREAD_GP));
+	/* Don't bother iterate this one anymore on nocb_gp_wait() */
+	list_del_rcu(&rdp->nocb_entry_rdp);
  	/*
  	 * Lock one last time to acquire latest callback updates from kthreads
  	 * so we can later handle callbacks locally without locking.

@@ -1066,6 +1076,15 @@ static long rcu_nocb_rdp_offload(void *arg)
  		return -EINVAL;
 
  	pr_info("Offloading %d\n", rdp->cpu);
+
+	/*
+	 * Iterate this CPU on nocb_gp_wait(). We do it before locking nocb_gp_lock,
+	 * resetting nocb_gp_sleep and waking up the related "rcuog". Since nocb_gp_wait()
+	 * in turn locks nocb_gp_lock before setting nocb_gp_sleep again, we are guaranteed
+	 * to iterate this new rdp before "rcuog" goes to sleep again.
+	 */
+	list_add_tail_rcu(&rdp->nocb_entry_rdp, &rdp->nocb_gp_rdp->nocb_head_rdp);
+
  	/*
  	 * Can't use rcu_nocb_lock_irqsave() before SEGCBLIST_LOCKING
  	 * is set.

@@ -1268,7 +1287,6 @@ static void __init rcu_organize_nocb_kthreads(void)
  	int nl = 0;  /* Next GP kthread. */
  	struct rcu_data *rdp;
  	struct rcu_data *rdp_gp = NULL;  /* Suppress misguided gcc warn. */
-	struct rcu_data *rdp_prev = NULL;
 
  	if (!cpumask_available(rcu_nocb_mask))
  		return;

@@ -1288,8 +1306,8 @@ static void __init rcu_organize_nocb_kthreads(void)
  			/* New GP kthread, set up for CBs & next GP. */
  			gotnocbs = true;
  			nl = DIV_ROUND_UP(rdp->cpu + 1, ls) * ls;
-			rdp->nocb_gp_rdp = rdp;
  			rdp_gp = rdp;
+			INIT_LIST_HEAD(&rdp->nocb_head_rdp);
  			if (dump_tree) {
  				if (!firsttime)
  					pr_cont("%s\n", gotnocbscbs

@@ -1302,12 +1320,11 @@ static void __init rcu_organize_nocb_kthreads(void)
  		} else {
  			/* Another CB kthread, link to previous GP kthread. */
  			gotnocbscbs = true;
-			rdp->nocb_gp_rdp = rdp_gp;
-			rdp_prev->nocb_next_cb_rdp = rdp;
  			if (dump_tree)
  				pr_cont(" %d", cpu);
  		}
-		rdp_prev = rdp;
+		rdp->nocb_gp_rdp = rdp_gp;
+		list_add_tail(&rdp->nocb_entry_rdp, &rdp_gp->nocb_head_rdp);
  	}
  	if (gotnocbs && dump_tree)
  		pr_cont("%s\n", gotnocbscbs ? "" : " (self only)");

@@ -1369,6 +1386,7 @@ static void show_rcu_nocb_state(struct rcu_data *rdp)
  {
  	char bufw[20];
  	char bufr[20];
+	struct rcu_data *nocb_next_rdp;
  	struct rcu_segcblist *rsclp = &rdp->cblist;
  	bool waslocked;
  	bool wassleep;

@@ -1376,11 +1394,16 @@ static void show_rcu_nocb_state(struct rcu_data *rdp)
  	if (rdp->nocb_gp_rdp == rdp)
  		show_rcu_nocb_gp_state(rdp);
 
+	nocb_next_rdp = list_next_or_null_rcu(&rdp->nocb_gp_rdp->nocb_head_rdp,
+					      &rdp->nocb_entry_rdp,
+					      typeof(*rdp),
+					      nocb_entry_rdp);
+
  	sprintf(bufw, "%ld", rsclp->gp_seq[RCU_WAIT_TAIL]);
  	sprintf(bufr, "%ld", rsclp->gp_seq[RCU_NEXT_READY_TAIL]);
  	pr_info("   CB %d^%d->%d %c%c%c%c%c%c F%ld L%ld C%d %c%c%s%c%s%c%c q%ld %c CPU %d%s\n",
  		rdp->cpu, rdp->nocb_gp_rdp->cpu,
-		rdp->nocb_next_cb_rdp ? rdp->nocb_next_cb_rdp->cpu : -1,
+		nocb_next_rdp ? nocb_next_rdp->cpu : -1,
  		"kK"[!!rdp->nocb_cb_kthread],
  		"bB"[raw_spin_is_locked(&rdp->nocb_bypass_lock)],
  		"cC"[!!atomic_read(&rdp->nocb_lock_contended)],

Quoted 13 lines

kernel/rcu/tree_nocb.h +4 -1

index 2461fe8d0c23..cc1165559177 100644
@@ -625,7 +625,15 @@ static void nocb_gp_wait(struct rcu_data *my_rdp)
  	 * and the global grace-period kthread are awakened if needed.
  	 */
  	WARN_ON_ONCE(my_rdp->nocb_gp_rdp != my_rdp);
-	for (rdp = my_rdp; rdp; rdp = rdp->nocb_next_cb_rdp) {
+	/*
+	 * An rdp can be removed from the list after being de-offloaded or added
+	 * to the list before being (re-)offloaded. If the below loop happens while
+	 * an rdp is de-offloaded and then re-offloaded shortly afterward, we may

kernel/rcu/tree_nocb.h static void nocb_gp_wait(struct rcu_data *my_rdp) · line 636

Quoted 5 lines

kernel/rcu/tree_nocb.h +5 -0

+	 * shortcut and ignore a part of the rdp list due to racy list iteration.
+	 * Fortunately a new run through the entire loop is forced after an rdp is
+	 * added here so that such race get quickly fixed.
+	 */
+	list_for_each_entry_rcu(rdp, &my_rdp->nocb_head_rdp, nocb_entry_rdp, 1) {

Can we hit a (unlikely) case where repeated calls to de-offload/offload cause this loop to continue for long time?

kernel/rcu/tree_nocb.h static void nocb_gp_wait(struct rcu_data *my_rdp) · line 639

Quoted 4 lines

kernel/rcu/tree_nocb.h +0 -0

  		bool needwake_state = false;
  		if (!nocb_gp_enabled_cb(rdp))

Now that we can probe flags here, without holding the nocb_gp_lock first ( the case where de-offload and offload happens while we are iterating the list); can it cause a WARNING from below code?

	WARN_ON_ONCE(!rcu_segcblist_test_flags(cblist, SEGCBLIST_KTHREAD_GP));
	rcu_segcblist_clear_flags(cblist, SEGCBLIST_KTHREAD_GP);

The sequence like this is possible?

1. <de-offload>
Clear SEGCBLIST_OFFLOADED
2. nocb_gp_wait() clears SEGCBLIST_KTHREAD_GP in nocb_gp_update_state_deoffloading() and continues to next rdp.
3. <offload>
rdp_offload_toggle() hasn't been called yet.
4. rcuog thread migrates to different CPU, while executing the loop in nocb_gp_wait().
5. nocb_gp_wait() reaches the tail rdp.
6. Current CPU , where rcog thread is running hasn't observed SEGCBLIST_OFFLOADED clearing done in step 1; so, nocb_gp_enabled_cb() passes.

7. nocb_gp_wait() acquires the rdp's nocb lock and read the state to be deoffloaded; however, SEGCBLIST_KTHREAD_GP is not set and we hit WARN_ON_ONCE(!rcu_segcblist_test_flags(cblist, SEGCBLIST_KTHREAD_GP));

Thanks
Neeraj

In order to be able to toggle the offloaded state from cpusets, a nocb kthread will need to be created for all possible CPUs whenever the "rcu_nocbs=" or "nohz_full=" parameters are passed.

Therefore nocb_cb_wait() kthread must prepare to start running on a de-offloaded rdp. Simply move the sleeping condition in the beginning of the kthread callback is enough to prevent from running callbacks before the rdp ever becomes offloaded.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

Cc Uladzislau Rezki <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Joel Fernandes <[email protected]>

---

  kernel/rcu/tree_nocb.h | 23 ++++++++++++-----------
  1 file changed, 12 insertions(+), 11 deletions(-)

kernel/rcu/tree_nocb.h +12 -11

index cc1165559177..e1cb06840454 100644
@@ -797,6 +797,18 @@ static void nocb_cb_wait(struct rcu_data *rdp)
  	bool can_sleep = true;
  	struct rcu_node *rnp = rdp->mynode;
 
+	do {
+		swait_event_interruptible_exclusive(rdp->nocb_cb_wq,
+						    nocb_cb_wait_cond(rdp));
+
+		// VVV Ensure CB invocation follows _sleep test.
+		if (smp_load_acquire(&rdp->nocb_cb_sleep)) { // ^^^
+			WARN_ON(signal_pending(current));
+			trace_rcu_nocb_wake(rcu_state.name, rdp->cpu, TPS("WokeEmpty"));
+		}
+	} while (!nocb_cb_can_run(rdp));
+
+
  	local_irq_save(flags);
  	rcu_momentary_dyntick_idle();
  	local_irq_restore(flags);

@@ -849,17 +861,6 @@ static void nocb_cb_wait(struct rcu_data *rdp)
 
  	if (needwake_state)
  		swake_up_one(&rdp->nocb_state_wq);
-
-	do {
-		swait_event_interruptible_exclusive(rdp->nocb_cb_wq,
-						    nocb_cb_wait_cond(rdp));
-
-		// VVV Ensure CB invocation follows _sleep test.
-		if (smp_load_acquire(&rdp->nocb_cb_sleep)) { // ^^^
-			WARN_ON(signal_pending(current));
-			trace_rcu_nocb_wake(rcu_state.name, rdp->cpu, TPS("WokeEmpty"));
-		}
-	} while (!nocb_cb_can_run(rdp));
  }
 
  /*

Currently cpumask_available() is used to prevent from unwanted NOCB initialization. However if neither "rcu_nocbs=" nor "nohz_full=" parameters are passed but CONFIG_CPUMASK_OFFSTACK=n, the initialization path is still taken, running through all sorts of needless operations and iterations on an empty cpumask.

Fix this with relying on a real initialization state instead. This also optimize kthreads creation, sparing iteration over all online CPUs when nocb isn't initialized.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

Cc Uladzislau Rezki <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Joel Fernandes <[email protected]>

---
 kernel/rcu/tree_nocb.h | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

kernel/rcu/tree_nocb.h +0 -0

index e1cb06840454..d8ed3ee47a67 100644

kernel/rcu/tree_nocb.h +15 -7

+
 static int __init rcu_nocb_setup(char *str)
 {
 	alloc_bootmem_cpumask_var(&rcu_nocb_mask);

@@ -67,6 +70,7 @@ static int __init rcu_nocb_setup(char *str)
 		pr_warn("rcu_nocbs= bad CPU range, all CPUs set\n");
 		cpumask_setall(rcu_nocb_mask);
 	}
+	rcu_nocb_is_setup = true;
 	return 1;
 }
 __setup("rcu_nocbs=", rcu_nocb_setup);

@@ -1159,13 +1163,17 @@ void __init rcu_init_nohz(void)
 		need_rcu_nocb_mask = true;
 #endif /* #if defined(CONFIG_NO_HZ_FULL) */
 
-	if (!cpumask_available(rcu_nocb_mask) && need_rcu_nocb_mask) {
-		if (!zalloc_cpumask_var(&rcu_nocb_mask, GFP_KERNEL)) {
-			pr_info("rcu_nocb_mask allocation failed, callback offloading disabled.\n");
-			return;
+	if (need_rcu_nocb_mask) {
+		if (!cpumask_available(rcu_nocb_mask)) {
+			if (!zalloc_cpumask_var(&rcu_nocb_mask, GFP_KERNEL)) {
+				pr_info("rcu_nocb_mask allocation failed, callback offloading disabled.\n");
+				return;
+			}
 		}
+		rcu_nocb_is_setup = true;
 	}
-	if (!cpumask_available(rcu_nocb_mask))
+
+	if (!rcu_nocb_is_setup)
 		return;
 
 #if defined(CONFIG_NO_HZ_FULL)

@@ -1267,8 +1275,10 @@ static void __init rcu_spawn_nocb_kthreads(void)
 {
 	int cpu;
 
-	for_each_online_cpu(cpu)
-		rcu_spawn_cpu_nocb_kthread(cpu);
+	if (rcu_nocb_is_setup) {
+		for_each_online_cpu(cpu)
+			rcu_spawn_cpu_nocb_kthread(cpu);
+	}
 }
 
 /* How many CB CPU IDs per GP kthread?  Default of -1 for sqrt(nr_cpu_ids). */

Signature · 1 line

2.25.1

Quoted 25 lines · Frederic Weisbecker

Currently cpumask_available() is used to prevent from unwanted NOCB initialization. However if neither "rcu_nocbs=" nor "nohz_full=" parameters are passed but CONFIG_CPUMASK_OFFSTACK=n, the initialization path is still taken, running through all sorts of needless operations and iterations on an empty cpumask.

Fix this with relying on a real initialization state instead. This also optimize kthreads creation, sparing iteration over all online CPUs when nocb isn't initialized.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

Cc Uladzislau Rezki <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Joel Fernandes <[email protected]>

---
 kernel/rcu/tree_nocb.h | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

kernel/rcu/tree_nocb.h +0 -0

index e1cb06840454..d8ed3ee47a67 100644

kernel/rcu/tree_nocb.h static inline bool rcu_current_is_nocb_kthread(struct rcu_data *rdp) · line 64

Quoted 5 lines

kernel/rcu/tree_nocb.h +2 -0

@@ -60,6 +60,9 @@ static inline bool rcu_current_is_nocb_kthread(struct rcu_data *rdp)
  * Parse the boot-time rcu_nocb_mask CPU list from the kernel parameters.
  * If the list is invalid, a warning is emitted and all CPUs are offloaded.
  */
+
+static bool rcu_nocb_is_setup;

I am taking this as is for now (modulo wordsmithing), but should this variable instead be in the rcu_state structure? The advantage of putting it there is keeping the state together. The corresponding disadvantage is that the state is globally visible within RCU.

Thoughts?

Currently cpumask_available() is used to prevent from unwanted NOCB initialization. However if neither "rcu_nocbs=" nor "nohz_full=" parameters are passed but CONFIG_CPUMASK_OFFSTACK=n, the initialization path is still taken, running through all sorts of needless operations and iterations on an empty cpumask.

Fix this with relying on a real initialization state instead. This also optimize kthreads creation, sparing iteration over all online CPUs when nocb isn't initialized.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

Cc Uladzislau Rezki <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Joel Fernandes <[email protected]>

---

  kernel/rcu/tree_nocb.h | 24 +++++++++++++++++-------
  1 file changed, 17 insertions(+), 7 deletions(-)

kernel/rcu/tree_nocb.h +17 -7

index e1cb06840454..d8ed3ee47a67 100644
@@ -60,6 +60,9 @@ static inline bool rcu_current_is_nocb_kthread(struct rcu_data *rdp)
   * Parse the boot-time rcu_nocb_mask CPU list from the kernel parameters.
   * If the list is invalid, a warning is emitted and all CPUs are offloaded.
   */
+
+static bool rcu_nocb_is_setup;
+
  static int __init rcu_nocb_setup(char *str)
  {
  	alloc_bootmem_cpumask_var(&rcu_nocb_mask);

@@ -67,6 +70,7 @@ static int __init rcu_nocb_setup(char *str)
  		pr_warn("rcu_nocbs= bad CPU range, all CPUs set\n");
  		cpumask_setall(rcu_nocb_mask);
  	}
+	rcu_nocb_is_setup = true;
  	return 1;
  }
  __setup("rcu_nocbs=", rcu_nocb_setup);

@@ -1159,13 +1163,17 @@ void __init rcu_init_nohz(void)
  		need_rcu_nocb_mask = true;
  #endif /* #if defined(CONFIG_NO_HZ_FULL) */
 
-	if (!cpumask_available(rcu_nocb_mask) && need_rcu_nocb_mask) {
-		if (!zalloc_cpumask_var(&rcu_nocb_mask, GFP_KERNEL)) {
-			pr_info("rcu_nocb_mask allocation failed, callback offloading disabled.\n");
-			return;
+	if (need_rcu_nocb_mask) {
+		if (!cpumask_available(rcu_nocb_mask)) {
+			if (!zalloc_cpumask_var(&rcu_nocb_mask, GFP_KERNEL)) {
+				pr_info("rcu_nocb_mask allocation failed, callback offloading disabled.\n");
+				return;
+			}
  		}
+		rcu_nocb_is_setup = true;
  	}
-	if (!cpumask_available(rcu_nocb_mask))
+
+	if (!rcu_nocb_is_setup)
  		return;
 
  #if defined(CONFIG_NO_HZ_FULL)

@@ -1267,8 +1275,10 @@ static void __init rcu_spawn_nocb_kthreads(void)
  {
  	int cpu;
 
-	for_each_online_cpu(cpu)
-		rcu_spawn_cpu_nocb_kthread(cpu);
+	if (rcu_nocb_is_setup) {
+		for_each_online_cpu(cpu)
+			rcu_spawn_cpu_nocb_kthread(cpu);
+	}
  }
 
  /* How many CB CPU IDs per GP kthread?  Default of -1 for sqrt(nr_cpu_ids). */

In order to be able to (de-)offload any CPU using cpuset in the future, create a NOCB kthread for all possible CPUs. For now this is done only as long as the "rcu_nocb=" or "nohz_full=" kernel parameters are passed

Hi,

Quoted 3 lines · Frederic Weisbecker

In order to be able to (de-)offload any CPU using cpuset in the future, create a NOCB kthread for all possible CPUs. For now this is done only as long as the "rcu_nocb=" or "nohz_full=" kernel parameters are passed

Super nitpick! In subject and above sentence, "rcu_nocb=" is actually "rcu_nocbs=", isn't it?

Feel free to ignore the noise, of course. :)

In order to be able to (de-)offload any CPU using cpuset in the future, create a NOCB kthread for all possible CPUs. For now this is done only as long as the "rcu_nocb=" or "nohz_full=" kernel parameters are passed to avoid the unnecessary overhead for most users.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

Cc Uladzislau Rezki <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Joel Fernandes <[email protected]>

---

  kernel/rcu/tree_nocb.h | 14 ++++++--------
  1 file changed, 6 insertions(+), 8 deletions(-)

kernel/rcu/tree_nocb.h +0 -0

index d8ed3ee47a67..9d37916278d4 100644

kernel/rcu/tree_nocb.h +6 -3

+	/* If it already has an rcuo kthread, then nothing to do. */
+	if (rdp->nocb_cb_kthread)
  		return;
 
  	/* If we didn't spawn the GP kthread first, reorganize! */

@@ -1261,7 +1258,7 @@ static void rcu_spawn_one_nocb_kthread(int cpu)
   */
  static void rcu_spawn_cpu_nocb_kthread(int cpu)
  {
-	if (rcu_scheduler_fully_active)
+	if (rcu_scheduler_fully_active && rcu_nocb_is_setup)
  		rcu_spawn_one_nocb_kthread(cpu);
  }
 

@@ -1311,7 +1308,7 @@ static void __init rcu_organize_nocb_kthreads(void)
  	 * Should the corresponding CPU come online in the future, then
  	 * we will spawn the needed set of rcu_nocb_kthread() kthreads.
  	 */
-	for_each_cpu(cpu, rcu_nocb_mask) {
+	for_each_possible_cpu(cpu) {
  		rdp = per_cpu_ptr(&rcu_data, cpu);
  		if (rdp->cpu >= nl) {
  			/* New GP kthread, set up for CBs & next GP. */

@@ -1335,7 +1332,8 @@ static void __init rcu_organize_nocb_kthreads(void)
  				pr_cont(" %d", cpu);
  		}
  		rdp->nocb_gp_rdp = rdp_gp;
-		list_add_tail(&rdp->nocb_entry_rdp, &rdp_gp->nocb_head_rdp);
+		if (cpumask_test_cpu(cpu, rcu_nocb_mask))
+			list_add_tail(&rdp->nocb_entry_rdp, &rdp_gp->nocb_head_rdp);
  	}
  	if (gotnocbs && dump_tree)
  		pr_cont("%s\n", gotnocbscbs ? "" : " (self only)");

Quoted 5 lines · Frederic Weisbecker

In order to be able to (de-)offload any CPU using cpuset in the future, create a NOCB kthread for all possible CPUs. For now this is done only as long as the "rcu_nocb=" or "nohz_full=" kernel parameters are passed to avoid the unnecessary overhead for most users.

Quoted 7 lines

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

Cc Uladzislau Rezki <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Joel Fernandes <[email protected]>

---

Reviewed-by Neeraj Upadhyay <[email protected]>

Quoted 3 lines

  kernel/rcu/tree_nocb.h | 14 ++++++--------
  1 file changed, 6 insertions(+), 8 deletions(-)

kernel/rcu/tree_nocb.h static void rcu_spawn_one_nocb_kthread(int cpu) · line 1235

Quoted 12 lines

kernel/rcu/tree_nocb.h +0 -5

index d8ed3ee47a67..9d37916278d4 100644
@@ -1229,11 +1229,8 @@ static void rcu_spawn_one_nocb_kthread(int cpu)
  	struct rcu_data *rdp_gp;
  	struct task_struct *t;
-	/*
-	 * If this isn't a no-CBs CPU or if it already has an rcuo kthread,
-	 * then nothing to do.
-	 */
-	if (!rcu_is_nocb_cpu(cpu) || rdp->nocb_cb_kthread)

As rcu_is_nocb_cpu() does not have a user, we can probably remove it?

If a user wants to boot without any CPU in offloaded mode initially but with the possibility to offload them later using cpusets, provide a way to simply pass an empty "rcu_nocbs" kernel parameter which will enforce the creation of dormant nocb kthreads.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

Cc Uladzislau Rezki <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Joel Fernandes <[email protected]>

---
 .../admin-guide/kernel-parameters.txt         | 26 ++++++++++++-------
 kernel/rcu/tree_nocb.h                        | 10 ++++---
 2 files changed, 23 insertions(+), 13 deletions(-)

Documentation/admin-guide/kernel-parameters.txt +17 -9

index cd860dc7c60b..6ff1a5f06383 100644
@@ -4351,20 +4351,28 @@
 				Disable the Correctable Errors Collector,
 				see CONFIG_RAS_CEC help text.
 
-	rcu_nocbs=	[KNL]
-			The argument is a cpu list, as described above.
+	rcu_nocbs[=cpu-list]
+			[KNL] The optional argument is a cpu list,
+			as described above.
 
-			In kernels built with CONFIG_RCU_NOCB_CPU=y, set
-			the specified list of CPUs to be no-callback CPUs.
-			Invocation of these CPUs' RCU callbacks will be
-			offloaded to "rcuox/N" kthreads created for that
-			purpose, where "x" is "p" for RCU-preempt, and
-			"s" for RCU-sched, and "N" is the CPU number.
-			This reduces OS jitter on the offloaded CPUs,
+			In kernels built with CONFIG_RCU_NOCB_CPU=y, enable the
+			no-callback CPU mode. Invocation of such CPUs' RCU
+			callbacks will be offloaded to "rcuox/N" kthreads
+			created	for that purpose, where "x" is "p" for
+			RCU-preempt, and "s" for RCU-sched, and "N" is the CPU
+			number.	This reduces OS jitter on the offloaded CPUs,
 			which can be useful for HPC and real-time
 			workloads.  It can also improve energy efficiency
 			for asymmetric multiprocessors.
 
+			If a cpulist is passed as an argument, the specified
+			list of	CPUs is set to no-callback mode from boot.
+
+			If otherwise the '=' sign and the cpulist arguments are
+			omitted, no CPU will be set to no-callback mode from
+			boot but cpuset will allow for toggling that mode at
+			runtime.
+
 	rcu_nocb_poll	[KNL]
 			Rather than requiring that offloaded CPUs
 			(specified by rcu_nocbs= above) explicitly

kernel/rcu/tree_nocb.h +6 -4

index 9d37916278d4..d915780d40c8 100644
@@ -66,14 +66,16 @@ static bool rcu_nocb_is_setup;
 static int __init rcu_nocb_setup(char *str)
 {
 	alloc_bootmem_cpumask_var(&rcu_nocb_mask);
-	if (cpulist_parse(str, rcu_nocb_mask)) {
-		pr_warn("rcu_nocbs= bad CPU range, all CPUs set\n");
-		cpumask_setall(rcu_nocb_mask);
+	if (*str == '=') {
+		if (cpulist_parse(++str, rcu_nocb_mask)) {
+			pr_warn("rcu_nocbs= bad CPU range, all CPUs set\n");
+			cpumask_setall(rcu_nocb_mask);
+		}
 	}
 	rcu_nocb_is_setup = true;
 	return 1;
 }
-__setup("rcu_nocbs=", rcu_nocb_setup);
+__setup("rcu_nocbs", rcu_nocb_setup);
 
 static int __init parse_rcu_nocb_poll(char *arg)
 {

Signature · 1 line

2.25.1

Quoted 4 lines · Frederic Weisbecker

If a user wants to boot without any CPU in offloaded mode initially but with the possibility to offload them later using cpusets, provide a way to simply pass an empty "rcu_nocbs" kernel parameter which will enforce the creation of dormant nocb kthreads.

Huh. This would have been a use for Yury Norov's "none" bitmask specifier. ;-)

I pulled this one in with the usual wordsmithing.

Quoted 8 lines · Paul E. McKenney

Quoted 4 lines · Frederic Weisbecker

If a user wants to boot without any CPU in offloaded mode initially but with the possibility to offload them later using cpusets, provide a way to simply pass an empty "rcu_nocbs" kernel parameter which will enforce the creation of dormant nocb kthreads.

Huh. This would have been a use for Yury Norov's "none" bitmask specifier. ;-)

This must be the last missing piece before we can finally support NR_CPUS=0 ;)

Quoted 1 line

I pulled this one in with the usual wordsmithing.

Thanks!

Quoted 4 lines · Frederic Weisbecker

If a user wants to boot without any CPU in offloaded mode initially but with the possibility to offload them later using cpusets, provide a way to simply pass an empty "rcu_nocbs" kernel parameter which will enforce the creation of dormant nocb kthreads.

Huh. This would have been a use for Yury Norov's "none" bitmask specifier. ;-)

I pulled this one in with the usual wordsmithing.

Thanx, Paul

Quoted 1 line · Paul E. McKenney

On Tue, Nov 23, 2021 at 01:37:07AM +0100, Frederic Weisbecker wrote:

lib/bitmap.c +1 -0

+	}

Quoted 9 lines · Paul E. McKenney

Quoted 4 lines · Frederic Weisbecker

If a user wants to boot without any CPU in offloaded mode initially but with the possibility to offload them later using cpusets, provide a way to simply pass an empty "rcu_nocbs" kernel parameter which will enforce the creation of dormant nocb kthreads.

Huh. This would have been a use for Yury Norov's "none" bitmask specifier. ;-)

Quoted 3 lines

I pulled this one in with the usual wordsmithing.

Thanx, Paul

I think 'rcu_nocbs=,' should work as 'none'. But I admit that it looks awkward. The following patch adds clear 'none' semantics to the parser. If you like it, I think you may drop non-documentation part of this patch.

Quoted 14 lines · Yury Norov

Quoted 12 lines · Paul E. McKenney

Quoted 4 lines · Frederic Weisbecker

If a user wants to boot without any CPU in offloaded mode initially but with the possibility to offload them later using cpusets, provide a way to simply pass an empty "rcu_nocbs" kernel parameter which will enforce the creation of dormant nocb kthreads.

Huh. This would have been a use for Yury Norov's "none" bitmask specifier. ;-)

I pulled this one in with the usual wordsmithing.

Thanx, Paul

Quoted 4 lines

I think 'rcu_nocbs=,' should work as 'none'. But I admit that it looks awkward. The following patch adds clear 'none' semantics to the parser. If you like it, I think you may drop non-documentation part of this patch.

I don't have real objection, but I fear that "rcu_nocbs=none" might be interpretated as rcu_nocbs is entirely deactivated, whereas "rcu_nocbs" alone makes it clear that we are turning something on.

We can support both though.

Thanks.

If a user wants to boot without any CPU in offloaded mode initially but with the possibility to offload them later using cpusets, provide a way to simply pass an empty "rcu_nocbs" kernel parameter which will enforce the creation of dormant nocb kthreads.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

Cc Uladzislau Rezki <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Joel Fernandes <[email protected]>

---

  .../admin-guide/kernel-parameters.txt         | 26 ++++++++++++-------
  kernel/rcu/tree_nocb.h                        | 10 ++++---
  2 files changed, 23 insertions(+), 13 deletions(-)

Documentation/admin-guide/kernel-parameters.txt +17 -9

index cd860dc7c60b..6ff1a5f06383 100644
@@ -4351,20 +4351,28 @@
  				Disable the Correctable Errors Collector,
  				see CONFIG_RAS_CEC help text.
 
-	rcu_nocbs=	[KNL]
-			The argument is a cpu list, as described above.
+	rcu_nocbs[=cpu-list]
+			[KNL] The optional argument is a cpu list,
+			as described above.
 
-			In kernels built with CONFIG_RCU_NOCB_CPU=y, set
-			the specified list of CPUs to be no-callback CPUs.
-			Invocation of these CPUs' RCU callbacks will be
-			offloaded to "rcuox/N" kthreads created for that
-			purpose, where "x" is "p" for RCU-preempt, and
-			"s" for RCU-sched, and "N" is the CPU number.
-			This reduces OS jitter on the offloaded CPUs,
+			In kernels built with CONFIG_RCU_NOCB_CPU=y, enable the
+			no-callback CPU mode. Invocation of such CPUs' RCU
+			callbacks will be offloaded to "rcuox/N" kthreads
+			created	for that purpose, where "x" is "p" for
+			RCU-preempt, and "s" for RCU-sched, and "N" is the CPU
+			number.	This reduces OS jitter on the offloaded CPUs,
  			which can be useful for HPC and real-time
  			workloads.  It can also improve energy efficiency
  			for asymmetric multiprocessors.
 
+			If a cpulist is passed as an argument, the specified
+			list of	CPUs is set to no-callback mode from boot.
+
+			If otherwise the '=' sign and the cpulist arguments are
+			omitted, no CPU will be set to no-callback mode from
+			boot but cpuset will allow for toggling that mode at
+			runtime.
+
  	rcu_nocb_poll	[KNL]
  			Rather than requiring that offloaded CPUs
  			(specified by rcu_nocbs= above) explicitly

kernel/rcu/tree_nocb.h +6 -4

index 9d37916278d4..d915780d40c8 100644
@@ -66,14 +66,16 @@ static bool rcu_nocb_is_setup;
  static int __init rcu_nocb_setup(char *str)
  {
  	alloc_bootmem_cpumask_var(&rcu_nocb_mask);
-	if (cpulist_parse(str, rcu_nocb_mask)) {
-		pr_warn("rcu_nocbs= bad CPU range, all CPUs set\n");
-		cpumask_setall(rcu_nocb_mask);
+	if (*str == '=') {
+		if (cpulist_parse(++str, rcu_nocb_mask)) {
+			pr_warn("rcu_nocbs= bad CPU range, all CPUs set\n");
+			cpumask_setall(rcu_nocb_mask);
+		}
  	}
  	rcu_nocb_is_setup = true;
  	return 1;
  }
-__setup("rcu_nocbs=", rcu_nocb_setup);
+__setup("rcu_nocbs", rcu_nocb_setup);
 
  static int __init parse_rcu_nocb_poll(char *arg)
  {

rcu_spawn_one_nocb_kthread() is only called by rcu_spawn_cpu_nocb_kthread(). Don't bother with two separate functions and merge them.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

Cc Uladzislau Rezki <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Joel Fernandes <[email protected]>

---

  kernel/rcu/tree_nocb.h | 15 ++++-----------
  1 file changed, 4 insertions(+), 11 deletions(-)

kernel/rcu/tree_nocb.h +4 -11

index d915780d40c8..7e8da346127a 100644
@@ -1225,12 +1225,15 @@ static void __init rcu_boot_init_nocb_percpu_data(struct rcu_data *rdp)
   * rcuo CB kthread, spawn it.  Additionally, if the rcuo GP kthread
   * for this CPU's group has not yet been created, spawn it as well.
   */
-static void rcu_spawn_one_nocb_kthread(int cpu)
+static void rcu_spawn_cpu_nocb_kthread(int cpu)
  {
  	struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
  	struct rcu_data *rdp_gp;
  	struct task_struct *t;
 
+	if (!rcu_scheduler_fully_active || !rcu_nocb_is_setup)
+		return;
+
  	/* If it already has an rcuo kthread, then nothing to do. */
  	if (rdp->nocb_cb_kthread)
  		return;

@@ -1254,16 +1257,6 @@ static void rcu_spawn_one_nocb_kthread(int cpu)
  	WRITE_ONCE(rdp->nocb_gp_kthread, rdp_gp->nocb_gp_kthread);
  }
 
-/*
- * If the specified CPU is a no-CBs CPU that does not already have its
- * rcuo kthread, spawn it.
- */
-static void rcu_spawn_cpu_nocb_kthread(int cpu)
-{
-	if (rcu_scheduler_fully_active && rcu_nocb_is_setup)
-		rcu_spawn_one_nocb_kthread(cpu);
-}
-
  /*
   * Once the scheduler is running, spawn rcuo kthreads for all online
   * no-CBs CPUs.  This assumes that the early_initcall()s happen before

Changes since v1 after Paul's reviews:

* Clarify why the DEL vs ADD possible race on rdp group list is ok [1/6]
* Update kernel parameters documentation [5/6]
* Only create rcuo[sp] kthreads for CPUs that have ever come online [4/6]
* Consider nohz_full= on changelogs

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
rcu/nocb

HEAD: da51363e5ddf54d6ce9c2cfbab946f8914519290

Hi,

Quoted 11 lines · Frederic Weisbecker

Changes since v1 after Paul's reviews:

* Clarify why the DEL vs ADD possible race on rdp group list is ok [1/6]
* Update kernel parameters documentation [5/6]
* Only create rcuo[sp] kthreads for CPUs that have ever come online [4/6]
* Consider nohz_full= on changelogs

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
rcu/nocb

HEAD: da51363e5ddf54d6ce9c2cfbab946f8914519290

I run this on RT. It survived rcutorture on different kernel cmdline configurations and creation of rcuox/N kthreads seemed sane. :)

FWIW, feel free to add

Tested-by Juri Lelli <[email protected]>

Thanks for working on this!