-
-
Notifications
You must be signed in to change notification settings - Fork 289
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix spam error message when setting energy_performance_preference #680
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for finding the cause of the write error spam. but I think we want to keep balance_performance
as the default EPP, so the solution to this would be to check if the user is running intel_pstate
, if they are, set the EPP to performance
, otherwise set it to balance_performance
Also does this not occur with set_powersave()
as well since that uses balance_power
? I can't test any of this as I don't have an Intel CPU
Lastly, in the future please try not to include all of that whitespace stuff in your commits
In the original issue #661 someone commented having the same issue with an AMD CPU with And yes this does not occur with |
I absolutely agree with what was said here and share the same opinion:
Also, since I'm using intel_pstate driver am I misunderstanding something because when I'm on powersave governor, EPP will be set to
i.e:
and opposite will be the case when setting performance governor, i.e:
Aren't you running on AMD @shadeyg56 ? |
6ec30c6
to
c5b418a
Compare
First, let me resolve the misunderstanding which was caused due to missing information in the PR comment and the commit message, which I have now hopefully resolved and new people stumbling on this PR will understand this better.
So, this is the relevant section of the kernel documentation regarding the why's of the issue. In summary, in system's with Intel CPU and AMD CPU, that support the hardware-managed P-states (HWP) running the This behavior causes the error message "write error: Device or resource busy", when auto-cpufreq tries to set the EPP to This does not happen when the scaling governor is set to Hopefully this clears the misunderstanding. Regarding keeping the default
I am hesitated to only apply this fix for |
If you insist on only applying this fix for |
I've used |
Well, I look like an idiot now 🥲, I already updated the code to only apply this for |
The 'intel_pstate' driver does not allow the EPP to be set to anything but 'performance' when the scaling governor is set to 'performance', previously auto-cpufreq when the scaling-governor was set to 'performance' tried to set the EPP to 'balance-performance' which caused a spam of write error messages in journalctl in system with 'intel_pstate' drivers. This is an intended behavior, since according to the [kernel documentation](https://docs.kernel.org/admin-guide/pm/intel_pstate.html#hwp-performance) when HWP is enabled[(which is enabled by default during boot with supported processors)](https://docs.kernel.org/admin-guide/pm/intel_pstate.html#active-mode-with-hwp) and scaling governor is set to performance the processor’s internal P-state selection logic is expected to focus entirely on performance. And this will override the EPP setting and reject any value different from 0 (“performance”). This commit just changes the 'balance-performance' EPP preference in set_performance() to 'performance'. Which fixes the spam issue.
fb01fac
to
828db0e
Compare
Yes it does happen with Intel, I have tested it with the I think before merging, the MR should be changed to also include this case, but I do believe the way that this is done is not the best. I think that instead of just checking if the path exists, we should read the value of said path and set the value accordingly. |
+1 |
OS: Arch Linux x86_64 |
intel_pstate/staus to make sure the condition only applies when "active"
Strange. After a kernel update I cannot reproduce either. This means that this might be an upstream kernel bug but what's the point of waiting for them. I like your current change regarding checking the state. @AdnanHodzic please review. |
Updated the code so that user written config energy_performance_preference are also handled. |
I started testing the changes on this branch, removed my auto-cpufreq config file and wanted to see what it runs on "default" settings. While laptop is connected to power it all works as expected in terms of EPP and governors. But then, when pulled out the power cable and ran: I noticed that both governor and EPP remained pinned to "powersave", and what's more worrying my CPU cores were also pinned to max 800Mhz. As while auto-cpufreq will switch you to
But then I noticed this was the case on |
Please disregard my last reply, from code POV, changes look good now, and I've been running auto-cpufreq with them for couple of days and I had 0 problems. @shadeyg56 I requested re-review of comment I made, as from what I see this request has been fulfilled, if not please raise it. Otherwise, thank you for your contribution @FosRexx, you will be credited for your work as part of upcoming auto-cpufreq release! |
The 'intel_pstate' driver does not allow the EPP to be set to anything but 'performance' when the scaling governor is set to 'performance', previously auto-cpufreq when the scaling-governor was set to 'performance' tried to set the EPP to 'balance-performance' which caused a spam of write error messages in journalctl in system with 'intel_pstate' drivers. This is an intended behavior, since according to the kernel documentation when HWP is enabled(which is enabled by default during boot with supported processors) and scaling governor is set to performance the processor’s internal P-state selection logic is expected to focus entirely on performance. And this will override the EPP setting and reject any value different from 0 (“performance”). This commit just changes the 'balance-performance' EPP preference in set_performance() to 'performance'. Which fixes the spam issue.
Fixes: #661