Properly fix sysfs_get_idlestate_count()
cover Properly fix sysfs_get_idlestate_count()
My previous commit 16b7c275c055 ("tools: cpupower: fix return checks for sysfs_get_idlestate_count()") was not correct. After looking
at the changelog for cpupower I noticed that Thomas had changed the return of sysfs_get_idlestate_count() to an unsigned int to simplify the code. The problem is really that both he (in his original change) and I (in my new change) missed the obvious that sysfs_get_idlestate_count()
can't return -ENODEV. It should just return 0 for "no c-states".
Patch 1 reverts my recent change and patch 2 fixes the problem correctly.
Sorry 'bout that Thomas. I should have caught that the the first time.
Prarit Bhargava (2):
Revert "tools: cpupower: fix return checks for
sysfs_get_idlestate_count()"
Fix no idle state information return value
tools/power/cpupower/utils/cpuidle-info.c | 8 ++++---- tools/power/cpupower/utils/helpers/sysfs.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-)
Signature · 1 line
1.7.9.3
PATCH 1/2 Revert "tools: cpupower: fix return checks for sysfs_get_idlestate_count()"
This reverts commit 16b7c275c055cc36218404b5d147be7f76575087.
My previous commit 16b7c275c055 ("tools: cpupower: fix return checks for sysfs_get_idlestate_count()") was not correct. After looking
at the changelog for cpupower I noticed that Thomas had changed the return of sysfs_get_idlestate_count() to an unsigned int to simplify the code. The problem is really that both he (in his original change) and I (in my new change) missed the obvious that sysfs_get_idlestate_count()
can't return -ENODEV. It should just return 0 for "no c-states".
--- tools/power/cpupower/utils/cpuidle-info.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
tools/power/cpupower/utils/cpuidle-info.c +4 -4
@@ -22,13 +22,13 @@static void cpuidle_cpu_output (unsigned int cpu,int verbose) { -int idlestates, idlestate; +unsigned int idlestates, idlestate;char *tmp;printf (_ ("Analyzing CPU %d:\n" ), cpu); idlestates =sysfs_get_idlestate_count (cpu); -if (idlestates <1 ) { +if (idlestates ==0 ) {printf (_ ("CPU %u: No idle states\n" ), cpu);return ; }
@@ -100,10 +100,10 @@ static void cpuidle_general_output(void)static void proc_cpuidle_cpu_output (unsigned int cpu) {long max_allowed_cstate =2000000000 ; -int cstate, cstates; +unsigned int cstate, cstates; cstates =sysfs_get_idlestate_count (cpu); -if (cstates <1 ) { +if (cstates ==0 ) {printf (_ ("CPU %u: No C-states info\n" ), cpu);return ; }
Signature · 1 line
1.7.9.3
PATCH 2/2 Fix no idle state information return value
In reply to Prarit Bhargavasysfs_get_idlestate_count() returns an unsigned int. Returning -ENODEV is not the right thing to do here, and in any case is handled the same way as if there are no states found.
--- tools/power/cpupower/utils/helpers/sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
tools/power/cpupower/utils/helpers/sysfs.c +1 -1
@@ -361,7 +361,7 @@ unsigned int sysfs_get_idlestate_count(unsigned int cpu)snprintf (file,SYSFS_PATH_MAX ,PATH_TO_CPU "cpuidle" );if (stat (file, &statbuf) !=0 || !S_ISDIR (statbuf.st_mode)) -return -ENODEV ; +return 0 ;snprintf (file,SYSFS_PATH_MAX ,PATH_TO_CPU "cpu%u/cpuidle/state0" , cpu);if (stat (file, &statbuf) !=0 || !S_ISDIR (statbuf.st_mode))
Signature · 1 line
1.7.9.3
Re: [PATCH 2/2] Fix no idle state information return value
Quoted 3 lines · Prarit Bhargava
sysfs_get_idlestate_count()returns an unsigned int. Returning-ENODEVis not the right thing to do here, and in any case is handled the same way as if there are no states found.
Yep, returning zero states instead of an error code makes a lot sense here.
Quoted 4 lines
Cc Thomas Renninger <[email protected]>Cc Rafael J. Wysocki <[email protected]>Signed-off-by Prarit Bhargava <[email protected]>
Thanks!
Thomas
Quoted 9 lines
--- tools/power/cpupower/utils/helpers/sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)(quoted diff) +0
-0
b/tools/power/cpupower/utils/helpers/sysfs.cindex 09afe5d..4e8fe2c 100644
tools/power/cpupower/utils/helpers/sysfs.c unsigned int sysfs_get_idlestate_count(unsigned int cpu) · line 366
Quoted 7 lines
tools/power/cpupower/utils/helpers/sysfs.c +1
-1@@ -361,7 +361,7 @@ unsigned int sysfs_get_idlestate_count(unsigned int cpu)snprintf (file,SYSFS_PATH_MAX ,PATH_TO_CPU "cpuidle" );if (stat (file, &statbuf) !=0 || !S_ISDIR (statbuf.st_mode)) -return -ENODEV ; +return 0 ; snprintf(file, SYSFS_PATH_MAX, PATH_TO_CPU "cpu%u/cpuidle/state0",
cpu);
Quoted 1 line
tools/power/cpupower/utils/helpers/sysfs.c +0
-0if (stat (file, &statbuf) !=0 || !S_ISDIR (statbuf.st_mode))
Quoted 16 lines · Prarit Bhargava
My previous commit
16b7c275c055("tools: cpupower: fix return checks forsysfs_get_idlestate_count()") was not correct. After looking
at the changelog for cpupower I noticed that Thomas had changed the return ofsysfs_get_idlestate_count()to an unsigned int to simplify the code. The problem is really that both he (in his original change) and I (in my new change) missed the obvious thatsysfs_get_idlestate_count()
can't return-ENODEV. It should just return 0 for "no c-states".Patch 1 reverts my recent change and patch 2 fixes the problem correctly.
Sorry 'bout that Thomas. I should have caught that the the first time.
Cc Thomas Renninger <[email protected]>Cc Rafael J. Wysocki <[email protected]>Signed-off-by Prarit Bhargava <[email protected]>
Quoted 8 lines
Prarit Bhargava (2):
Revert "tools: cpupower: fix return checks for
sysfs_get_idlestate_count()"
Fix no idle state information return valuetools/power/cpupower/utils/cpuidle-info.c | 8 ++++---- tools/power/cpupower/utils/helpers/sysfs.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-)
I'll queue up the revert, but I need an ACK from Thomas for the second patch.
Signature · 2 lines
I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center.