[RFC PATCH 0/8] cgroup/cpuset: Support RCU_NOCB on isolated partitions

This patch series is based on the RFC patch from Frederic [1]. Instead
of offering RCU_NOCB as a separate option, it is now lumped into a
root-only cpuset.cpus.isolation_full flag that will enable all the
additional CPU isolation capabilities available for isolated partitions
if set. RCU_NOCB is just the first one to this party. Additional dynamic
CPU isolation capabilities will be added in the future.

The first 2 patches are adopted from Federic with minor twists to fix
merge conflicts and compilation issue. The rests are for implementing
the new cpuset.cpus.isolation_full interface which is essentially a flag
to globally enable or disable full CPU isolation on isolated partitions.
On read, it also shows the CPU isolation capabilities that are currently
enabled. RCU_NOCB requires that the rcu_nocbs option be present in
the kernel boot command line. Without that, the rcu_nocb functionality
cannot be enabled even if the isolation_full flag is set. So we allow
users to check the isolation_full file to verify that if the desired
CPU isolation capability is enabled or not.

Only sanity checking has been done so far. More testing, especially on
the RCU side, will be needed.

[1] https://lore.kernel.org/lkml/20220525221055.1152307-1-frederic@kernel.org/

Frederic Weisbecker (2):
  rcu/nocb: Pass a cpumask instead of a single CPU to offload/deoffload
  rcu/nocb: Prepare to change nocb cpumask from CPU-hotplug protected
    cpuset caller

Waiman Long (6):
  rcu/no_cb: Add rcu_nocb_enabled() to expose the rcu_nocb state
  cgroup/cpuset: Better tracking of addition/deletion of isolated CPUs
  cgroup/cpuset: Add cpuset.cpus.isolation_full
  cgroup/cpuset: Enable dynamic rcu_nocb mode on isolated CPUs
  cgroup/cpuset: Document the new cpuset.cpus.isolation_full control
    file
  cgroup/cpuset: Update test_cpuset_prs.sh to handle
    cpuset.cpus.isolation_full

 Documentation/admin-guide/cgroup-v2.rst       |  24 ++
 include/linux/rcupdate.h                      |  15 +-
 kernel/cgroup/cpuset.c                        | 237 ++++++++++++++----
 kernel/rcu/rcutorture.c                       |   6 +-
 kernel/rcu/tree_nocb.h                        | 118 ++++++---
 .../selftests/cgroup/test_cpuset_prs.sh       |  23 +-
 6 files changed, 337 insertions(+), 86 deletions(-)

-- 
2.39.3

From: Frederic Weisbecker <frederic@kernel.org>

Currently the interface to toggle callbacks offloading state only takes
a single CPU per call. Now driving RCU NOCB through cpusets requires
to be able to change the offloading state of a whole set of CPUs.

To make it easier, extend the (de-)offloading interface to support a
cpumask.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Zefan Li <redacted>
Cc: Tejun Heo <tj@kernel.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Phil Auld <redacted>
Cc: Nicolas Saenz Julienne <nsaenz@kernel.org>
Cc: Marcelo Tosatti <redacted>
Cc: Paul Gortmaker <redacted>
Cc: Waiman Long <longman@redhat.com>
Cc: Daniel Bristot de Oliveira <bristot@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 include/linux/rcupdate.h |   9 ++--
 kernel/rcu/rcutorture.c  |   4 +-
 kernel/rcu/tree_nocb.h   | 102 ++++++++++++++++++++++++++-------------
 3 files changed, 76 insertions(+), 39 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 0746b1b0b663..b649344075d2 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h

@@ -142,13 +142,14 @@ static inline void rcu_irq_work_resched(void) { }
 
 #ifdef CONFIG_RCU_NOCB_CPU
 void rcu_init_nohz(void);
-int rcu_nocb_cpu_offload(int cpu);
-int rcu_nocb_cpu_deoffload(int cpu);
+int rcu_nocb_cpumask_update(const struct cpumask *cpumask, bool offload);
 void rcu_nocb_flush_deferred_wakeup(void);
 #else /* #ifdef CONFIG_RCU_NOCB_CPU */
 static inline void rcu_init_nohz(void) { }
-static inline int rcu_nocb_cpu_offload(int cpu) { return -EINVAL; }
-static inline int rcu_nocb_cpu_deoffload(int cpu) { return 0; }
+static inline int rcu_nocb_cpumask_update(const struct cpumask *cpumask, bool offload)
+{
+	return -EINVAL;
+}
 static inline void rcu_nocb_flush_deferred_wakeup(void) { }
 #endif /* #else #ifdef CONFIG_RCU_NOCB_CPU */
 

diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 7567ca8e743c..228a5488eb5e 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c

@@ -2140,10 +2140,10 @@ static int rcu_nocb_toggle(void *arg)
 		r = torture_random(&rand);
 		cpu = (r >> 1) % (maxcpu + 1);
 		if (r & 0x1) {
-			rcu_nocb_cpu_offload(cpu);
+			rcu_nocb_cpumask_update(cpumask_of(cpu), true);
 			atomic_long_inc(&n_nocb_offload);
 		} else {
-			rcu_nocb_cpu_deoffload(cpu);
+			rcu_nocb_cpumask_update(cpumask_of(cpu), false);
 			atomic_long_inc(&n_nocb_deoffload);
 		}
 		toggle_delay = torture_random(&rand) % toggle_fuzz + toggle_interval;

diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
index 4efbf7333d4e..60b0a15ed6e2 100644
--- a/kernel/rcu/tree_nocb.h
+++ b/kernel/rcu/tree_nocb.h

@@ -1203,29 +1203,23 @@ static long rcu_nocb_rdp_deoffload(void *arg)
 	return 0;
 }
 
