Re: [PATCH v1 2/6] cpufreq: intel_pstate: Do not update global.turbo_disabled after initialization

[Xi Ruoyao]

On Sat, 2024-06-01 at 21:03 -0700, srinivas pandruvada wrote:
> Hi Xi,
> 
> On Sun, 2024-06-02 at 11:21 +0800, Xi Ruoyao wrote:
> > On Mon, 2024-03-25 at 18:02 +0100, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> > > 
> > > The global.turbo_disabled is updated quite often, especially in the
> > > passive mode in which case it is updated every time the scheduler
> > > calls
> > > into the driver.  However, this is generally not necessary and it
> > > adds
> > > MSR read overhead to scheduler code paths (and that particular MSR
> > > is
> > > slow to read).
> > > 
> > > For this reason, make the driver read
> > > MSR_IA32_MISC_ENABLE_TURBO_DISABLE
> > > just once at the cpufreq driver registration time and remove all of
> > > the
> > > in-flight updates of global.turbo_disabled.
> > 
> > Hi Rafael and Srinivas,
> > 
> > Thanks for the clean up, but unfortunately on one of my laptops
> > (based
> > on i5-11300H) MSR_IA32_MISC_ENABLE_TURBO_DISABLE is mysteriously
> > changing from 1 to 0 in about one minute after system boot.  I've no
> > idea why this is happening (firmware is doing some stupid thing?)
> > 
> > I've noticed the issue before and "hacked it around"
> > (https://bugzilla.kernel.org/show_bug.cgi?id=218702). But after this
> > change I can no longer hack it around and the system is much slower.
> > 
> > Is it possible to hack it around again?
> > 
> Please try the attached diff and build kernel and try.
> 
> git apply update_max_freq.diff
> 
> Then build kernel and install.

Unfortunately it didn't work.  Then I tried:

@@ -1304,6 +1310,10 @@ static ssize_t store_no_turbo(struct kobject *a, struct kobj_attribute *b,
 	if (no_turbo == global.no_turbo)
 		goto unlock_driver;
 
+	global.turbo_disabled = turbo_is_disabled();
+	global.no_turbo = global.turbo_disabled;
+	arch_set_max_freq_ratio(global.turbo_disabled);
+
 	if (global.turbo_disabled) {
 		pr_notice_once("Turbo disabled by BIOS or unavailable on processor\n");
 		count = -EPERM;

and my old hack worked again.  Curiously after I writing 0 to
/sys/devices/system/cpu/intel_pstate/no_turbo successfully, your code is
triggered.

$ dmesg | grep intel_pstate
[    0.554425] intel_pstate: Intel P-state driver initializing
[    0.554877] intel_pstate: HWP enabled
[    1.780021] intel_pstate: Turbo disabled by BIOS or unavailable on processor
[   21.789044] intel_pstate: intel_pstate_update_limits cpu:0
[   21.789053] intel_pstate: intel_pstate_update_limits cpu:1
[   21.789060] intel_pstate: intel_pstate_update_limits cpu:2
[   21.789189] intel_pstate: intel_pstate_update_limits cpu:3
[   21.789198] intel_pstate: intel_pstate_update_limits cpu:4
[   21.789203] intel_pstate: intel_pstate_update_limits cpu:5
[   21.789209] intel_pstate: intel_pstate_update_limits cpu:6
[   21.789276] intel_pstate: intel_pstate_update_limits cpu:7

The message at [1.780021] is from the first attempt writing 0 to
/sys/devices/system/cpu/intel_pstate/no_turbo when
MSR_IA32_MISC_ENABLE_TURBO_DISABLE is still 1.

-- 
Xi Ruoyao <xry111@xxxxxxxxxxx>
School of Aerospace Science and Technology, Xidian University