| Message ID | 20180706144106.56937-1-elliott@hpe.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] cpufreq: exit if parsing the cpufreq directory fails | expand |
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>
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>
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; } }
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(-)