Messages in this thread | | | Subject | Re: [RFC][PATCH] cpufreq: intel_pstate: Delete intel_pstate sysfs if failed to register the driver | From | srinivas pandruvada <> | Date | Mon, 12 Oct 2020 06:22:40 -0700 |
| |
On Fri, 2020-10-09 at 11:30 +0800, Chen Yu wrote: > There is a corner case that if the intel_pstate driver failed to be > registered(might be due to invalid MSR access) Do you have logs why it is not loaded? On supported platforms MSRs should be invalid. It may be a case when we are trying to bring up pre-production systems where some instability in MSRs on certain CPUs.
But the patch is correct. We can't have invalid folder when intel_pstate is not used.
> and with the acpi_cpufreq > loaded, the intel_pstate sysfs might still be created, which makes > the > user confusing(turbostat for example): > > grep . /sys/devices/system/cpu/cpu0/cpufreq/scaling_driver > acpi-cpufreq > > grep . /sys/devices/system/cpu/intel_pstate/* > /sys/devices/system/cpu/intel_pstate/max_perf_pct:0 > /sys/devices/system/cpu/intel_pstate/min_perf_pct:0 > grep: /sys/devices/system/cpu/intel_pstate/no_turbo: Resource > temporarily unavailable > grep: /sys/devices/system/cpu/intel_pstate/num_pstates: Resource > temporarily unavailable > /sys/devices/system/cpu/intel_pstate/status:off > grep: /sys/devices/system/cpu/intel_pstate/turbo_pct: Resource > temporarily unavailable > > The existing of intel_pstate sysfs does not mean that the > intel_pstate driver > has been successfully loaded(for example, echo off to status), but > the > intel_pstate sysfs should not co-exist when acpi-cpufreq is also > present. > Fix this issue by deleting the intel_pstate sysfs if the driver > failed > to be loaded during bootup. > > Reported-by: Wendy Wang <wendy.wang@intel.com> > Suggested-by: Zhang Rui <rui.zhang@intel.com> > Signed-off-by: Chen Yu <yu.c.chen@intel.com> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com
> ---> > drivers/cpufreq/intel_pstate.c | 24 +++++++++++++++++++++++- > 1 file changed, 23 insertions(+), 1 deletion(-) > > diff --git a/drivers/cpufreq/intel_pstate.c > b/drivers/cpufreq/intel_pstate.c > index 9a515c460a00..8c5f9680de83 100644 > --- a/drivers/cpufreq/intel_pstate.c > +++ b/drivers/cpufreq/intel_pstate.c > @@ -1420,6 +1420,26 @@ static void __init > intel_pstate_sysfs_expose_params(void) > } > } > > +static void __init intel_pstate_sysfs_clean(void) > +{ > + if (!intel_pstate_kobject) > + return; > + > + sysfs_remove_group(intel_pstate_kobject, > &intel_pstate_attr_group); > + > + if (per_cpu_limits) > + goto release_kobj; > + > + sysfs_remove_file(intel_pstate_kobject, &max_perf_pct.attr); > + sysfs_remove_file(intel_pstate_kobject, &min_perf_pct.attr); > + > + if (x86_match_cpu(intel_pstate_cpu_ee_disable_ids)) > + sysfs_remove_file(intel_pstate_kobject, > &energy_efficiency.attr); > + > +release_kobj: > + kobject_put(intel_pstate_kobject); > +} > + > static void intel_pstate_sysfs_expose_hwp_dynamic_boost(void) > { > int rc; > @@ -3063,8 +3083,10 @@ static int __init intel_pstate_init(void) > mutex_lock(&intel_pstate_driver_lock); > rc = intel_pstate_register_driver(default_driver); > mutex_unlock(&intel_pstate_driver_lock); > - if (rc) > + if (rc) { > + intel_pstate_sysfs_clean(); > return rc; > + } > > if (hwp_active) { > const struct x86_cpu_id *id;
| |