-int rcu_nocb_cpu_deoffload(int cpu)
+static int rcu_nocb_cpu_deoffload(int cpu)
 {
 	struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
 	int ret = 0;
 
-	cpus_read_lock();
-	mutex_lock(&rcu_state.barrier_mutex);
-	if (rcu_rdp_is_offloaded(rdp)) {
-		if (cpu_online(cpu)) {
-			ret = work_on_cpu(cpu, rcu_nocb_rdp_deoffload, rdp);
-			if (!ret)
-				cpumask_clear_cpu(cpu, rcu_nocb_mask);
-		} else {
-			pr_info("NOCB: Cannot CB-deoffload offline CPU %d\n", rdp->cpu);
-			ret = -EINVAL;
-		}
-	}
-	mutex_unlock(&rcu_state.barrier_mutex);
-	cpus_read_unlock();
+	if (cpu_is_offline(cpu))
+		return -EINVAL;
+
+	if (!rcu_rdp_is_offloaded(rdp))
+		return 0;
+
+	ret = work_on_cpu(cpu, rcu_nocb_rdp_deoffload, rdp);
+	if (!ret)
+		cpumask_clear_cpu(cpu, rcu_nocb_mask);
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(rcu_nocb_cpu_deoffload);
 
 static long rcu_nocb_rdp_offload(void *arg)
 {

@@ -1236,12 +1230,6 @@ static long rcu_nocb_rdp_offload(void *arg)
 	struct rcu_data *rdp_gp = rdp->nocb_gp_rdp;
 
 	WARN_ON_ONCE(rdp->cpu != raw_smp_processor_id());
-	/*
-	 * For now we only support re-offload, ie: the rdp must have been
-	 * offloaded on boot first.
-	 */
-	if (!rdp->nocb_gp_rdp)
-		return -EINVAL;
 
 	if (WARN_ON_ONCE(!rdp_gp->nocb_gp_kthread))
 		return -EINVAL;

@@ -1288,29 +1276,77 @@ static long rcu_nocb_rdp_offload(void *arg)
 	return 0;
 }
 
-int rcu_nocb_cpu_offload(int cpu)
+static int rcu_nocb_cpu_offload(int cpu)
 {
 	struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
-	int ret = 0;
+	int ret;
+
+	if (cpu_is_offline(cpu))
+		return -EINVAL;
+
+	if (rcu_rdp_is_offloaded(rdp))
+		return 0;
+
+	ret = work_on_cpu(cpu, rcu_nocb_rdp_offload, rdp);
+	if (!ret)
+		cpumask_set_cpu(cpu, rcu_nocb_mask);
+
+	return ret;
+}
+
+int rcu_nocb_cpumask_update(const struct cpumask *cpumask, bool offload)
+{
+	int cpu;
+	int err = 0;
+	int err_cpu;
+	cpumask_var_t saved_nocb_mask;
+
+	if (!alloc_cpumask_var(&saved_nocb_mask, GFP_KERNEL))
+		return -ENOMEM;
+
+	cpumask_copy(saved_nocb_mask, rcu_nocb_mask);
 
 	cpus_read_lock();
 	mutex_lock(&rcu_state.barrier_mutex);
-	if (!rcu_rdp_is_offloaded(rdp)) {
-		if (cpu_online(cpu)) {
-			ret = work_on_cpu(cpu, rcu_nocb_rdp_offload, rdp);
-			if (!ret)
-				cpumask_set_cpu(cpu, rcu_nocb_mask);
+	for_each_cpu(cpu, cpumask) {
+		if (offload) {
+			err = rcu_nocb_cpu_offload(cpu);
+			if (err < 0) {
+				err_cpu = cpu;
+				pr_err("NOCB: offload cpu %d failed (%d)\n", cpu, err);
+				break;
+			}
 		} else {
-			pr_info("NOCB: Cannot CB-offload offline CPU %d\n", rdp->cpu);
-			ret = -EINVAL;
+			err = rcu_nocb_cpu_deoffload(cpu);
+			if (err < 0) {
+				err_cpu = cpu;
+				pr_err("NOCB: deoffload cpu %d failed (%d)\n", cpu, err);
+				break;
+			}
+		}
+	}
+
+	/* Rollback in case of error */
+	if (err < 0) {
+		err_cpu = cpu;
+		for_each_cpu(cpu, cpumask) {
+			if (err_cpu == cpu)
+				break;
+			if (cpumask_test_cpu(cpu, saved_nocb_mask))
+				WARN_ON_ONCE(rcu_nocb_cpu_offload(cpu));
+			else
+				WARN_ON_ONCE(rcu_nocb_cpu_deoffload(cpu));
 		}
 	}
+
 	mutex_unlock(&rcu_state.barrier_mutex);
 	cpus_read_unlock();
 
-	return ret;
+	free_cpumask_var(saved_nocb_mask);
+
+	return err;
 }
-EXPORT_SYMBOL_GPL(rcu_nocb_cpu_offload);
+EXPORT_SYMBOL_GPL(rcu_nocb_cpumask_update);
 
 #ifdef CONFIG_RCU_LAZY
 static unsigned long

-- 
2.39.3

From: Frederic Weisbecker <frederic@kernel.org>

cpusets is going to use the NOCB (de-)offloading interface while
holding hotplug lock. Therefore pull out the responsibility of protecting
against concurrent CPU-hotplug changes to the callers of
rcu_nocb_cpumask_update().

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Zefan Li <redacted>
Cc: Tejun Heo <tj@kernel.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Phil Auld <redacted>
Cc: Nicolas Saenz Julienne <nsaenz@kernel.org>
Cc: Marcelo Tosatti <redacted>
Cc: Paul Gortmaker <redacted>
Cc: Waiman Long <longman@redhat.com>
Cc: Daniel Bristot de Oliveira <bristot@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/rcu/rcutorture.c | 2 ++
 kernel/rcu/tree_nocb.h  | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/rcu/rcutorture.c b/kernel/rcu/rcutorture.c
index 228a5488eb5e..e935152346ff 100644
--- a/kernel/rcu/rcutorture.c
+++ b/kernel/rcu/rcutorture.c

@@ -2139,6 +2139,7 @@ static int rcu_nocb_toggle(void *arg)
 	do {
 		r = torture_random(&rand);
 		cpu = (r >> 1) % (maxcpu + 1);
+		cpus_read_lock();
 		if (r & 0x1) {
 			rcu_nocb_cpumask_update(cpumask_of(cpu), true);
 			atomic_long_inc(&n_nocb_offload);

@@ -2146,6 +2147,7 @@ static int rcu_nocb_toggle(void *arg)
 			rcu_nocb_cpumask_update(cpumask_of(cpu), false);
 			atomic_long_inc(&n_nocb_deoffload);
 		}
+		cpus_read_unlock();
 		toggle_delay = torture_random(&rand) % toggle_fuzz + toggle_interval;
 		set_current_state(TASK_INTERRUPTIBLE);
 		schedule_hrtimeout(&toggle_delay, HRTIMER_MODE_REL);

diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
index 60b0a15ed6e2..bbcf6f4152a3 100644
--- a/kernel/rcu/tree_nocb.h
+++ b/kernel/rcu/tree_nocb.h

@@ -1301,12 +1301,13 @@ int rcu_nocb_cpumask_update(const struct cpumask *cpumask, bool offload)
 	int err_cpu;
 	cpumask_var_t saved_nocb_mask;
 
+	lockdep_assert_cpus_held();
+
 	if (!alloc_cpumask_var(&saved_nocb_mask, GFP_KERNEL))
 		return -ENOMEM;
 
 	cpumask_copy(saved_nocb_mask, rcu_nocb_mask);
 
-	cpus_read_lock();
 	mutex_lock(&rcu_state.barrier_mutex);
 	for_each_cpu(cpu, cpumask) {
 		if (offload) {

@@ -1340,7 +1341,6 @@ int rcu_nocb_cpumask_update(const struct cpumask *cpumask, bool offload)
 	}
 
 	mutex_unlock(&rcu_state.barrier_mutex);
-	cpus_read_unlock();
 
 	free_cpumask_var(saved_nocb_mask);
 

-- 
2.39.3

Add a new rcu_nocb_enabled() helper to expose the rcu_nocb state
to other kernel subsystems like cpuset.  That will allow cpuset to
determine if RCU no-callback can be enabled on isolated CPUs within
isolated partitions. If so, the corresponding RCU functions can be
called to enable it when full CPU isolation is requested.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 include/linux/rcupdate.h |  6 ++++++
 kernel/rcu/tree_nocb.h   | 12 ++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index b649344075d2..976d55a3e523 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h

@@ -120,6 +120,12 @@ void rcu_init(void);
 extern int rcu_scheduler_active;
 void rcu_sched_clock_irq(int user);
 
+#ifdef CONFIG_RCU_NOCB_CPU
+int rcu_nocb_enabled(struct cpumask *out_mask);
+#else
+static inline int rcu_nocb_enabled(struct cpumask *out_mask) { return 0; }
+#endif
+
 #ifdef CONFIG_TASKS_RCU_GENERIC
 void rcu_init_tasks_generic(void);
 #else

diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
index bbcf6f4152a3..020a347ccd52 100644
--- a/kernel/rcu/tree_nocb.h
+++ b/kernel/rcu/tree_nocb.h

@@ -81,6 +81,18 @@ static int __init parse_rcu_nocb_poll(char *arg)
 }
 __setup("rcu_nocb_poll", parse_rcu_nocb_poll);
 
+/*
+ * Return the rcu_nocb state & optionally copy out rcu_nocb_mask.
+ */
+int rcu_nocb_enabled(struct cpumask *out_mask)
+{
+	if (!rcu_state.nocb_is_setup)
+		return 0;
+	if (out_mask)
+		cpumask_copy(out_mask, rcu_nocb_mask);
+	return 1;
+}
+
 /*
  * Don't bother bypassing ->cblist if the call_rcu() rate is low.
  * After all, the main point of bypassing is to avoid lock contention

-- 
2.39.3

This patch adds a new root only cpuset.cpus.isolation_full control file
for enabling or disabling full CPU isolation mode where additional CPU
isolation methods available to be used by cpuset will be turned on or
off for all the isolated CPUs within isolated partitions.

On write, cpuset.cpus.isolation_full accepts any integer. A zero value
will disable full CPU isolation while a non-zero value will enable it.
On read, cpuset.cpus.isolation_full will return either "0" (disabled)
or "1" (enabled) followed by a comma separated list of additional
CPU isolation methods that are enabled. The list of these available
isolation methods will depend on kernel configuration options used as
well as the presence of some pre-conditions for some of them.

This patch only provides the infrastructure code. The various isolation
methods will be added later on.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/cgroup/cpuset.c | 88 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index 0479af76a5dc..d1d4ce213979 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c

@@ -218,6 +218,30 @@ enum isolated_cpus_modifiers {
  */
 static cpumask_var_t	isolated_cpus;
 
+/*
+ * Enable full CPU isolation in isolated partitions, if set.
+ */
+static bool isolation_full;
+
+/*
+ * A flag indicating what cpuset full isolation modes can be enabled.
+ */
+static int isolation_flags;
+
+enum cpuset_isolation_types {
+	ISOL_TYPE_MAX,
+};
+
+static const char * const isolation_type_names[ISOL_TYPE_MAX] = {
+};
+
+/* Detect the cpuset isolation modes that can be enabled */
+static __init int set_isolation_flags(void)
+{
+	return 0;
+}
+late_initcall(set_isolation_flags);
+
 /* List of remote partition root children */
 static struct list_head remote_children;
 

@@ -1524,6 +1548,8 @@ static int partition_xcpus_del(int old_prs, struct cpuset *parent,
  * state and the whole isolated_cpus is passed. Repeated calls with the same
  * isolated_cpus will not cause further action other than a wasted mutex
  * lock/unlock.
+ *
+ * The other isolation modes will only be activated when isolation_full is set.
  */
 static int update_isolation_cpumasks(struct cpumask *mask, int modifier)
 {

@@ -1536,6 +1562,13 @@ static int update_isolation_cpumasks(struct cpumask *mask, int modifier)
 
 	err = workqueue_unbound_exclude_cpumask(isolated_cpus);
 	WARN_ON_ONCE(err);
+
+	if (!isolation_flags || !isolation_full)
+		return err;
+
+	if (WARN_ON_ONCE(cpumask_empty(mask)))
+		return -EINVAL;
+
 	return err;
 }
 

@@ -3514,6 +3547,7 @@ typedef enum {
 	FILE_EXCLUSIVE_CPULIST,
 	FILE_EFFECTIVE_XCPULIST,
 	FILE_ISOLATED_CPULIST,
+	FILE_ISOLATION_FULL,
 	FILE_CPU_EXCLUSIVE,
 	FILE_MEM_EXCLUSIVE,
 	FILE_MEM_HARDWALL,

@@ -3713,6 +3747,25 @@ static int cpuset_common_seq_show(struct seq_file *sf, void *v)
 	case FILE_ISOLATED_CPULIST:
 		seq_printf(sf, "%*pbl\n", cpumask_pr_args(isolated_cpus));
 		break;
+	case FILE_ISOLATION_FULL:
+		if (isolation_full) {
+			int i, cnt;
+
+			/* Also print the isolation modes that are enabled */
+			seq_puts(sf, "1");
+			for (i = cnt = 0; i < ISOL_TYPE_MAX; i++) {
+				if (!(isolation_flags & BIT(i)))
+					continue;
+
+				seq_printf(sf, "%c%s", cnt ? ',' : ' ',
+					   isolation_type_names[i]);
+				cnt++;
+			}
+			seq_puts(sf, "\n");
+		} else {
+			seq_puts(sf, "0\n");
+		}
+		break;
 	default:
 		ret = -EINVAL;
 	}

@@ -3833,6 +3886,33 @@ static ssize_t sched_partition_write(struct kernfs_open_file *of, char *buf,
 	return retval ?: nbytes;
 }
 
+/*
+ * cpuset_write_isolfull - enable/disable cpuset isolation full mode
+ */
+static int cpuset_write_isolfull(struct cgroup_subsys_state *css,
+				 struct cftype *cft, u64 val)
+{
+	struct cpuset *cs = css_cs(css);
+	int retval = 0;
+
+	cpus_read_lock();
+	mutex_lock(&cpuset_mutex);
+	if (!is_cpuset_online(cs)) {
+		retval = -ENODEV;
+	} else if (isolation_full != !!val) {
+		isolation_full = !!val;
+		if (!cpumask_empty(isolated_cpus)) {
+			int mod = isolation_full
+				  ? ISOL_CPUS_ADD : ISOL_CPUS_DELETE;
+
+			retval = update_isolation_cpumasks(isolated_cpus, mod);
+		}
+	}
+	mutex_unlock(&cpuset_mutex);
+	cpus_read_unlock();
+	return retval;
+}
+
 /*
  * for the common functions, 'private' gives the type of file
  */

@@ -4013,6 +4093,14 @@ static struct cftype dfl_files[] = {
 		.flags = CFTYPE_ONLY_ON_ROOT,
 	},
 
+	{
+		.name = "cpus.isolation_full",
+		.seq_show = cpuset_common_seq_show,
+		.write_u64 = cpuset_write_isolfull,
+		.private = FILE_ISOLATION_FULL,
+		.flags = CFTYPE_ONLY_ON_ROOT,
+	},
+
 	{ }	/* terminate */
 };
 

-- 
2.39.3

The patch adds RCU no-callback isolation mode dynamically to isolated
CPUs within isolated partitions when the full CPU isolation mode is
enabled. This isolation feature will only be available for use by cpuset
if the "rcu_nocb" boot command line option is specified in the kernel
command line with or without the optional CPU list argument.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/cgroup/cpuset.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index d1d4ce213979..40bbb0a9cb84 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c

@@ -218,6 +218,11 @@ enum isolated_cpus_modifiers {
  */
 static cpumask_var_t	isolated_cpus;
 
+/*
+ * rcu_nocb_mask set up at boot time.
+ */
+static cpumask_var_t	rcu_nocb_mask_preset;
+
 /*
  * Enable full CPU isolation in isolated partitions, if set.
  */

@@ -229,15 +234,26 @@ static bool isolation_full;
 static int isolation_flags;
 
 enum cpuset_isolation_types {
+	ISOL_TYPE_RCU,	/* RCU no-callback CPU mode */
 	ISOL_TYPE_MAX,
 };
 
+enum cpuset_isolation_flags {
+	ISOL_FLAG_RCU = BIT(ISOL_TYPE_RCU),
+};
+
 static const char * const isolation_type_names[ISOL_TYPE_MAX] = {
+	[ISOL_TYPE_RCU] = "rcu_nocbs",
 };
 
 /* Detect the cpuset isolation modes that can be enabled */
 static __init int set_isolation_flags(void)
 {
+	if (rcu_nocb_enabled(NULL)) {
+		BUG_ON(!zalloc_cpumask_var(&rcu_nocb_mask_preset, GFP_KERNEL));
+		(void)rcu_nocb_enabled(rcu_nocb_mask_preset);
+		isolation_flags |= ISOL_FLAG_RCU;
+	}
 	return 0;
 }
 late_initcall(set_isolation_flags);

@@ -1554,6 +1570,7 @@ static int partition_xcpus_del(int old_prs, struct cpuset *parent,
 static int update_isolation_cpumasks(struct cpumask *mask, int modifier)
 {
 	int err;
+	bool enable = (modifier == ISOL_CPUS_ADD);
 
 	lockdep_assert_cpus_held();
 

@@ -1569,6 +1586,25 @@ static int update_isolation_cpumasks(struct cpumask *mask, int modifier)
 	if (WARN_ON_ONCE(cpumask_empty(mask)))
 		return -EINVAL;
 
+	err = 0;
+	if (isolation_flags & ISOL_FLAG_RCU) {
+		/*
+		 * When disabling rcu_nocb, make sure that we don't touch any
+		 * CPUs that have already been set in rcu_nocb_mask_preset.
+		 */
+		if (!enable && cpumask_intersects(mask, rcu_nocb_mask_preset)) {
+			cpumask_var_t tmp_mask;
+
+			if (WARN_ON_ONCE(!alloc_cpumask_var(&tmp_mask, GFP_KERNEL)))
+				return -ENOMEM;
+			if (cpumask_andnot(tmp_mask, mask, rcu_nocb_mask_preset))
+				err = rcu_nocb_cpumask_update(tmp_mask, enable);
+			free_cpumask_var(tmp_mask);
+		} else {
+			err = rcu_nocb_cpumask_update(mask, enable);
+		}
+	}
+	WARN_ON_ONCE(err);
 	return err;
 }
 

-- 
2.39.3

The process of updating workqueue unbound cpumask to exclude isolated
CPUs in cpuset only requires the use of the aggregated isolated_cpus
cpumask.  Other types of CPU isolation, like the RCU no-callback CPU
mode, may require knowing more granular addition and deletion of isolated
CPUs. To enable these types of CPU isolation at run time, we need to
provide better tracking of the addition and deletion of isolated CPUs.

This patch adds a new isolated_cpus_modifier enum type for tracking
the addition and deletion of isolated CPUs as well as renaming
update_unbound_workqueue_cpumask() to update_isolation_cpumasks()
to accommodate additional CPU isolation modes in the future.

There is no functional change.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 kernel/cgroup/cpuset.c | 113 +++++++++++++++++++++++++----------------
 1 file changed, 69 insertions(+), 44 deletions(-)

diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
index dfbb16aca9f4..0479af76a5dc 100644
--- a/kernel/cgroup/cpuset.c
+++ b/kernel/cgroup/cpuset.c

@@ -206,6 +206,13 @@ struct cpuset {
  */
 static cpumask_var_t	subpartitions_cpus;
 
+/* Enum types for possible changes to the set of isolated CPUs */
+enum isolated_cpus_modifiers {
+	ISOL_CPUS_NONE = 0,
+	ISOL_CPUS_ADD,
+	ISOL_CPUS_DELETE,
+};
+
 /*
  * Exclusive CPUs in isolated partitions
  */

@@ -1446,14 +1453,14 @@ static void partition_xcpus_newstate(int old_prs, int new_prs, struct cpumask *x
  * @new_prs: new partition_root_state
  * @parent: parent cpuset
  * @xcpus: exclusive CPUs to be added
- * Return: true if isolated_cpus modified, false otherwise
+ * Return: isolated_cpus modifier
  *
  * Remote partition if parent == NULL
  */
-static bool partition_xcpus_add(int new_prs, struct cpuset *parent,
-				struct cpumask *xcpus)
+static int partition_xcpus_add(int new_prs, struct cpuset *parent,
+			       struct cpumask *xcpus)
 {
-	bool isolcpus_updated;
+	int icpus_mod = ISOL_CPUS_NONE;
 
 	WARN_ON_ONCE(new_prs < 0);
 	lockdep_assert_held(&callback_lock);

@@ -1464,13 +1471,14 @@ static bool partition_xcpus_add(int new_prs, struct cpuset *parent,
 	if (parent == &top_cpuset)
 		cpumask_or(subpartitions_cpus, subpartitions_cpus, xcpus);
 
-	isolcpus_updated = (new_prs != parent->partition_root_state);
-	if (isolcpus_updated)
+	if (new_prs != parent->partition_root_state) {
 		partition_xcpus_newstate(parent->partition_root_state, new_prs,
 					 xcpus);
-
+		icpus_mod = (new_prs == PRS_ISOLATED)
+			    ? ISOL_CPUS_ADD : ISOL_CPUS_DELETE;
+	}
 	cpumask_andnot(parent->effective_cpus, parent->effective_cpus, xcpus);
-	return isolcpus_updated;
+	return icpus_mod;
 }
 
 /*

@@ -1478,14 +1486,14 @@ static bool partition_xcpus_add(int new_prs, struct cpuset *parent,
  * @old_prs: old partition_root_state
  * @parent: parent cpuset
  * @xcpus: exclusive CPUs to be removed
- * Return: true if isolated_cpus modified, false otherwise
+ * Return: isolated_cpus modifier
  *
  * Remote partition if parent == NULL
  */
-static bool partition_xcpus_del(int old_prs, struct cpuset *parent,
+static int partition_xcpus_del(int old_prs, struct cpuset *parent,
 				struct cpumask *xcpus)
 {
-	bool isolcpus_updated;
+	int icpus_mod;
 
 	WARN_ON_ONCE(old_prs < 0);
 	lockdep_assert_held(&callback_lock);

@@ -1495,27 +1503,40 @@ static bool partition_xcpus_del(int old_prs, struct cpuset *parent,
 	if (parent == &top_cpuset)
 		cpumask_andnot(subpartitions_cpus, subpartitions_cpus, xcpus);
 
-	isolcpus_updated = (old_prs != parent->partition_root_state);
-	if (isolcpus_updated)
+	if (old_prs != parent->partition_root_state) {
 		partition_xcpus_newstate(old_prs, parent->partition_root_state,
 					 xcpus);
-
+		icpus_mod = (old_prs == PRS_ISOLATED)
+			    ? ISOL_CPUS_DELETE : ISOL_CPUS_ADD;
+	}
 	cpumask_and(xcpus, xcpus, cpu_active_mask);
 	cpumask_or(parent->effective_cpus, parent->effective_cpus, xcpus);
-	return isolcpus_updated;
+	return icpus_mod;
 }
 
-static void update_unbound_workqueue_cpumask(bool isolcpus_updated)
+/**
+ * update_isolation_cpumasks - Add or remove CPUs to/from full isolation state
+ * @mask: cpumask of the CPUs to be added or removed
+ * @modifier: enum isolated_cpus_modifiers
+ * Return: 0 if successful, error code otherwise
+ *
+ * Workqueue unbound cpumask update is applied irrespective of isolation_full
+ * state and the whole isolated_cpus is passed. Repeated calls with the same
+ * isolated_cpus will not cause further action other than a wasted mutex
+ * lock/unlock.
+ */
+static int update_isolation_cpumasks(struct cpumask *mask, int modifier)
 {
-	int ret;
+	int err;
 
 	lockdep_assert_cpus_held();
 
-	if (!isolcpus_updated)
-		return;
+	if (!modifier)
+		return 0;	/* No change in isolated CPUs */
 
-	ret = workqueue_unbound_exclude_cpumask(isolated_cpus);
-	WARN_ON_ONCE(ret < 0);
+	err = workqueue_unbound_exclude_cpumask(isolated_cpus);
+	WARN_ON_ONCE(err);
+	return err;
 }
 
 /**

@@ -1577,7 +1598,7 @@ static inline bool is_local_partition(struct cpuset *cs)
 static int remote_partition_enable(struct cpuset *cs, int new_prs,
 				   struct tmpmasks *tmp)
 {
-	bool isolcpus_updated;
+	int icpus_mod;
 
 	/*
 	 * The user must have sysadmin privilege.

@@ -1600,7 +1621,7 @@ static int remote_partition_enable(struct cpuset *cs, int new_prs,
 		return 0;
 
 	spin_lock_irq(&callback_lock);
-	isolcpus_updated = partition_xcpus_add(new_prs, NULL, tmp->new_cpus);
+	icpus_mod = partition_xcpus_add(new_prs, NULL, tmp->new_cpus);
 	list_add(&cs->remote_sibling, &remote_children);
 	if (cs->use_parent_ecpus) {
 		struct cpuset *parent = parent_cs(cs);

@@ -1609,7 +1630,7 @@ static int remote_partition_enable(struct cpuset *cs, int new_prs,
 		parent->child_ecpus_count--;
 	}
 	spin_unlock_irq(&callback_lock);
-	update_unbound_workqueue_cpumask(isolcpus_updated);
+	update_isolation_cpumasks(tmp->new_cpus, icpus_mod);
 
 	/*
 	 * Proprogate changes in top_cpuset's effective_cpus down the hierarchy.

@@ -1630,7 +1651,7 @@ static int remote_partition_enable(struct cpuset *cs, int new_prs,
  */
 static void remote_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
 {
-	bool isolcpus_updated;
+	int icpus_mod;
 
 	compute_effective_exclusive_cpumask(cs, tmp->new_cpus);
 	WARN_ON_ONCE(!is_remote_partition(cs));

@@ -1638,14 +1659,14 @@ static void remote_partition_disable(struct cpuset *cs, struct tmpmasks *tmp)
 
 	spin_lock_irq(&callback_lock);
 	list_del_init(&cs->remote_sibling);
-	isolcpus_updated = partition_xcpus_del(cs->partition_root_state,
-					       NULL, tmp->new_cpus);
+	icpus_mod = partition_xcpus_del(cs->partition_root_state, NULL,
+					tmp->new_cpus);
 	cs->partition_root_state = -cs->partition_root_state;
 	if (!cs->prs_err)
 		cs->prs_err = PERR_INVCPUS;
 	reset_partition_data(cs);
 	spin_unlock_irq(&callback_lock);
-	update_unbound_workqueue_cpumask(isolcpus_updated);
+	update_isolation_cpumasks(tmp->new_cpus, icpus_mod);
 
 	/*
 	 * Proprogate changes in top_cpuset's effective_cpus down the hierarchy.

@@ -1668,7 +1689,8 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *newmask,
 {
 	bool adding, deleting;
 	int prs = cs->partition_root_state;
-	int isolcpus_updated = 0;
+	int icpus_add_mod = ISOL_CPUS_NONE;
+	int icpus_del_mod = ISOL_CPUS_NONE;
 
 	if (WARN_ON_ONCE(!is_remote_partition(cs)))
 		return;

@@ -1693,12 +1715,12 @@ static void remote_cpus_update(struct cpuset *cs, struct cpumask *newmask,
 
 	spin_lock_irq(&callback_lock);
 	if (adding)
-		isolcpus_updated += partition_xcpus_add(prs, NULL, tmp->addmask);
+		icpus_add_mod = partition_xcpus_add(prs, NULL, tmp->addmask);
 	if (deleting)
-		isolcpus_updated += partition_xcpus_del(prs, NULL, tmp->delmask);
+		icpus_del_mod = partition_xcpus_del(prs, NULL, tmp->delmask);
 	spin_unlock_irq(&callback_lock);
-	update_unbound_workqueue_cpumask(isolcpus_updated);
-
+	update_isolation_cpumasks(tmp->addmask, icpus_add_mod);
+	update_isolation_cpumasks(tmp->delmask, icpus_del_mod);
 	/*
 	 * Proprogate changes in top_cpuset's effective_cpus down the hierarchy.
 	 */

@@ -1819,7 +1841,8 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 	int part_error = PERR_NONE;	/* Partition error? */
 	int subparts_delta = 0;
 	struct cpumask *xcpus;		/* cs effective_xcpus */
-	int isolcpus_updated = 0;
+	int icpus_add_mod = ISOL_CPUS_NONE;
+	int icpus_del_mod = ISOL_CPUS_NONE;
 	bool nocpu;
 
 	lockdep_assert_held(&cpuset_mutex);

@@ -2052,22 +2075,23 @@ static int update_parent_effective_cpumask(struct cpuset *cs, int cmd,
 			cs->nr_subparts = 0;
 	}
 	/*
-	 * Adding to parent's effective_cpus means deletion CPUs from cs
+	 * Adding to parent's effective_cpus means deleting CPUs from cs
 	 * and vice versa.
 	 */
 	if (adding)
-		isolcpus_updated += partition_xcpus_del(old_prs, parent,
-							tmp->addmask);
+		icpus_add_mod = partition_xcpus_del(old_prs, parent,
+						    tmp->addmask);
 	if (deleting)
-		isolcpus_updated += partition_xcpus_add(new_prs, parent,
-							tmp->delmask);
+		icpus_del_mod = partition_xcpus_add(new_prs, parent,
+						    tmp->delmask);
 
 	if (is_partition_valid(parent)) {
 		parent->nr_subparts += subparts_delta;
 		WARN_ON_ONCE(parent->nr_subparts < 0);
 	}
 	spin_unlock_irq(&callback_lock);
-	update_unbound_workqueue_cpumask(isolcpus_updated);
+	update_isolation_cpumasks(tmp->addmask, icpus_add_mod);
+	update_isolation_cpumasks(tmp->delmask, icpus_del_mod);
 
 	if ((old_prs != new_prs) && (cmd == partcmd_update))
 		update_partition_exclusive(cs, new_prs);

@@ -3044,7 +3068,7 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 	int err = PERR_NONE, old_prs = cs->partition_root_state;
 	struct cpuset *parent = parent_cs(cs);
 	struct tmpmasks tmpmask;
-	bool new_xcpus_state = false;
+	int icpus_mod = ISOL_CPUS_NONE;
 
 	if (old_prs == new_prs)
 		return 0;

@@ -3096,7 +3120,8 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 		/*
 		 * A change in load balance state only, no change in cpumasks.
 		 */
-		new_xcpus_state = true;
+		icpus_mod = (new_prs == PRS_ISOLATED)
+			    ? ISOL_CPUS_ADD : ISOL_CPUS_DELETE;
 	} else {
 		/*
 		 * Switching back to member is always allowed even if it

@@ -3128,10 +3153,10 @@ static int update_prstate(struct cpuset *cs, int new_prs)
 	WRITE_ONCE(cs->prs_err, err);
 	if (!is_partition_valid(cs))
 		reset_partition_data(cs);
-	else if (new_xcpus_state)
+	else if (icpus_mod)
 		partition_xcpus_newstate(old_prs, new_prs, cs->effective_xcpus);
 	spin_unlock_irq(&callback_lock);
-	update_unbound_workqueue_cpumask(new_xcpus_state);
+	update_isolation_cpumasks(cs->effective_xcpus, icpus_mod);
 
 	/* Force update if switching back to member */
 	update_cpumasks_hier(cs, &tmpmask, !new_prs ? HIER_CHECKALL : 0);

-- 
2.39.3

Document the new cpuset.cpus.isolation_full control file. Currently only
the rcu_nocbs flag is supported, but more will be added in the future.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 Documentation/admin-guide/cgroup-v2.rst | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/Documentation/admin-guide/cgroup-v2.rst b/Documentation/admin-guide/cgroup-v2.rst
index 17e6e9565156..bbd066838f93 100644
--- a/Documentation/admin-guide/cgroup-v2.rst
+++ b/Documentation/admin-guide/cgroup-v2.rst

@@ -2352,6 +2352,30 @@ Cpuset Interface Files
 	isolated partitions. It will be empty if no isolated partition
 	is created.
 
+  cpuset.cpus.isolation_full
+	A read multiple values and write single value file which exists
+	on root cgroup only.
+
+	This file shows the state of full CPU isolation mode for isolated
+	CPUs in isolated partitions.  It shows either "0' if full CPU
+	isolation mode is disabled, or "1" followed by a comma-separated
+	list of additional CPU isolation flags that are enabled.
+	The currently supported CPU isolation flag is:
+
+	  rcu_nocbs
+		RCU no-callback CPU mode, which prevents such CPUs'
+		callbacks from being invoked in softirq context.
+		Invocation of such CPUs' RCU callbacks will instead be
+		offloaded to "rcuox/N" kthreads created for that purpose.
+		It is similar in functionality to the "rcu_nocbs"
+		boot command line option, but for dynamically created
+		isolated CPUs in isolated partitions.  This flag can
+		only be enabled if such a "rcu_nocbs" option is present
+		in the boot command line of the running kernel.
+
+	Full CPU isolation mode is enabled by writing a non-zero value
+	to this file and disabled by writing a zero value to it.
+
   cpuset.cpus.partition
 	A read-write single value file which exists on non-root
 	cpuset-enabled cgroups.  This flag is owned by the parent cgroup

-- 
2.39.3

Add a new "-F" option to cpuset.cpus.isolation_full to enable
cpuset.cpus.isolation_full for trying out the effect of enabling
full CPU isolation.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 .../selftests/cgroup/test_cpuset_prs.sh       | 23 ++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/cgroup/test_cpuset_prs.sh b/tools/testing/selftests/cgroup/test_cpuset_prs.sh
index b5eb1be2248c..2a8f0cb8d252 100755
--- a/tools/testing/selftests/cgroup/test_cpuset_prs.sh
+++ b/tools/testing/selftests/cgroup/test_cpuset_prs.sh

@@ -32,6 +32,7 @@ NR_CPUS=$(lscpu | grep "^CPU(s):" | sed -e "s/.*:[[:space:]]*//")
 PROG=$1
 VERBOSE=0
 DELAY_FACTOR=1
+ISOLATION_FULL=
 SCHED_DEBUG=
 while [[ "$1" = -* ]]
 do

@@ -44,7 +45,10 @@ do
 		-d) DELAY_FACTOR=$2
 		    shift
 		    ;;
-		*)  echo "Usage: $PROG [-v] [-d <delay-factor>"
+		-F) ISOLATION_FULL=1
+		    shift
+		    ;;
+		*)  echo "Usage: $PROG [-v] [-d <delay-factor>] [-F]"
 		    exit
 		    ;;
 	esac

@@ -108,6 +112,22 @@ console_msg()
 	pause 0.01
 }
 
+setup_isolation_full()
+{
+	ISOL_FULL=${CGROUP2}/cpuset.cpus.isolation_full
+	if [[ -n "$ISOLATION_FULL" ]]
+	then
+		echo 1 > $ISOL_FULL
+		set -- $(cat $ISOL_FULL)
+		ISOLATION_FLAGS=$2
+		[[ $VERBOSE -gt 0 ]] && {
+			echo "Full CPU isolation flags: $ISOLATION_FLAGS"
+		}
+	else
+		echo 0 > $ISOL_FULL
+	fi
+}
+
 test_partition()
 {
 	EXPECTED_VAL=$1

@@ -930,6 +950,7 @@ test_inotify()
 }
 
 trap cleanup 0 2 3 6
+setup_isolation_full
 run_state_test TEST_MATRIX
 test_isolated
 test_inotify

-- 
2.39.3

Hello,

On Wed, Jan 17, 2024 at 11:35:03AM -0500, Waiman Long wrote:

The first 2 patches are adopted from Federic with minor twists to fix
merge conflicts and compilation issue. The rests are for implementing
the new cpuset.cpus.isolation_full interface which is essentially a flag
to globally enable or disable full CPU isolation on isolated partitions.

I think the interface is a bit premature. The cpuset partition feature is
already pretty restrictive and makes it really clear that it's to isolate
the CPUs. I think it'd be better to just enable all the isolation features
by default. If there are valid use cases which can't be served without
disabling some isolation features, we can worry about adding the interface
at that point.

Thanks.

-- 
tejun

On 1/17/24 12:07, Tejun Heo wrote:

Hello,

On Wed, Jan 17, 2024 at 11:35:03AM -0500, Waiman Long wrote:

quoted

The first 2 patches are adopted from Federic with minor twists to fix
merge conflicts and compilation issue. The rests are for implementing
the new cpuset.cpus.isolation_full interface which is essentially a flag
to globally enable or disable full CPU isolation on isolated partitions.

I think the interface is a bit premature. The cpuset partition feature is
already pretty restrictive and makes it really clear that it's to isolate
the CPUs. I think it'd be better to just enable all the isolation features
by default. If there are valid use cases which can't be served without
disabling some isolation features, we can worry about adding the interface
at that point.

My current thought is to make isolated partitions act like isolcpus=domain, additional CPU isolation capabilities are optional and can be turned on using isolation_full. However, I am fine with making all these turned on by default if it is the consensus.

Cheers,
Longman

On Wed, Jan 17, 2024 at 11:35:03AM -0500, Waiman Long wrote:

This patch series is based on the RFC patch from Frederic [1]. Instead
of offering RCU_NOCB as a separate option, it is now lumped into a
root-only cpuset.cpus.isolation_full flag that will enable all the
additional CPU isolation capabilities available for isolated partitions
if set. RCU_NOCB is just the first one to this party. Additional dynamic
CPU isolation capabilities will be added in the future.

The first 2 patches are adopted from Federic with minor twists to fix
merge conflicts and compilation issue. The rests are for implementing
the new cpuset.cpus.isolation_full interface which is essentially a flag
to globally enable or disable full CPU isolation on isolated partitions.
On read, it also shows the CPU isolation capabilities that are currently
enabled. RCU_NOCB requires that the rcu_nocbs option be present in
the kernel boot command line. Without that, the rcu_nocb functionality
cannot be enabled even if the isolation_full flag is set. So we allow
users to check the isolation_full file to verify that if the desired
CPU isolation capability is enabled or not.

Only sanity checking has been done so far. More testing, especially on
the RCU side, will be needed.

There has been some discussion of simplifying the (de-)offloading code
to handle only offline CPUs.  Along with some discussion of eliminating
the (de-)offloading capability altogehter.

We clearly should converge on the capability to be provided before
exposing this to userspace.  ;-)

							Thanx, Paul

[1] https://lore.kernel.org/lkml/20220525221055.1152307-1-frederic@kernel.org/

Frederic Weisbecker (2):
  rcu/nocb: Pass a cpumask instead of a single CPU to offload/deoffload
  rcu/nocb: Prepare to change nocb cpumask from CPU-hotplug protected
    cpuset caller

Waiman Long (6):
  rcu/no_cb: Add rcu_nocb_enabled() to expose the rcu_nocb state
  cgroup/cpuset: Better tracking of addition/deletion of isolated CPUs
  cgroup/cpuset: Add cpuset.cpus.isolation_full
  cgroup/cpuset: Enable dynamic rcu_nocb mode on isolated CPUs
  cgroup/cpuset: Document the new cpuset.cpus.isolation_full control
    file
  cgroup/cpuset: Update test_cpuset_prs.sh to handle
    cpuset.cpus.isolation_full

 Documentation/admin-guide/cgroup-v2.rst       |  24 ++
 include/linux/rcupdate.h                      |  15 +-
 kernel/cgroup/cpuset.c                        | 237 ++++++++++++++----
 kernel/rcu/rcutorture.c                       |   6 +-
 kernel/rcu/tree_nocb.h                        | 118 ++++++---
 .../selftests/cgroup/test_cpuset_prs.sh       |  23 +-
 6 files changed, 337 insertions(+), 86 deletions(-)

-- 
2.39.3

Hello Waiman.

On Wed, Jan 17, 2024 at 11:35:03AM -0500, Waiman Long [off-list ref] wrote:

This patch series is based on the RFC patch from Frederic [1]. Instead
of offering RCU_NOCB as a separate option, it is now lumped into a
root-only cpuset.cpus.isolation_full flag that will enable all the
additional CPU isolation capabilities available for isolated partitions
if set. RCU_NOCB is just the first one to this party. Additional dynamic
CPU isolation capabilities will be added in the future.

IIUC this is similar to what I suggested back in the day and you didn't
consider it [1]. Do I read this right that you've changed your mind?

(It's fine if you did, I'm only asking to follow the heading of cpuset
controller.)

Thanks,
Michal

[1] https://lore.kernel.org/r/58c87587-417b-1498-185f-1db6bb612c82@redhat.com/

On 1/22/24 10:07, Michal Koutný wrote:

Hello Waiman.

On Wed, Jan 17, 2024 at 11:35:03AM -0500, Waiman Long [off-list ref] wrote:

quoted

This patch series is based on the RFC patch from Frederic [1]. Instead
of offering RCU_NOCB as a separate option, it is now lumped into a
root-only cpuset.cpus.isolation_full flag that will enable all the
additional CPU isolation capabilities available for isolated partitions
if set. RCU_NOCB is just the first one to this party. Additional dynamic
CPU isolation capabilities will be added in the future.

IIUC this is similar to what I suggested back in the day and you didn't
consider it [1]. Do I read this right that you've changed your mind?

I didn't said that we were not going to do this at the time. It's just that more evaluation will need to be done before we are going to do this. I was also looking to see if there were use cases where such capabilities were needed. Now I am aware that such use cases do exist and we should start looking into it.

(It's fine if you did, I'm only asking to follow the heading of cpuset
controller.)

OK, the title of the cover-letter may be too specific. I will make it more general in the next version.

Cheers,
Longman

Le Wed, Jan 17, 2024 at 12:15:07PM -0500, Waiman Long a écrit :

On 1/17/24 12:07, Tejun Heo wrote:

quoted

Hello,

On Wed, Jan 17, 2024 at 11:35:03AM -0500, Waiman Long wrote:

quoted

The first 2 patches are adopted from Federic with minor twists to fix
merge conflicts and compilation issue. The rests are for implementing
the new cpuset.cpus.isolation_full interface which is essentially a flag
to globally enable or disable full CPU isolation on isolated partitions.

I think the interface is a bit premature. The cpuset partition feature is
already pretty restrictive and makes it really clear that it's to isolate
the CPUs. I think it'd be better to just enable all the isolation features
by default. If there are valid use cases which can't be served without
disabling some isolation features, we can worry about adding the interface
at that point.

My current thought is to make isolated partitions act like isolcpus=domain,
additional CPU isolation capabilities are optional and can be turned on
using isolation_full. However, I am fine with making all these turned on by
default if it is the consensus.

Right it was the consensus last time I tried. Along with the fact that mutating
this isolation_full set has to be done on offline CPUs to simplify the whole
picture.

So lemme try to summarize what needs to be done:

1) An all-isolation feature file (that is, all the HK_TYPE_* things) on/off for
  now. And if it ever proves needed, provide a way later for more finegrained
  tuning.

2) This file must only apply to offline CPUs because it avoids migrations and
  stuff.

3) I need to make RCU NOCB tunable only on offline CPUs, which isn't that much
   changes.

4) HK_TYPE_TIMER:
   * Wrt. timers in general, not much needs to be done, the CPUs are
     offline. But:
   * arch/x86/kvm/x86.c does something weird
   * drivers/char/random.c might need some care
   * watchdog needs to be (de-)activated
   
5) HK_TYPE_DOMAIN:
   * This one I fear is not mutable, this is isolcpus...

6) HK_TYPE_MANAGED_IRQ:
   * I prefer not to think about it :-)

