RE: Please revert "ACPICA: AML interpreter: add region addresses in global list during initialization"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux