Re: [PATCH] sched/isolation: reconcile rcu_nocbs= and nohz_full=

From: Paul E. McKenney
Date: Thu Apr 22 2021 - 17:24:50 EST


On Mon, Apr 19, 2021 at 12:26:59AM -0400, Paul Gortmaker wrote:
> We have a mismatch between RCU and isolation -- in relation to what is
> considered the maximum valid CPU number.
>
> This matters because nohz_full= and rcu_nocbs= are joined at the hip; in
> fact the former will enforce the latter. So we don't want a CPU mask to
> be valid for one and denied for the other.
>
> The difference 1st appeared as of v4.15; further details are below.

I pulled this into -rcu for testing and further review.

If it should instead go through some other tree:

Acked-by: Paul E. McKenney <paulmck@xxxxxxxxxx>

> As it is confusing to anyone who isn't looking at the code regularly, a
> reminder is in order; three values exist here:
>
> CONFIG_NR_CPUS - compiled in maximum cap on number of CPUs supported.
> nr_cpu_ids - possible # of CPUs (typically reflects what ACPI says)
> cpus_present - actual number of present/detected/installed CPUs.
>
> For this example, I'll refer to NR_CPUS=64 from "make defconfig" and
> nr_cpu_ids=6 for ACPI reporting on a board that could run a six core,
> and present=4 for a quad that is physically in the socket. From dmesg:
>
> smpboot: Allowing 6 CPUs, 2 hotplug CPUs
> setup_percpu: NR_CPUS:64 nr_cpumask_bits:64 nr_cpu_ids:6 nr_node_ids:1
> rcu: RCU restricting CPUs from NR_CPUS=64 to nr_cpu_ids=6.
> smp: Brought up 1 node, 4 CPUs
>
> And from userspace, see:
>
> paul@trash:/sys/devices/system/cpu$ cat present
> 0-3
> paul@trash:/sys/devices/system/cpu$ cat possible
> 0-5
> paul@trash:/sys/devices/system/cpu$ cat kernel_max
> 63
>
> Everything is fine if we boot 5x5 for rcu/nohz:
>
> Command line: BOOT_IMAGE=/boot/bzImage nohz_full=2-5 rcu_nocbs=2-5 root=/dev/sda1 ro
> NO_HZ: Full dynticks CPUs: 2-5.
> rcu: Offload RCU callbacks from CPUs: 2-5.
>
> ..even though there is no CPU 4 or 5. Both RCU and nohz_full are OK.
> Now we push that > 6 but less than NR_CPU and with 15x15 we get:
>
> Command line: BOOT_IMAGE=/boot/bzImage rcu_nocbs=2-15 nohz_full=2-15 root=/dev/sda1 ro
> rcu: Note: kernel parameter 'rcu_nocbs=', 'nohz_full', or 'isolcpus=' contains nonexistent CPUs.
> rcu: Offload RCU callbacks from CPUs: 2-5.
>
> These are both functionally equivalent, as we are only changing flags on
> phantom CPUs that don't exist, but note the kernel interpretation changes.
> And worse, it only changes for one of the two - which is the problem.
>
> RCU doesn't care if you want to restrict the flags on phantom CPUs but
> clearly nohz_full does after this change from v4.15 (edb9382175c3):
>
> - if (cpulist_parse(str, non_housekeeping_mask) < 0) {
> - pr_warn("Housekeeping: Incorrect nohz_full cpumask\n");
> + err = cpulist_parse(str, non_housekeeping_mask);
> + if (err < 0 || cpumask_last(non_housekeeping_mask) >= nr_cpu_ids) {
> + pr_warn("Housekeeping: nohz_full= or isolcpus= incorrect CPU range\n");
>
> To be clear, the sanity check on "possible" (nr_cpu_ids) is new here.
>
> The goal was reasonable ; not wanting housekeeping to land on a
> not-possible CPU, but note two things:
>
> 1) this is an exclusion list, not an inclusion list; we are tracking
> non_housekeeping CPUs; not ones who are explicitly assigned housekeeping
>
> 2) we went one further in 9219565aa890 - ensuring that housekeeping was
> sanity checking against present and not just possible CPUs.
>
> To be clear, this means the check added in v4.15 is doubly redundant.
> And more importantly, overly strict/restrictive.
>
> We care now, because the bitmap boot arg parsing now knows that a value
> of "N" is NR_CPUS; the size of the bitmap, but the bitmap code doesn't
> know anything about the subtleties of our max/possible/present CPU
> specifics as outlined above.
>
> So drop the check added in v4.15 (edb9382175c3) and make RCU and
> nohz_full both in alignment again on NR_CPUS so "N" works for both,
> and then they can fall back to nr_cpu_ids internally just as before.
>
> Command line: BOOT_IMAGE=/boot/bzImage nohz_full=2-N rcu_nocbs=2-N root=/dev/sda1 ro
> NO_HZ: Full dynticks CPUs: 2-5.
> rcu: Offload RCU callbacks from CPUs: 2-5.
>
> As shown above, with this change, RCU and nohz_full are in sync, even
> with the use of the "N" placeholder. Same result is achieved with "15".
>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Paul E. McKenney <paulmck@xxxxxxxxxx>
> Cc: Frederic Weisbecker <frederic@xxxxxxxxxx>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx>
>
> diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
> index 5a6ea03f9882..7f06eaf12818 100644
> --- a/kernel/sched/isolation.c
> +++ b/kernel/sched/isolation.c
> @@ -81,11 +81,9 @@ static int __init housekeeping_setup(char *str, enum hk_flags flags)
> {
> cpumask_var_t non_housekeeping_mask;
> cpumask_var_t tmp;
> - int err;
>
> alloc_bootmem_cpumask_var(&non_housekeeping_mask);
> - err = cpulist_parse(str, non_housekeeping_mask);
> - if (err < 0 || cpumask_last(non_housekeeping_mask) >= nr_cpu_ids) {
> + if (cpulist_parse(str, non_housekeeping_mask) < 0) {
> pr_warn("Housekeeping: nohz_full= or isolcpus= incorrect CPU range\n");
> free_bootmem_cpumask_var(non_housekeeping_mask);
> return 0;
> --
> 2.25.1
>