7) HK_TYPE_TICK:
   * Maybe some tiny ticks internals to revisit, I'll check that.
   * There is a remote tick to take into consideration, but again the
     CPUs are offline so it shouldn't be too complicated.

8) HK_TYPE_WQ:
   * Fortunately we already have all the mutable interface in place.
     But we must make it live nicely with the sysfs workqueue affinity
     files.

9) HK_FLAG_SCHED:
   * Oops, this one is ignored by nohz_full/isolcpus, isn't it?
   Should be removed?

10) HK_TYPE_RCU:
    * That's point 3) and also some kthreads to affine, which leads us
     to the following in HK_TYPE_KTHREAD:

11) HK_FLAG_KTHREAD:
    * I'm guessing it's fine as long as isolation_full is also an
      isolated partition. Then unbound kthreads shouldn't run there.

12) HK_TYPE_MISC:
    * Should be fine as ILB isn't running on offline CPUs.

Thanks.

On Tue, Feb 06, 2024 at 01:56:23PM +0100, Frederic Weisbecker wrote:

Le Wed, Jan 17, 2024 at 12:15:07PM -0500, Waiman Long a écrit :

quoted

On 1/17/24 12:07, Tejun Heo wrote:

quoted

Hello,

On Wed, Jan 17, 2024 at 11:35:03AM -0500, Waiman Long wrote:

