diff mbox

cpupower tools: Fix error when running cpupower monitor

Message ID 1438582560-13352-1-git-send-email-shreyas@linux.vnet.ibm.com (mailing list archive)
State Accepted, archived
Delegated to: Rafael Wysocki
Headers show

Commit Message

Shreyas B. Prabhu Aug. 3, 2015, 6:16 a.m. UTC
get_cpu_topology() tries to get topology info from all cpus by reading
files in the topology sysfs dir. If a cpu is offlined, since it doesn't
have topology dir, this function fails and returns -1. This causes
functions relying on get_cpu_topology() to fail. For example-

$ cpupower monitor
Cannot read number of available processors

Fix this by skipping fetching topology info for offline cpus.

Signed-off-by: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com>
Reported-by: Pavaman Subramaniyam <pavsubra@linux.vnet.ibm.com>
---
 tools/power/cpupower/utils/helpers/topology.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Thomas Renninger Aug. 10, 2015, 12:28 p.m. UTC | #1
On Monday, August 03, 2015 11:46:00 AM Shreyas B. Prabhu wrote:
> get_cpu_topology() tries to get topology info from all cpus by reading
> files in the topology sysfs dir. If a cpu is offlined, since it doesn't
> have topology dir, this function fails and returns -1. This causes
> functions relying on get_cpu_topology() to fail. For example-
> 
> $ cpupower monitor
> Cannot read number of available processors
> 
> Fix this by skipping fetching topology info for offline cpus.

Looks fine.

Thanks!

Acked-by: Thomas Renninger <trenn@suse.de>


