This repository has been archived by the owner on Sep 9, 2024. It is now read-only.
Release v1.3.0 ACPI Driver #12
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit aims to change the accounting system of CPUs. In some cases, the number of CPUs obtained through the runtime library function is slightly less than the actual/real number of CPUs. This change introduces reading the number of CPUs online through sysfs (/sys/devices/system/cpu/online). Signed-off-by: Eduardo Alberti <eduardo.alberti@windriver.com> Co-authored by: Pedro de Souza Silva <pedro.desouzasilva@windriver.com> Co-authored-by: Eduardo Alberti <eduardo.alberti@windriver.com>
* feat: ACPI integration * Makes EPP a seperate feature * Library tweaked to accept acpi dev * Library altered to accept alternate file system side affect of changing function names to be driver agnostic validated through unit tests and in conjunction with power manager * fix: Adding getNumberOfCpus to libconfig Adding getNumberOfCpus allows us to set the number of available cores. This helps in unit tests, especially since rt.numcpus can break unit tests when a large number of isolcpus are enabled on the system
- moves cores to shared pool in fuzz test to prevent error - adds cstate operation to test
Bumps [github.com/stretchr/testify](https://github.com/stretchr/testify) from 1.8.2 to 1.8.4. - [Release notes](https://github.com/stretchr/testify/releases) - [Commits](stretchr/testify@v1.8.2...v1.8.4) --- updated-dependencies: - dependency-name: github.com/stretchr/testify dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* docs: Updating README to factor in acpi support * adds acpi-cpufreq to readme * renames p-states file to scaling_driver
pbrownlow7
approved these changes
Jul 26, 2023
adorney99
approved these changes
Jul 26, 2023
catblade
reviewed
Jul 26, 2023
README.md
Outdated
| @@ -23,8 +23,7 @@ This library is currently used as part of the | |||
| # Prerequisites | |||
|
|
|||
| - Linux based OS | |||
| - P-States | |||
| - ``intel_pstates`` enabled - no entries in kernel cmdline disabling it | |||
| - P-States or acpi-cpufreq scaling driver enabled | |||
There was a problem hiding this comment.
Suggested change
| - P-States or acpi-cpufreq scaling driver enabled | |
| - P-State or acpi-cpufreq scaling driver enabled |
catblade
reviewed
Jul 26, 2023
pkg/power/README.md
Outdated
|
|
||
| * Performance governor - The CPUfreq governor "performance" sets the CPU statically to the highest frequency within the | ||
| borders of scaling_min_freq and scaling_max_freq. | ||
| * Powersave governor - The CPUfreq governor "powersave" sets the CPU statically to the lowest frequency within the | ||
| borders of scaling_min_freq and scaling_max_freq. | ||
|
|
||
| ### acpi-cpufreq | ||
| The acpi-cpufreq driver operates much like the P-state driver but has a number of both static and dynamic governors. For more information see [here](https://www.kernel.org/doc/html/v4.12/admin-guide/pm/cpufreq.html). |
There was a problem hiding this comment.
Suggested change
| The acpi-cpufreq driver operates much like the P-state driver but has a number of both static and dynamic governors. For more information see [here](https://www.kernel.org/doc/html/v4.12/admin-guide/pm/cpufreq.html). | |
| The acpi-cpufreq driver setting operates much like the P-state driver but has a different set of available governors. For more information see [here](https://www.kernel.org/doc/html/v4.12/admin-guide/pm/cpufreq.html). |
catblade
reviewed
Jul 26, 2023
pkg/power/README.md
Outdated
| ### acpi-cpufreq | ||
| The acpi-cpufreq driver operates much like the P-state driver but has a number of both static and dynamic governors. For more information see [here](https://www.kernel.org/doc/html/v4.12/admin-guide/pm/cpufreq.html). | ||
| One thing to note is that acpi-cpufreq reports the base clock as the frequency hardware limits however the P-state driver uses turbo frequency limits. | ||
| Both drivers can make use of turbo frequency however acpi-cpufreq will exceed its hardware frequency limits when using turbo frequency. This is important to take into account when setting frequencies for profiles. |
There was a problem hiding this comment.
Suggested change
| Both drivers can make use of turbo frequency however acpi-cpufreq will exceed its hardware frequency limits when using turbo frequency. This is important to take into account when setting frequencies for profiles. | |
| Both drivers can make use of turbo frequency; however, acpi-cpufreq can exceed hardware frequency limits when using turbo frequency. This is important to take into account when setting frequencies for profiles. |
catblade
reviewed
Jul 26, 2023
pkg/power/cpu_test.go
Outdated
| @@ -110,7 +113,7 @@ func setupCpuPStatesTests(cpufiles map[string]map[string]string) func() { | |||
| // revert get number of system cpus function | |||
| getNumberOfCpus = origGetNumOfCpusFunc | |||
| // revert p-states feature to un initialised state | |||
There was a problem hiding this comment.
probably want to update this comment.
catblade
reviewed
Jul 26, 2023
pkg/power/cpu_test.go
Outdated
| @@ -140,7 +143,7 @@ func TestNewCore(t *testing.T) { | |||
| }, cpu) | |||
|
|
|||
| // now "break" P-States by setting a feature error | |||
There was a problem hiding this comment.
probably want to update this comment.
catblade
reviewed
Jul 26, 2023
pkg/power/scaling_driver.go
Outdated
| } | ||
| return cpu.setPStatesValues(defaultPowerProfile) | ||
| return cpu.setDriverValues(defaultPowerProfile) | ||
| } | ||
|
|
||
| // setPStatesValues is an entrypoint to P-States feature consolidation |
There was a problem hiding this comment.
Suggested change
| // setPStatesValues is an entrypoint to P-States feature consolidation | |
| // setDriverValues is an entrypoint to power governor feature consolidation |
catblade
suggested changes
Jul 26, 2023
madalazar
approved these changes
Jul 27, 2023
Comment on lines
43
to
+45
| exclusivePools: PoolList{}, | ||
| featureStates: &featureList, | ||
| } | ||
|
|
||
| host.featureStates = &featureList |
There was a problem hiding this comment.
Just a small question: why is this needed? It seems like we're doing the same thing
There was a problem hiding this comment.
I can't remember exactly why I did this but I think it was because I was getting some weird behavior where the host was using a snapshot of the pointers' value instead of the pointer itself. Think this was just for the purposes of debugging to see the field go from unassigned to assigned but probably best to leave as is right now just in case and revert it for next time around
patricia-cahill
approved these changes
Jul 31, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Release to support ACPI Driver