quoted

The first 2 patches are adopted from Federic with minor twists to fix
merge conflicts and compilation issue. The rests are for implementing
the new cpuset.cpus.isolation_full interface which is essentially a flag
to globally enable or disable full CPU isolation on isolated partitions.

I think the interface is a bit premature. The cpuset partition feature is
already pretty restrictive and makes it really clear that it's to isolate
the CPUs. I think it'd be better to just enable all the isolation features
by default. If there are valid use cases which can't be served without
disabling some isolation features, we can worry about adding the interface
at that point.

My current thought is to make isolated partitions act like isolcpus=domain,
additional CPU isolation capabilities are optional and can be turned on
using isolation_full. However, I am fine with making all these turned on by
default if it is the consensus.

Right it was the consensus last time I tried. Along with the fact that mutating
this isolation_full set has to be done on offline CPUs to simplify the whole
picture.

So lemme try to summarize what needs to be done:

1) An all-isolation feature file (that is, all the HK_TYPE_* things) on/off for
  now. And if it ever proves needed, provide a way later for more finegrained
  tuning.

2) This file must only apply to offline CPUs because it avoids migrations and
  stuff.

3) I need to make RCU NOCB tunable only on offline CPUs, which isn't that much
   changes.

4) HK_TYPE_TIMER:
   * Wrt. timers in general, not much needs to be done, the CPUs are
     offline. But:
   * arch/x86/kvm/x86.c does something weird
   * drivers/char/random.c might need some care
   * watchdog needs to be (de-)activated
   
