Bug 13389
| Summary: | Warning 'Invalid throttling state, reset' gets displayed when it should not be | ||
|---|---|---|---|
| Product: | ACPI | Reporter: | Frans Pop (elendil) |
| Component: | BIOS | Assignee: | Zhang Rui (rui.zhang) |
| Status: | CLOSED CODE_FIX | ||
| Severity: | normal | CC: | acpi-bugzilla, james, lenb, maciej.rutecki, rjw, rui.zhang, yakui.zhao |
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 2.6.30-rc6 | Subsystem: | |
| Regression: | Yes | Bisected commit-id: | |
| Bug Depends on: | |||
| Bug Blocks: | 13070 | ||
| Attachments: |
Dubug patch and debug output
Patch to force throttling reset Updated patch to change warning to debug level Patch to force throttling reset Updated patch to change warning to debug level |
||
|
Description
Frans Pop
2009-05-26 15:24:09 UTC
I just see that my analysis here is not complete.
> 3) The reset that is being done when the warning triggers is bogus for my
> case because in acpi_processor_set_throttling_ptc() it is ignored due to
> the following test:
> if (state == pr->throttling.state)
> return 0;
The problem is that the code creates a self-fulfilling prophesy.
In acpi_processor_get_throttling_ptc():
pr->throttling.state = 0;
[...]
state = acpi_get_throttling_state(pr, value);
if (state == -1) {
ACPI_WARNING((AE_INFO,
"Invalid throttling state, reset"));
state = 0;
ret = acpi_processor_set_throttling(pr, state);
So acpi_processor_get_throttling_ptc() sets both pr->throttling.state _and_ state to 0 before calling acpi_processor_set_throttling(). No wonder the test there always succeeds and no change is actually made!
Created attachment 21671 [details] Patch to force throttling reset In the attachment one possible patch to fix the problem that no reset to T0 actually takes place. I've tested it on my system and with that there remains only one reset message per processor (during boot). Because the reset now really does happen, the later warnings during 'cat /proc/acpi/processor/*/throttling' are gone. After this bug is fixed I don't have any real objection to changing the warning to debug level, as suggested by Zhang Rui in http://bugzilla.kernel.org/show_bug.cgi?id=13259#c29. The attached patch is straightforward, but fairly big. One much simpler solution could be the following patch: --- a/drivers/acpi/processor_throttling.c +++ b/drivers/acpi/processor_throttling.c @@ -842,6 +842,7 @@ static int acpi_processor_get_throttling_ptc( ACPI_WARNING((AE_INFO, "Invalid throttling state, reset\n")); state = 0; + pr->throttling.state = 9; /* Ensure reset happens */ ret = acpi_processor_set_throttling(pr, state); if (ret) return ret; Yet another approach could be to always initialize the throttling state to T0 in acpi_processor_get_throttling_info(), but that would still require the change to T0 to be forced somehow. Comments welcome. Created attachment 21672 [details]
Updated patch to change warning to debug level
Based on Zhang Rui's earlier proposal, but updated because of my patch to remove the spurious newline, which is needed again now :-P
To be applied after a fix for the reset issue.
Handled-By : Frans Pop <elendil@planet.nl> Patch : http://bugzilla.kernel.org/attachment.cgi?id=21671 Patch : http://bugzilla.kernel.org/attachment.cgi?id=21672 Today I've been able to test that throttling works correctly with these patches when the passive trip point is reached for a thermal zone. For details, see: http://bugzilla.kernel.org/show_bug.cgi?id=13918#c14. Created attachment 22857 [details]
Patch to force throttling reset
Created attachment 22858 [details]
Updated patch to change warning to debug level
2a908002c7b1b666616103e9df2419b38d7c6f1f ACPI processor: force throttling state when BIOS returns incorrect value bdf57de4e6abc389cc3f3bd94ec15cce74cf6f4b acpi processor: remove superfluous warning message shipped in 2.6.31-rc7-git5 closed. |