Re: [PATCH 1/2] sched/isolation: really align nohz_full with rcu_nocbs

[Paul E. McKenney]

On Mon, Dec 06, 2021 at 09:59:49AM -0500, Paul Gortmaker wrote:
> At the moment it is currently possible to sneak a core into nohz_full
> that lies between nr_possible and NR_CPUS - but you won't "see" it
> because cpumask_pr_args() implicitly hides anything above nr_cpu_ids.
> 
> This becomes a problem when the nohz_full CPU set doesn't contain at
> least one other valid nohz CPU - in which case we end up with the
> tick_nohz_full_running set and no tick core specified, which trips an
> endless sequence of WARN() and renders the machine unusable.
> 
> I inadvertently opened the door for this when fixing an overly
> restrictive nohz_full conditional in the below Fixes: commit - and then
> courtesy of my optimistic ACPI reporting nr_possible of 64 (the default
> Kconfig for NR_CPUS) and the not-so helpful implict filtering done by
> cpumask_pr_args, I unfortunately did not spot it during my testing.
> 
> So here, I don't rely on what was printed anymore, but code exactly what
> our restrictions should be in order to be aligned with rcu_nocbs - which
> was the original goal.  Since the checks lie in "__init" code it is largely
> free for us to do this anyway.
> 
> Building with NOHZ_FULL and NR_CPUS=128 on an otherwise defconfig, and
> booting with "rcu_nocbs=8-127 nohz_full=96-127" on the same 16 core T5500
> Dell machine now results in the following (only relevant lines shown):
> 
>  smpboot: Allowing 64 CPUs, 48 hotplug CPUs
>  setup_percpu: NR_CPUS:128 nr_cpumask_bits:128 nr_cpu_ids:64 nr_node_ids:2
>  housekeeping: kernel parameter 'nohz_full=' or 'isolcpus=' contains nonexistent CPUs.
>  housekeeping: kernel parameter 'nohz_full=' or 'isolcpus=' has no valid CPUs.
>  rcu:     RCU restricting CPUs from NR_CPUS=128 to nr_cpu_ids=64.
>  rcu:     Note: kernel parameter 'rcu_nocbs=', 'nohz_full', or 'isolcpus=' contains nonexistent CPUs.
>  rcu:     Offload RCU callbacks from CPUs: 8-63.
> 
> One can see both new housekeeping checks are triggered in the above.
> The same invalid boot arg combination would have previously resulted in
> an infinitely scrolling mix of WARN from all cores per tick on this box.
> 
> Fixes: 915a2bc3c6b7 ("sched/isolation: Reconcile rcu_nocbs= and nohz_full=")
> Cc: Paul E. McKenney <paulmck@xxxxxxxxxx>
> Cc: Frederic Weisbecker <fweisbec@xxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx>
> ---
>  kernel/sched/isolation.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
> index 7f06eaf12818..01abc8400d6c 100644
> --- a/kernel/sched/isolation.c
> +++ b/kernel/sched/isolation.c
> @@ -89,6 +89,18 @@ static int __init housekeeping_setup(char *str, enum hk_flags flags)
>  		return 0;
>  	}
>  
> +	if (!cpumask_subset(non_housekeeping_mask, cpu_possible_mask)) {
> +		pr_info("housekeeping: kernel parameter 'nohz_full=' or 'isolcpus=' contains nonexistent CPUs.\n");
> +		cpumask_and(non_housekeeping_mask, cpu_possible_mask,
> +			    non_housekeeping_mask);
> +	}
> +
> +	if (cpumask_empty(non_housekeeping_mask)) {
> +		pr_info("housekeeping: kernel parameter 'nohz_full=' or 'isolcpus=' has no valid CPUs.\n");
> +		free_bootmem_cpumask_var(non_housekeeping_mask);
> +		return 0;

If Frederic applies his rcu_nocbs work to nohz_full, it may some day be
valid to specify an empty nohz_full CPU mask.  Of course, it might well
be that warning in the meantime is a good thing, but I figured that I
should call attention to the possibility.

							Thanx, Paul

> +	}
> +
>  	alloc_bootmem_cpumask_var(&tmp);
>  	if (!housekeeping_flags) {
>  		alloc_bootmem_cpumask_var(&housekeeping_mask);
> -- 
> 2.17.1
>