5) HK_TYPE_DOMAIN:
   * This one I fear is not mutable, this is isolcpus...

Except for HK_TYPE_DOMAIN, i have never seen anyone use any of this
flags.

6) HK_TYPE_MANAGED_IRQ:
   * I prefer not to think about it :-)

7) HK_TYPE_TICK:
   * Maybe some tiny ticks internals to revisit, I'll check that.
   * There is a remote tick to take into consideration, but again the
     CPUs are offline so it shouldn't be too complicated.

8) HK_TYPE_WQ:
   * Fortunately we already have all the mutable interface in place.
     But we must make it live nicely with the sysfs workqueue affinity
     files.

9) HK_FLAG_SCHED:
   * Oops, this one is ignored by nohz_full/isolcpus, isn't it?
   Should be removed?

10) HK_TYPE_RCU:
    * That's point 3) and also some kthreads to affine, which leads us
     to the following in HK_TYPE_KTHREAD:

11) HK_FLAG_KTHREAD:
    * I'm guessing it's fine as long as isolation_full is also an
      isolated partition. Then unbound kthreads shouldn't run there.

12) HK_TYPE_MISC:
    * Should be fine as ILB isn't running on offline CPUs.

Thanks.

Le Tue, Feb 06, 2024 at 04:15:18PM -0300, Marcelo Tosatti a écrit :

