RE: Please revert "ACPICA: AML interpreter: add region addresses in global list during initialization"
Snip
On Tue, Nov 20, 2018 at 10:03:59AM +0100, Jean Delvare wrote:
On Tue, 20 Nov 2018 09:54:19 +0100, Greg KH wrote:
Ok, I'll go revert this, but shouldn't it also be reverted in Linus's tree as well?
Hi Jean and Greg,
No. As I understand it (with my limited knowledge of ACPICA), the change itself is correct. The problem is that it will detect resource conflicts which were unnoticed before, and that will prevent drivers from loading. Some of them may be addressed with driver fixes or new drivers. Others are false positives (due to bogus BIOS) which users will have to work around with acpi_resource_conflicts=lax. We have been through this before, nothing new really, but it takes years to address such problems. This just
can't be done in stable kernel series.
I would like to give you more context.
There was a fairly complicated change that occurred in 4.17 and we caused a regression by forgetting to add region addresses in a global list during operation region initialization. We found the regression when bug reporters tried to boot their macbook pro and asus laptop and saw that there was a difference in behavior when drivers are being loaded
Commit 4abb951b73ff0a8a979113ef185651aa3c8da19b has no Fixes tag. Which commit introduced the regression? Can you point me to the associated bug reports?
Sorry about the missing fixed tag. It's supposed to fix 5a8361f7ecceaed64b4064000d16cb703462be49 ACPICA: Integrate package handling with module-level code
So what I am trying to say is that we have been emitting these errors for a while before we caused the regression. The goal with this patch is to keep the behavior the same as kernels older than 4.17 where warnings are printed to dmesg due to resource conflicts.
Fine with me for upstream, but I still need to be convinced that it belongs to stable series. For now, the only 2 related bugs I know of are #200011 (which is NOT fixed by commit
This bug is kind of messy. It's like 2 bugs in 1 report. This patch fixes a part of their problem. Another part has to do with ECDT load order.
Would it help to open a separate bug report for this particular issue and close it? I think this might be a little confusing to someone who reads the commit message.
4abb951b73ff0a8a979113ef185651aa3c8da19b) and #201721 (which is
caused
I did not know about this bug report.
by that commit). 1 vs 0, revert wins. If you want me to change my mind, you must provide additional data points proving that commit 4abb951b73ff0a8a979113ef185651aa3c8da19b solves more functional regressions than it causes.
Here's an additional data point from the discussion on bz201721. The user started using fancontrol during the period where resource conflict checking was unintentionally removed. If the reporter tries using fancontrol on 4.18.13, they report that fancontrol does not work. So it was basically always the case that he needed to use acpi_resource_conflicts=lax for a short period of time.
I understand that this "breaks" this machine but we shouldn't be reverting resource conflict checking for all other drivers just because of fancontrol. By removing this check, we are suppressing warnings and changing the loading behavior of other drivers as well.
Thanks, Erik
If we do not have include this commit, then we are in a state where resource conflicts are not properly detected. These detections help kernels decide which drivers to load. However, I do see your point about this breaking the reporter's machine and it's possible that I may have missed something.
We should get the reporter's dmesg for older kernels.
It would be ideal to resolve this bug AND add region addresses in the global address list. I've asked the bug reporter in #201721 for more information. We can delay this patch for now and look at the reporter's issue more carefully.
Thanks for all of the information!
Erik
Thanks,
Jean Delvare SUSE L3 Support
Hi Erik, Greg,
On Wed, 28 Nov 2018 00:27:37 +0000, Schmauss, Erik wrote:
Here's an additional data point from the discussion on bz201721. The user started using fancontrol during the period where resource conflict checking was unintentionally removed. If the reporter tries using fancontrol on 4.18.13, they report that fancontrol does not work. So it was basically always the case that he needed to use acpi_resource_conflicts=lax for a short period of time.
I understand that this "breaks" this machine but we shouldn't be reverting resource conflict checking for all other drivers just because of fancontrol. By removing this check, we are suppressing warnings and changing the loading behavior of other drivers as well.
I agree. The incorrect (or at the very least incomplete and confusing) claims in bug #201721 made me think it was a regression from older versions. As it turns out that this isn't the case, my request to revert 4abb951b73ff0a8a979113ef185651aa3c8da19b from stable kernel series was incorrect. I apologize for the noise and ask Greg to undo the revert.
Thanks,
On Wed, Nov 28, 2018 at 09:56:27AM +0100, Jean Delvare wrote:
Hi Erik, Greg,
On Wed, 28 Nov 2018 00:27:37 +0000, Schmauss, Erik wrote:
Here's an additional data point from the discussion on bz201721. The user started using fancontrol during the period where resource conflict checking was unintentionally removed. If the reporter tries using fancontrol on 4.18.13, they report that fancontrol does not work. So it was basically always the case that he needed to use acpi_resource_conflicts=lax for a short period of time.
I understand that this "breaks" this machine but we shouldn't be reverting resource conflict checking for all other drivers just because of fancontrol. By removing this check, we are suppressing warnings and changing the loading behavior of other drivers as well.
I agree. The incorrect (or at the very least incomplete and confusing) claims in bug #201721 made me think it was a regression from older versions. As it turns out that this isn't the case, my request to revert 4abb951b73ff0a8a979113ef185651aa3c8da19b from stable kernel series was incorrect. I apologize for the noise and ask Greg to undo the revert.
Ok, now added back, thanks.
greg k-h
linux-stable-mirror@lists.linaro.org
-
'Greg KH' -
Jean Delvare -
Schmauss, Erik