* [PATCH] cpufreq: powernv: Use node ID in init_chip_info
@ 2016-10-19 22:02 Emily Shaffer
2016-10-20 3:59 ` Viresh Kumar
0 siblings, 1 reply; 3+ messages in thread
From: Emily Shaffer @ 2016-10-19 22:02 UTC (permalink / raw)
To: benh, paulus, mpe, rjw, viresh.kumar; +Cc: linuxppc-dev, linux-pm
Fixed assumption that node_id==chip_id in powernv-cpufreq.c:init_chip_info;
explicitly use node ID where necessary.
Tested: All CPUs report in /sys/devices/system/cpu*/cpufreq/throttle_stats
Effort: platforms/arch-powerpc
Google-Bug-Id: 26979978
Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
Change-Id: I22eb626b32fbb8053b3bbb9c75e677c700d0c2fb
---
drivers/cpufreq/powernv-cpufreq.c | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/drivers/cpufreq/powernv-cpufreq.c
b/drivers/cpufreq/powernv-cpufreq.c
index d3ffde8..3750b58 100644
--- a/drivers/cpufreq/powernv-cpufreq.c
+++ b/drivers/cpufreq/powernv-cpufreq.c
@@ -911,32 +911,47 @@ static struct cpufreq_driver powernv_cpufreq_driver = {
static int init_chip_info(void)
{
- unsigned int chip[256];
+ int rc = 0;
unsigned int cpu, i;
unsigned int prev_chip_id = UINT_MAX;
+ unsigned int *chip, *node;
+
+ chip = kcalloc(num_possible_cpus(), sizeof(unsigned int), GFP_KERNEL);
+ node = kcalloc(num_possible_cpus(), sizeof(unsigned int), GFP_KERNEL);
+ if (!chip || !node) {
+ rc = -ENOMEM;
+ goto out;
+ }
for_each_possible_cpu(cpu) {
unsigned int id = cpu_to_chip_id(cpu);
if (prev_chip_id != id) {
prev_chip_id = id;
- chip[nr_chips++] = id;
+ node[nr_chips] = cpu_to_node(cpu);
+ chip[nr_chips] = id;
+ nr_chips++;
}
}
chips = kcalloc(nr_chips, sizeof(struct chip), GFP_KERNEL);
- if (!chips)
- return -ENOMEM;
+ if (!chips) {
+ rc = -ENOMEM;
+ goto out;
+ }
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, cpumask_of_node(node[i]));
INIT_WORK(&chips[i].throttle, powernv_cpufreq_work_fn);
for_each_cpu(cpu, &chips[i].mask)
per_cpu(chip_info, cpu) = &chips[i];
}
- return 0;
+out:
+ kfree(node);
+ kfree(chip);
+ return rc;
}
static inline void clean_chip_info(void)
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] cpufreq: powernv: Use node ID in init_chip_info
2016-10-19 22:02 [PATCH] cpufreq: powernv: Use node ID in init_chip_info Emily Shaffer
@ 2016-10-20 3:59 ` Viresh Kumar
2016-10-20 7:03 ` Shilpasri G Bhat
0 siblings, 1 reply; 3+ messages in thread
From: Viresh Kumar @ 2016-10-20 3:59 UTC (permalink / raw)
To: Emily Shaffer
Cc: benh, paulus, mpe, rjw, linuxppc-dev, linux-pm, shilpa.bhat,
akshay.adiga, ego
+ few IBM guys who have been working on this.
On 19-10-16, 15:02, Emily Shaffer wrote:
> Fixed assumption that node_id==chip_id in powernv-cpufreq.c:init_chip_info;
> explicitly use node ID where necessary.
>
> Tested: All CPUs report in /sys/devices/system/cpu*/cpufreq/throttle_stats
>
> Effort: platforms/arch-powerpc
> Google-Bug-Id: 26979978
Is this relevant upstream?
>
> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
> Change-Id: I22eb626b32fbb8053b3bbb9c75e677c700d0c2fb
Gerrit id isn't required for upstream..
> ---
> drivers/cpufreq/powernv-cpufreq.c | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/cpufreq/powernv-cpufreq.c
> b/drivers/cpufreq/powernv-cpufreq.c
> index d3ffde8..3750b58 100644
> --- a/drivers/cpufreq/powernv-cpufreq.c
> +++ b/drivers/cpufreq/powernv-cpufreq.c
> @@ -911,32 +911,47 @@ static struct cpufreq_driver powernv_cpufreq_driver = {
>
> static int init_chip_info(void)
> {
> - unsigned int chip[256];
> + int rc = 0;
> unsigned int cpu, i;
> unsigned int prev_chip_id = UINT_MAX;
> + unsigned int *chip, *node;
> +
> + chip = kcalloc(num_possible_cpus(), sizeof(unsigned int), GFP_KERNEL);
> + node = kcalloc(num_possible_cpus(), sizeof(unsigned int), GFP_KERNEL);
> + if (!chip || !node) {
> + rc = -ENOMEM;
> + goto out;
> + }
>
> for_each_possible_cpu(cpu) {
> unsigned int id = cpu_to_chip_id(cpu);
>
> if (prev_chip_id != id) {
> prev_chip_id = id;
> - chip[nr_chips++] = id;
> + node[nr_chips] = cpu_to_node(cpu);
> + chip[nr_chips] = id;
> + nr_chips++;
> }
> }
>
> chips = kcalloc(nr_chips, sizeof(struct chip), GFP_KERNEL);
> - if (!chips)
> - return -ENOMEM;
> + if (!chips) {
> + rc = -ENOMEM;
> + goto out;
> + }
>
> 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, cpumask_of_node(node[i]));
> INIT_WORK(&chips[i].throttle, powernv_cpufreq_work_fn);
> for_each_cpu(cpu, &chips[i].mask)
> per_cpu(chip_info, cpu) = &chips[i];
> }
>
> - return 0;
> +out:
> + kfree(node);
> + kfree(chip);
> + return rc;
> }
>
> static inline void clean_chip_info(void)
> --
> 2.8.0.rc3.226.g39d4020
--
viresh
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] cpufreq: powernv: Use node ID in init_chip_info
2016-10-20 3:59 ` Viresh Kumar
@ 2016-10-20 7:03 ` Shilpasri G Bhat
0 siblings, 0 replies; 3+ messages in thread
From: Shilpasri G Bhat @ 2016-10-20 7:03 UTC (permalink / raw)
To: Emily Shaffer
Cc: Viresh Kumar, benh, paulus, mpe, rjw, linuxppc-dev, linux-pm,
akshay.adiga, ego
Hi,
On 10/20/2016 09:29 AM, Viresh Kumar wrote:
> + few IBM guys who have been working on this.
>
> On 19-10-16, 15:02, Emily Shaffer wrote:
>> Fixed assumption that node_id==chip_id in powernv-cpufreq.c:init_chip_info;
>> explicitly use node ID where necessary.
Thanks for the bug fix. I agree that the node-ids should not be assumed to be
always be equal to chip-ids. But I think it would be better to get rid of
cpumask_of_node() as it has problems when the powernv-cpufreq driver is
initialized with offline cpus, like reported in the post below.
https://patchwork.kernel.org/patch/8887591/
(^^ This should also solve the node_id=chip_id problem)
Since throttle stats are common for all cpus in the chip, so we are better of
not using cpumask_of_node() instead define something like cpumask_of_chip()
where the driver doesn't have to compute chip cpumask.
Thanks and Regards,
Shilpa
>>
>> Tested: All CPUs report in /sys/devices/system/cpu*/cpufreq/throttle_stats
>>
>> Effort: platforms/arch-powerpc
>> Google-Bug-Id: 26979978
>
> Is this relevant upstream?
>
>>
>> Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
>> Change-Id: I22eb626b32fbb8053b3bbb9c75e677c700d0c2fb
>
> Gerrit id isn't required for upstream..
>
>> ---
>> drivers/cpufreq/powernv-cpufreq.c | 27 +++++++++++++++++++++------
>> 1 file changed, 21 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/cpufreq/powernv-cpufreq.c
>> b/drivers/cpufreq/powernv-cpufreq.c
>> index d3ffde8..3750b58 100644
>> --- a/drivers/cpufreq/powernv-cpufreq.c
>> +++ b/drivers/cpufreq/powernv-cpufreq.c
>> @@ -911,32 +911,47 @@ static struct cpufreq_driver powernv_cpufreq_driver = {
>>
>> static int init_chip_info(void)
>> {
>> - unsigned int chip[256];
>> + int rc = 0;
>> unsigned int cpu, i;
>> unsigned int prev_chip_id = UINT_MAX;
>> + unsigned int *chip, *node;
>> +
>> + chip = kcalloc(num_possible_cpus(), sizeof(unsigned int), GFP_KERNEL);
>> + node = kcalloc(num_possible_cpus(), sizeof(unsigned int), GFP_KERNEL);
>> + if (!chip || !node) {
>> + rc = -ENOMEM;
>> + goto out;
>> + }
>>
>> for_each_possible_cpu(cpu) {
>> unsigned int id = cpu_to_chip_id(cpu);
>>
>> if (prev_chip_id != id) {
>> prev_chip_id = id;
>> - chip[nr_chips++] = id;
>> + node[nr_chips] = cpu_to_node(cpu);
>> + chip[nr_chips] = id;
>> + nr_chips++;
>> }
>> }
>>
>> chips = kcalloc(nr_chips, sizeof(struct chip), GFP_KERNEL);
>> - if (!chips)
>> - return -ENOMEM;
>> + if (!chips) {
>> + rc = -ENOMEM;
>> + goto out;
>> + }
>>
>> 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, cpumask_of_node(node[i]));
>> INIT_WORK(&chips[i].throttle, powernv_cpufreq_work_fn);
>> for_each_cpu(cpu, &chips[i].mask)
>> per_cpu(chip_info, cpu) = &chips[i];
>> }
>>
>> - return 0;
>> +out:
>> + kfree(node);
>> + kfree(chip);
>> + return rc;
>> }
>>
>> static inline void clean_chip_info(void)
>> --
>> 2.8.0.rc3.226.g39d4020
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-10-20 7:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-19 22:02 [PATCH] cpufreq: powernv: Use node ID in init_chip_info Emily Shaffer
2016-10-20 3:59 ` Viresh Kumar
2016-10-20 7:03 ` Shilpasri G Bhat
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).