On Tue, Feb 06, 2024 at 01:56:23PM +0100, Frederic Weisbecker wrote:

quoted

Le Wed, Jan 17, 2024 at 12:15:07PM -0500, Waiman Long a écrit :

quoted

On 1/17/24 12:07, Tejun Heo wrote:

quoted

Hello,

On Wed, Jan 17, 2024 at 11:35:03AM -0500, Waiman Long wrote:

quoted

The first 2 patches are adopted from Federic with minor twists to fix
merge conflicts and compilation issue. The rests are for implementing
the new cpuset.cpus.isolation_full interface which is essentially a flag
to globally enable or disable full CPU isolation on isolated partitions.

I think the interface is a bit premature. The cpuset partition feature is
already pretty restrictive and makes it really clear that it's to isolate
the CPUs. I think it'd be better to just enable all the isolation features
by default. If there are valid use cases which can't be served without
disabling some isolation features, we can worry about adding the interface
at that point.

My current thought is to make isolated partitions act like isolcpus=domain,
additional CPU isolation capabilities are optional and can be turned on
using isolation_full. However, I am fine with making all these turned on by
default if it is the consensus.

Right it was the consensus last time I tried. Along with the fact that mutating
this isolation_full set has to be done on offline CPUs to simplify the whole
picture.

