[1/2] cpufreq: exit if parsing the cpufreq directory fails

Message ID 20180706144106.56937-1-elliott@hpe.com
State Accepted
Headers show
Series
  • [1/2] cpufreq: exit if parsing the cpufreq directory fails
Related show

Commit Message

Elliott, Robert (Persistent Memory) July 6, 2018, 2:41 p.m.
Merge the loops parsing cpufreq directory content and setting the
governor to userspace so the function can exit if parsing fails.
Include the CPU number in the error messages.

That can happen if the kernel runs into this:
[   24.218674] cpufreq: cpufreq_online: ->get() failed
and does not create /sys/devices/system/cpu/cpuNN/cpufreq.

Signed-off-by: Robert Elliott <elliott@hpe.com>
---
 src/cpu/cpufreq/cpufreq.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

Comments

Alex Hung July 6, 2018, 10:15 p.m. | #1
On 2018-07-06 07:41 AM, Robert Elliott wrote:
> Merge the loops parsing cpufreq directory content and setting the
> governor to userspace so the function can exit if parsing fails.
> Include the CPU number in the error messages.
> 
> That can happen if the kernel runs into this:
> [   24.218674] cpufreq: cpufreq_online: ->get() failed
> and does not create /sys/devices/system/cpu/cpuNN/cpufreq.
> 
> Signed-off-by: Robert Elliott <elliott@hpe.com>
> ---
>   src/cpu/cpufreq/cpufreq.c | 21 ++++++++++++++-------
>   1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/src/cpu/cpufreq/cpufreq.c b/src/cpu/cpufreq/cpufreq.c
> index d1e50cdf..a65b9d08 100644
> --- a/src/cpu/cpufreq/cpufreq.c
> +++ b/src/cpu/cpufreq/cpufreq.c
> @@ -834,18 +834,25 @@ static int cpufreq_init(fwts_framework *fw)
>   	num_cpus = scandir(FWTS_CPU_PATH, &dirs, is_cpu_dir, versionsort);
>   	cpus = calloc(num_cpus, sizeof(*cpus));
>   
> -	for (i = 0; i < num_cpus; i++)
> -		parse_cpu_info(fw, &cpus[i], dirs[i]);
> -
>   	/* all test require a userspace governor */
>   	for (i = 0; i < num_cpus; i++) {
> -		int rc = cpu_set_governor(fw, &cpus[i], "userspace");
> +		int rc;
>   
> +		rc = parse_cpu_info(fw, &cpus[i], dirs[i]);
>   		if (rc != FWTS_OK) {
> -			fwts_log_warning(fw, "Failed to initialize cpufreq "
> -					"to set CPU speed");
> +			fwts_log_warning(fw,
> +				"Failed to parse cpufreq for CPU %d", i);
>   			cpufreq_settable = false;
> -			break;
> +			return FWTS_ERROR;
> +		}
> +
> +		rc = cpu_set_governor(fw, &cpus[i], "userspace");
> +
> +		if (rc != FWTS_OK) {
> +			fwts_log_warning(fw,"Failed to initialize cpufreq "
> +					"to set CPU speed for CPU %d", i);
> +			cpufreq_settable = false;
> +			return FWTS_ERROR;
>   		}
>   	}
>   
> 


Acked-by: Alex Hung <alex.hung@canonical.com>
Colin King July 7, 2018, 8:50 a.m. | #2
On 06/07/18 15:41, Robert Elliott wrote:
> Merge the loops parsing cpufreq directory content and setting the
> governor to userspace so the function can exit if parsing fails.
> Include the CPU number in the error messages.
> 
> That can happen if the kernel runs into this:
> [   24.218674] cpufreq: cpufreq_online: ->get() failed
> and does not create /sys/devices/system/cpu/cpuNN/cpufreq.
> 
> Signed-off-by: Robert Elliott <elliott@hpe.com>
> ---
>  src/cpu/cpufreq/cpufreq.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/src/cpu/cpufreq/cpufreq.c b/src/cpu/cpufreq/cpufreq.c
> index d1e50cdf..a65b9d08 100644
> --- a/src/cpu/cpufreq/cpufreq.c
> +++ b/src/cpu/cpufreq/cpufreq.c
> @@ -834,18 +834,25 @@ static int cpufreq_init(fwts_framework *fw)
>  	num_cpus = scandir(FWTS_CPU_PATH, &dirs, is_cpu_dir, versionsort);
>  	cpus = calloc(num_cpus, sizeof(*cpus));
>  
> -	for (i = 0; i < num_cpus; i++)
> -		parse_cpu_info(fw, &cpus[i], dirs[i]);
> -
>  	/* all test require a userspace governor */
>  	for (i = 0; i < num_cpus; i++) {
> -		int rc = cpu_set_governor(fw, &cpus[i], "userspace");
> +		int rc;
>  
> +		rc = parse_cpu_info(fw, &cpus[i], dirs[i]);
>  		if (rc != FWTS_OK) {
> -			fwts_log_warning(fw, "Failed to initialize cpufreq "
> -					"to set CPU speed");
> +			fwts_log_warning(fw,
> +				"Failed to parse cpufreq for CPU %d", i);
>  			cpufreq_settable = false;
> -			break;
> +			return FWTS_ERROR;
> +		}
> +
> +		rc = cpu_set_governor(fw, &cpus[i], "userspace");
> +
> +		if (rc != FWTS_OK) {
> +			fwts_log_warning(fw,"Failed to initialize cpufreq "
> +					"to set CPU speed for CPU %d", i);
> +			cpufreq_settable = false;
> +			return FWTS_ERROR;
>  		}
>  	}
>  
> 
Acked-by: Colin Ian King <colin.king@canonical.com>

Patch

diff --git a/src/cpu/cpufreq/cpufreq.c b/src/cpu/cpufreq/cpufreq.c
index d1e50cdf..a65b9d08 100644
--- a/src/cpu/cpufreq/cpufreq.c
+++ b/src/cpu/cpufreq/cpufreq.c
@@ -834,18 +834,25 @@  static int cpufreq_init(fwts_framework *fw)
 	num_cpus = scandir(FWTS_CPU_PATH, &dirs, is_cpu_dir, versionsort);
 	cpus = calloc(num_cpus, sizeof(*cpus));
 
-	for (i = 0; i < num_cpus; i++)
-		parse_cpu_info(fw, &cpus[i], dirs[i]);
-
 	/* all test require a userspace governor */
 	for (i = 0; i < num_cpus; i++) {
-		int rc = cpu_set_governor(fw, &cpus[i], "userspace");
+		int rc;
 
+		rc = parse_cpu_info(fw, &cpus[i], dirs[i]);
 		if (rc != FWTS_OK) {
-			fwts_log_warning(fw, "Failed to initialize cpufreq "
-					"to set CPU speed");
+			fwts_log_warning(fw,
+				"Failed to parse cpufreq for CPU %d", i);
 			cpufreq_settable = false;
-			break;
+			return FWTS_ERROR;
+		}
+
+		rc = cpu_set_governor(fw, &cpus[i], "userspace");
+
+		if (rc != FWTS_OK) {
+			fwts_log_warning(fw,"Failed to initialize cpufreq "
+					"to set CPU speed for CPU %d", i);
+			cpufreq_settable = false;
+			return FWTS_ERROR;
 		}
 	}