Re: [RFC][PATCH] cpufreq: intel_pstate: Delete intel_pstate sysfs if failed to register the driver

[Chen Yu]

Hi Srinivas,
On Mon, Oct 12, 2020 at 06:22:40AM -0700, srinivas pandruvada wrote:
> 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.
Unfortunately we don't have the boot up log for now, as it is
a pre-production platform and the low-level simulation(for MSR)
might be unstable.( And there seems to be some environment issue
on pre-production platform to reproduce this issue).
But we can hack the code in intel_pstate to make the driver failed
to be loaded and the issue was reproduced.
> 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@xxxxxxxxx>
> > Suggested-by: Zhang Rui <rui.zhang@xxxxxxxxx>
> > Signed-off-by: Chen Yu <yu.c.chen@xxxxxxxxx>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx
Thanks!


Best,
Chenyu