So lemme try to summarize what needs to be done:

1) An all-isolation feature file (that is, all the HK_TYPE_* things) on/off for
  now. And if it ever proves needed, provide a way later for more finegrained
  tuning.

2) This file must only apply to offline CPUs because it avoids migrations and
  stuff.

3) I need to make RCU NOCB tunable only on offline CPUs, which isn't that much
   changes.

4) HK_TYPE_TIMER:
   * Wrt. timers in general, not much needs to be done, the CPUs are
     offline. But:
   * arch/x86/kvm/x86.c does something weird
   * drivers/char/random.c might need some care
   * watchdog needs to be (de-)activated
   
5) HK_TYPE_DOMAIN:
   * This one I fear is not mutable, this is isolcpus...

Except for HK_TYPE_DOMAIN, i have never seen anyone use any of this
flags.

HK_TYPE_DOMAIN is used by isolcpus=domain,....
HK_TYPE_MANAGED_IRQ is used by isolcpus=managed_irq,...

All the others (except HK_TYPE_SCHED) are used by nohz_full=

Thanks.

quoted

6) HK_TYPE_MANAGED_IRQ:
   * I prefer not to think about it :-)

7) HK_TYPE_TICK:
   * Maybe some tiny ticks internals to revisit, I'll check that.
   * There is a remote tick to take into consideration, but again the
     CPUs are offline so it shouldn't be too complicated.

8) HK_TYPE_WQ:
   * Fortunately we already have all the mutable interface in place.
     But we must make it live nicely with the sysfs workqueue affinity
     files.

9) HK_FLAG_SCHED:
   * Oops, this one is ignored by nohz_full/isolcpus, isn't it?
   Should be removed?

10) HK_TYPE_RCU:
    * That's point 3) and also some kthreads to affine, which leads us
     to the following in HK_TYPE_KTHREAD:

11) HK_FLAG_KTHREAD:
    * I'm guessing it's fine as long as isolation_full is also an
      isolated partition. Then unbound kthreads shouldn't run there.

12) HK_TYPE_MISC:
    * Should be fine as ILB isn't running on offline CPUs.

Thanks.

On Wed, Feb 07, 2024 at 03:47:46PM +0100, Frederic Weisbecker wrote:

Le Tue, Feb 06, 2024 at 04:15:18PM -0300, Marcelo Tosatti a écrit :

quoted

On Tue, Feb 06, 2024 at 01:56:23PM +0100, Frederic Weisbecker wrote:

quoted

Le Wed, Jan 17, 2024 at 12:15:07PM -0500, Waiman Long a écrit :

quoted

On 1/17/24 12:07, Tejun Heo wrote:

quoted

Hello,

On Wed, Jan 17, 2024 at 11:35:03AM -0500, Waiman Long wrote:

quoted

The first 2 patches are adopted from Federic with minor twists to fix
merge conflicts and compilation issue. The rests are for implementing
the new cpuset.cpus.isolation_full interface which is essentially a flag
to globally enable or disable full CPU isolation on isolated partitions.