> 
> Signed-off-by: Shreyas B. Prabhu <shreyas@linux.vnet.ibm.com>
> Reported-by: Pavaman Subramaniyam <pavsubra@linux.vnet.ibm.com>
> ---
>  tools/power/cpupower/utils/helpers/topology.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/power/cpupower/utils/helpers/topology.c
> b/tools/power/cpupower/utils/helpers/topology.c index
> c13120af519b..cea398c176e7 100644
> --- a/tools/power/cpupower/utils/helpers/topology.c
> +++ b/tools/power/cpupower/utils/helpers/topology.c
> @@ -73,6 +73,8 @@ int get_cpu_topology(struct cpupower_topology *cpu_top)
>  	for (cpu = 0; cpu < cpus; cpu++) {
>  		cpu_top->core_info[cpu].cpu = cpu;
>  		cpu_top->core_info[cpu].is_online = sysfs_is_cpu_online(cpu);
> +		if (!cpu_top->core_info[cpu].is_online)
> +			continue;
>  		if(sysfs_topology_read_file(
>  			cpu,
>  			"physical_package_id",

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shreyas B. Prabhu Aug. 17, 2015, 7:52 a.m. UTC | #2
On 08/10/2015 05:58 PM, Thomas Renninger wrote:
> On Monday, August 03, 2015 11:46:00 AM Shreyas B. Prabhu wrote:
>> get_cpu_topology() tries to get topology info from all cpus by reading
>> files in the topology sysfs dir. If a cpu is offlined, since it doesn't
>> have topology dir, this function fails and returns -1. This causes
>> functions relying on get_cpu_topology() to fail. For example-
>>
>> $ cpupower monitor
>> Cannot read number of available processors
>>
>> Fix this by skipping fetching topology info for offline cpus.
> 
> Looks fine.
> 
> Thanks!
> 
> Acked-by: Thomas Renninger <trenn@suse.de>
> 

Thanks Thomas!
Rafael, can you please pick this patch?


Thanks,
Shreyas

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shreyas B. Prabhu Aug. 25, 2015, 11:59 a.m. UTC | #3
On 08/17/2015 01:22 PM, Shreyas B Prabhu wrote:
> 
> 
> On 08/10/2015 05:58 PM, Thomas Renninger wrote:
>> On Monday, August 03, 2015 11:46:00 AM Shreyas B. Prabhu wrote:
>>> get_cpu_topology() tries to get topology info from all cpus by reading
>>> files in the topology sysfs dir. If a cpu is offlined, since it doesn't
>>> have topology dir, this function fails and returns -1. This causes
>>> functions relying on get_cpu_topology() to fail. For example-
>>>
>>> $ cpupower monitor
>>> Cannot read number of available processors
>>>
>>> Fix this by skipping fetching topology info for offline cpus.
>>
>> Looks fine.
>>
>> Thanks!
>>
>> Acked-by: Thomas Renninger <trenn@suse.de>
>>
> 
> Thanks Thomas!
> Rafael, can you please pick this patch?
> 
> 


Hi Rafael,

If this patch looks good can you please pick this up?


Thanks,
Shreyas

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shreyas B. Prabhu Sept. 3, 2015, 6:21 a.m. UTC | #4
On 08/25/2015 05:29 PM, Shreyas B Prabhu wrote:
> 
> 
> On 08/17/2015 01:22 PM, Shreyas B Prabhu wrote:
>>
>>
>> On 08/10/2015 05:58 PM, Thomas Renninger wrote:
>>> On Monday, August 03, 2015 11:46:00 AM Shreyas B. Prabhu wrote:
>>>> get_cpu_topology() tries to get topology info from all cpus by reading
>>>> files in the topology sysfs dir. If a cpu is offlined, since it doesn't
>>>> have topology dir, this function fails and returns -1. This causes
>>>> functions relying on get_cpu_topology() to fail. For example-
>>>>
>>>> $ cpupower monitor
>>>> Cannot read number of available processors
>>>>
>>>> Fix this by skipping fetching topology info for offline cpus.
>>>
>>> Looks fine.
>>>
>>> Thanks!
>>>
>>> Acked-by: Thomas Renninger <trenn@suse.de>
>>>
>>
>> Thanks Thomas!
>> Rafael, can you please pick this patch?
>>
>>
> 
> 
> Hi Rafael,
> 
> If this patch looks good can you please pick this up?
> 
> 
> Thanks,
> Shreyas
> 

Hi Rafael,

If this patch looks good can you please pick this up?


Thanks,
Shreyas

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Shreyas B. Prabhu Sept. 4, 2015, 2:29 p.m. UTC | #5
>>
>> Hi Rafael,
>>
>> If this patch looks good can you please pick this up?
> 
> I picked it up last week, sorry for being silent about that.
> 
> It should be in the Linus' tree already.
> 

Thanks! Sorry I missed the fact that you had picked it last week.

Thanks,
Shreyas

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki Sept. 4, 2015, 2:51 p.m. UTC | #6
On Thursday, September 03, 2015 11:51:22 AM Shreyas B Prabhu wrote:
> 
> On 08/25/2015 05:29 PM, Shreyas B Prabhu wrote:
> > 
> > 
> > On 08/17/2015 01:22 PM, Shreyas B Prabhu wrote:
> >>
> >>
> >> On 08/10/2015 05:58 PM, Thomas Renninger wrote:
> >>> On Monday, August 03, 2015 11:46:00 AM Shreyas B. Prabhu wrote:
> >>>> get_cpu_topology() tries to get topology info from all cpus by reading
> >>>> files in the topology sysfs dir. If a cpu is offlined, since it doesn't
> >>>> have topology dir, this function fails and returns -1. This causes
> >>>> functions relying on get_cpu_topology() to fail. For example-
> >>>>
> >>>> $ cpupower monitor
> >>>> Cannot read number of available processors
> >>>>
> >>>> Fix this by skipping fetching topology info for offline cpus.
> >>>
> >>> Looks fine.
> >>>
> >>> Thanks!
> >>>
> >>> Acked-by: Thomas Renninger <trenn@suse.de>
> >>>
> >>
> >> Thanks Thomas!
> >> Rafael, can you please pick this patch?
> >>
> >>
> > 
> > 
> > Hi Rafael,
> > 
> > If this patch looks good can you please pick this up?
> > 
> > 
> > Thanks,
> > Shreyas
> > 
> 
> Hi Rafael,
> 
> If this patch looks good can you please pick this up?

I picked it up last week, sorry for being silent about that.

It should be in the Linus' tree already.

Thanks,
Rafael

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/tools/power/cpupower/utils/helpers/topology.c b/tools/power/cpupower/utils/helpers/topology.c
index c13120af519b..cea398c176e7 100644
--- a/tools/power/cpupower/utils/helpers/topology.c
+++ b/tools/power/cpupower/utils/helpers/topology.c
@@ -73,6 +73,8 @@  int get_cpu_topology(struct cpupower_topology *cpu_top)
 	for (cpu = 0; cpu < cpus; cpu++) {
 		cpu_top->core_info[cpu].cpu = cpu;
 		cpu_top->core_info[cpu].is_online = sysfs_is_cpu_online(cpu);
+		if (!cpu_top->core_info[cpu].is_online)
+			continue;
 		if(sysfs_topology_read_file(
 			cpu,
 			"physical_package_id",