Re: [tip:sched/core] sched/isolation: Require a present CPU in housekeeping mask

[Frederic Weisbecker]

On Fri, May 03, 2019 at 10:47:37AM -0700, tip-bot for Nicholas Piggin wrote:
> Commit-ID:  9219565aa89033a9cfdae788c1940473a1253d6c
> Gitweb:     https://git.kernel.org/tip/9219565aa89033a9cfdae788c1940473a1253d6c
> Author:     Nicholas Piggin <npiggin@xxxxxxxxx>
> AuthorDate: Thu, 11 Apr 2019 13:34:47 +1000
> Committer:  Ingo Molnar <mingo@xxxxxxxxxx>
> CommitDate: Fri, 3 May 2019 19:42:58 +0200
> 
> sched/isolation: Require a present CPU in housekeeping mask
> 
> During housekeeping mask setup, currently a possible CPU is required.
> That does not guarantee the CPU would be available at boot time, so
> check to ensure that at least one present CPU is in the mask.
> 
> Signed-off-by: Nicholas Piggin <npiggin@xxxxxxxxx>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx>
> Cc: Frederic Weisbecker <fweisbec@xxxxxxxxx>
> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Rafael J . Wysocki <rafael.j.wysocki@xxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: linuxppc-dev@xxxxxxxxxxxxxxxx
> Link: https://lkml.kernel.org/r/20190411033448.20842-5-npiggin@xxxxxxxxx
> Signed-off-by: Ingo Molnar <mingo@xxxxxxxxxx>
> ---
>  kernel/sched/isolation.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/sched/isolation.c b/kernel/sched/isolation.c
> index b02d148e7672..687302051a27 100644
> --- a/kernel/sched/isolation.c
> +++ b/kernel/sched/isolation.c
> @@ -65,6 +65,7 @@ void __init housekeeping_init(void)
>  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);
> @@ -75,16 +76,23 @@ static int __init housekeeping_setup(char *str, enum hk_flags flags)
>  		return 0;
>  	}
>  
> +	alloc_bootmem_cpumask_var(&tmp);
>  	if (!housekeeping_flags) {
>  		alloc_bootmem_cpumask_var(&housekeeping_mask);
>  		cpumask_andnot(housekeeping_mask,
>  			       cpu_possible_mask, non_housekeeping_mask);
> -		if (cpumask_empty(housekeeping_mask))
> +
> +		cpumask_andnot(tmp, cpu_present_mask, non_housekeeping_mask);
> +		if (cpumask_empty(tmp)) {
> +			pr_warn("Housekeeping: must include one present CPU, "
> +				"using boot CPU:%d\n", smp_processor_id());
>  			__cpumask_set_cpu(smp_processor_id(), housekeeping_mask);
> +			__cpumask_clear_cpu(smp_processor_id(), non_housekeeping_mask);

Ah that line (along with its twin below) is actually also a fix for the upstream code.
If the housekeeping mask is empty and we force the current one, we must make sure that
the current CPU is excluded from the nohz_full set. Ideally it should have come as a
separate patch but it's nice that you fixed it anyway.

Thanks!

Acked-by: Frederic Weisbecker <frederic@xxxxxxxxxx>

> +		}
>  	} else {
> -		cpumask_var_t tmp;
> -
> -		alloc_bootmem_cpumask_var(&tmp);
> +		cpumask_andnot(tmp, cpu_present_mask, non_housekeeping_mask);
> +		if (cpumask_empty(tmp))
> +			__cpumask_clear_cpu(smp_processor_id(), non_housekeeping_mask);
>  		cpumask_andnot(tmp, cpu_possible_mask, non_housekeeping_mask);
>  		if (!cpumask_equal(tmp, housekeeping_mask)) {
>  			pr_warn("Housekeeping: nohz_full= must match isolcpus=\n");
> @@ -92,8 +100,8 @@ static int __init housekeeping_setup(char *str, enum hk_flags flags)
>  			free_bootmem_cpumask_var(non_housekeeping_mask);
>  			return 0;
>  		}
> -		free_bootmem_cpumask_var(tmp);
>  	}
> +	free_bootmem_cpumask_var(tmp);
>  
>  	if ((flags & HK_FLAG_TICK) && !(housekeeping_flags & HK_FLAG_TICK)) {
>  		if (IS_ENABLED(CONFIG_NO_HZ_FULL)) {