I think the interface is a bit premature. The cpuset partition feature is
already pretty restrictive and makes it really clear that it's to isolate
the CPUs. I think it'd be better to just enable all the isolation features
by default. If there are valid use cases which can't be served without
disabling some isolation features, we can worry about adding the interface
at that point.

My current thought is to make isolated partitions act like isolcpus=domain,
additional CPU isolation capabilities are optional and can be turned on
using isolation_full. However, I am fine with making all these turned on by
default if it is the consensus.

Right it was the consensus last time I tried. Along with the fact that mutating
this isolation_full set has to be done on offline CPUs to simplify the whole
picture.

So lemme try to summarize what needs to be done:

1) An all-isolation feature file (that is, all the HK_TYPE_* things) on/off for
  now. And if it ever proves needed, provide a way later for more finegrained
  tuning.

2) This file must only apply to offline CPUs because it avoids migrations and
  stuff.

3) I need to make RCU NOCB tunable only on offline CPUs, which isn't that much
   changes.

4) HK_TYPE_TIMER:
   * Wrt. timers in general, not much needs to be done, the CPUs are
     offline. But:
   * arch/x86/kvm/x86.c does something weird
   * drivers/char/random.c might need some care
   * watchdog needs to be (de-)activated
   
5) HK_TYPE_DOMAIN:
   * This one I fear is not mutable, this is isolcpus...

Except for HK_TYPE_DOMAIN, i have never seen anyone use any of this
flags.

HK_TYPE_DOMAIN is used by isolcpus=domain,....

HK_TYPE_MANAGED_IRQ is used by isolcpus=managed_irq,...

All the others (except HK_TYPE_SCHED) are used by nohz_full=

I mean i've never seen any use of the individual flags being set.

You either want full isolation (nohz_full and all the flags together,
except for HK_TYPE_DOMAIN which is sometimes enabled/disabled), or not.

So why not group them all together ?

Do you know of any separate uses of these flags (except for
HK_TYPE_DOMAIN).

Thanks.

quoted

quoted

6) HK_TYPE_MANAGED_IRQ:
   * I prefer not to think about it :-)

7) HK_TYPE_TICK:
   * Maybe some tiny ticks internals to revisit, I'll check that.
   * There is a remote tick to take into consideration, but again the
     CPUs are offline so it shouldn't be too complicated.

8) HK_TYPE_WQ:
   * Fortunately we already have all the mutable interface in place.
     But we must make it live nicely with the sysfs workqueue affinity
     files.

9) HK_FLAG_SCHED:
   * Oops, this one is ignored by nohz_full/isolcpus, isn't it?
   Should be removed?

10) HK_TYPE_RCU:
    * That's point 3) and also some kthreads to affine, which leads us
     to the following in HK_TYPE_KTHREAD:

11) HK_FLAG_KTHREAD:
    * I'm guessing it's fine as long as isolation_full is also an
      isolated partition. Then unbound kthreads shouldn't run there.

12) HK_TYPE_MISC:
    * Should be fine as ILB isn't running on offline CPUs.

Thanks.

On 2/6/24 07:56, Frederic Weisbecker wrote:

Le Wed, Jan 17, 2024 at 12:15:07PM -0500, Waiman Long a écrit :

quoted

On 1/17/24 12:07, Tejun Heo wrote:

quoted

Hello,

On Wed, Jan 17, 2024 at 11:35:03AM -0500, Waiman Long wrote:

quoted

The first 2 patches are adopted from Federic with minor twists to fix
merge conflicts and compilation issue. The rests are for implementing
the new cpuset.cpus.isolation_full interface which is essentially a flag
to globally enable or disable full CPU isolation on isolated partitions.

I think the interface is a bit premature. The cpuset partition feature is
already pretty restrictive and makes it really clear that it's to isolate
the CPUs. I think it'd be better to just enable all the isolation features
by default. If there are valid use cases which can't be served without
disabling some isolation features, we can worry about adding the interface
at that point.

My current thought is to make isolated partitions act like isolcpus=domain,
additional CPU isolation capabilities are optional and can be turned on
using isolation_full. However, I am fine with making all these turned on by
default if it is the consensus.

Right it was the consensus last time I tried. Along with the fact that mutating
this isolation_full set has to be done on offline CPUs to simplify the whole
picture.

So lemme try to summarize what needs to be done:

1) An all-isolation feature file (that is, all the HK_TYPE_* things) on/off for
   now. And if it ever proves needed, provide a way later for more finegrained
   tuning.

That is more or less the current plan. As detailed below, HK_TYPE_DOMAIN & HK_TYPE_WQ isolation are included in the isolated partitions by default. I am also thinking about including other relatively cheap isolation flags by default. The expensive ones will have to be enabled via isolation_full.

2) This file must only apply to offline CPUs because it avoids migrations and
   stuff.

Well, the process of first moving the CPUs offline first is rather expensive. I won't mind doing some partial offlining based on the existing set of teardown and bringup callbacks, but I would try to avoid fully offlining the CPUs first.

3) I need to make RCU NOCB tunable only on offline CPUs, which isn't that much
    changes.

4) HK_TYPE_TIMER:
    * Wrt. timers in general, not much needs to be done, the CPUs are
      offline. But:
    * arch/x86/kvm/x86.c does something weird
    * drivers/char/random.c might need some care
    * watchdog needs to be (de-)activated
    
5) HK_TYPE_DOMAIN:
    * This one I fear is not mutable, this is isolcpus...

HK_TYPE_DOMAIN is already available via the current cpuset isolated partition functionality. What I am currently doing is to extend that to other HK_TYPE* flags.

6) HK_TYPE_MANAGED_IRQ:
    * I prefer not to think about it :-)

7) HK_TYPE_TICK:
    * Maybe some tiny ticks internals to revisit, I'll check that.
    * There is a remote tick to take into consideration, but again the
      CPUs are offline so it shouldn't be too complicated.

8) HK_TYPE_WQ:
    * Fortunately we already have all the mutable interface in place.
      But we must make it live nicely with the sysfs workqueue affinity
      files.

HK_TYPE_WQ is basically done and it is going to work properly with the workqueue affinity sysfs files. From the workqueue of view, HK_TYPE_WQ is currently treated the same as HK_TYPE_DOMAIN.

9) HK_FLAG_SCHED:
    * Oops, this one is ignored by nohz_full/isolcpus, isn't it?
    Should be removed?

I don't think HK_FLAG_SCHED is being used at all. So I believe we should remove it to avoid confusion.

10) HK_TYPE_RCU:
     * That's point 3) and also some kthreads to affine, which leads us
      to the following in HK_TYPE_KTHREAD:

11) HK_FLAG_KTHREAD:
     * I'm guessing it's fine as long as isolation_full is also an
       isolated partition. Then unbound kthreads shouldn't run there.

Yes, isolation_full applies only to isolated partitions. It extends the amount of CPU isolation by enabling all the other CPU available isolation flags.

Cheers,
Longman

On 1/19/24 05:24, Paul E. McKenney wrote:

On Wed, Jan 17, 2024 at 11:35:03AM -0500, Waiman Long wrote:

quoted

This patch series is based on the RFC patch from Frederic [1]. Instead
of offering RCU_NOCB as a separate option, it is now lumped into a
root-only cpuset.cpus.isolation_full flag that will enable all the
additional CPU isolation capabilities available for isolated partitions
if set. RCU_NOCB is just the first one to this party. Additional dynamic
CPU isolation capabilities will be added in the future.

The first 2 patches are adopted from Federic with minor twists to fix
merge conflicts and compilation issue. The rests are for implementing
the new cpuset.cpus.isolation_full interface which is essentially a flag
to globally enable or disable full CPU isolation on isolated partitions.
On read, it also shows the CPU isolation capabilities that are currently
enabled. RCU_NOCB requires that the rcu_nocbs option be present in
the kernel boot command line. Without that, the rcu_nocb functionality
cannot be enabled even if the isolation_full flag is set. So we allow
users to check the isolation_full file to verify that if the desired
CPU isolation capability is enabled or not.

Only sanity checking has been done so far. More testing, especially on
the RCU side, will be needed.

There has been some discussion of simplifying the (de-)offloading code
to handle only offline CPUs.  Along with some discussion of eliminating
the (de-)offloading capability altogehter.

We clearly should converge on the capability to be provided before
exposing this to userspace.  ;-)

Would you mind giving me a pointer to the discussion of simplifying the de-offloading code to  handle only offline CPUs?

Thanks,
Longman