diff mbox series

cpufreq:powernv: Fix init_chip_info initialization in numa=off

Message ID 20210726170758.61041-1-psampat@linux.ibm.com (mailing list archive)
State Not Applicable, archived
Headers show
Series cpufreq:powernv: Fix init_chip_info initialization in numa=off | expand

Commit Message

Pratik R. Sampat July 26, 2021, 5:07 p.m. UTC
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@vger.kernel.org
Fixes: 053819e0bf84 ("cpufreq: powernv: Handle throttling due to Pmax capping at chip level")
Signed-off-by: Pratik R. Sampat <psampat@linux.ibm.com>
Reported-by: Shirisha Ganta <shirisha.ganta1@ibm.com>
---
 drivers/cpufreq/powernv-cpufreq.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Comments

Gautham R Shenoy July 27, 2021, 6:16 a.m. UTC | #1
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@vger.kernel.org
> Fixes: 053819e0bf84 ("cpufreq: powernv: Handle throttling due to Pmax capping at chip level")
> Signed-off-by: Pratik R. Sampat <psampat@linux.ibm.com>
> Reported-by: Shirisha Ganta <shirisha.ganta1@ibm.com>
> ---
>  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@linux.vnet.ibm.com>



> +	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
>
Pratik R. Sampat July 27, 2021, 6:49 a.m. UTC | #2
On 27/07/21 11:46 am, Gautham R Shenoy wrote:
> 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@vger.kernel.org
>> Fixes: 053819e0bf84 ("cpufreq: powernv: Handle throttling due to Pmax capping at chip level")
>> Signed-off-by: Pratik R. Sampat <psampat@linux.ibm.com>
>> Reported-by: Shirisha Ganta <shirisha.ganta1@ibm.com>
>> ---
>>   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.

ack, I could #define the constant.

> Otherwise, the patch looks good to me.
>
> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
>
Thanks
Pratik

>
>> +	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
>>
diff mbox series

Patch

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);
+	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;