Re: [PATCH] cpufreq:powernv: Fix init_chip_info initialization in numa=off

[Gautham R Shenoy]

On Mon, Jul 26, 2021 at 10:37:57PM +0530, Pratik R. Sampat wrote:
> In the numa=off kernel command-line configuration init_chip_info() loops
> around the number of chips and attempts to copy the cpumask of that node
> which is NULL for all iterations after the first chip.
> 
> Hence, store the cpu mask for each chip instead of derving cpumask from
> node while populating the "chips" struct array and copy that to the
> chips[i].mask
> 
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 053819e0bf84 ("cpufreq: powernv: Handle throttling due to Pmax capping at chip level")
> Signed-off-by: Pratik R. Sampat <psampat@xxxxxxxxxxxxx>
> Reported-by: Shirisha Ganta <shirisha.ganta1@xxxxxxx>
> ---
>  drivers/cpufreq/powernv-cpufreq.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/powernv-cpufreq.c b/drivers/cpufreq/powernv-cpufreq.c
> index 005600cef273..8ec10d9aed8f 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -1046,12 +1046,20 @@ static int init_chip_info(void)
>  	unsigned int *chip;
>  	unsigned int cpu, i;
>  	unsigned int prev_chip_id = UINT_MAX;
> +	cpumask_t *chip_cpu_mask;
>  	int ret = 0;
> 
>  	chip = kcalloc(num_possible_cpus(), sizeof(*chip), GFP_KERNEL);
>  	if (!chip)
>  		return -ENOMEM;
> 
> +	/* Allocate a chip cpu mask large enough to fit mask for all chips */
> +	chip_cpu_mask = kcalloc(32, sizeof(cpumask_t), GFP_KERNEL);

I suppose by 32 you mean the maximum number of chips possible. You
could use a #define for that.

Otherwise, the patch looks good to me.

Reviewed-by: Gautham R. Shenoy <ego@xxxxxxxxxxxxxxxxxx>



> +	if (!chip_cpu_mask) {
> +		ret = -ENOMEM;
> +		goto free_and_return;
> +	}
> +
>  	for_each_possible_cpu(cpu) {
>  		unsigned int id = cpu_to_chip_id(cpu);
> 
> @@ -1059,22 +1067,25 @@ static int init_chip_info(void)
>  			prev_chip_id = id;
>  			chip[nr_chips++] = id;
>  		}
> +		cpumask_set_cpu(cpu, &chip_cpu_mask[nr_chips-1]);
>  	}
> 
>  	chips = kcalloc(nr_chips, sizeof(struct chip), GFP_KERNEL);
>  	if (!chips) {
>  		ret = -ENOMEM;
> -		goto free_and_return;
> +		goto out_chip_cpu_mask;
>  	}
> 
>  	for (i = 0; i < nr_chips; i++) {
>  		chips[i].id = chip[i];
> -		cpumask_copy(&chips[i].mask, cpumask_of_node(chip[i]));
> +		cpumask_copy(&chips[i].mask, &chip_cpu_mask[i]);
>  		INIT_WORK(&chips[i].throttle, powernv_cpufreq_work_fn);
>  		for_each_cpu(cpu, &chips[i].mask)
>  			per_cpu(chip_info, cpu) =  &chips[i];
>  	}
> 
> +out_chip_cpu_mask:
> +	kfree(chip_cpu_mask);
>  free_and_return:
>  	kfree(chip);
>  	return ret;
> -- 
> 2.31.1
>