Re: [lm-sensors] Could the k8temp driver be interfering with ACPI?

[Jean Delvare <khali@linux-fr.org>]

Hi Bodo,

On Tue, 6 Mar 2007 21:40:19 +0100 (CET), Bodo Eggert wrote:
> On Tue, 6 Mar 2007, Jean Delvare wrote:
> > On Mon, 05 Mar 2007 14:56:44 +0100, Bodo Eggert wrote:
> 
> > > 2) make ACPI take this lock whenever it touches ports not allocated by itself
> > >    and release it on function return.
> > 
> > This is costly.
> 
> TANSTAAFL. You'll need to take some lock, and if you want port emulation 
> or per-device-mutex, you'll have to pay the price.

True, but precisely, I wonder what will perform the best in practice, a
single semaphore that is unconditionally taken by ACPI/AML, or your
finer-grained approach and its many additional tests.

> 
> > > 3) Make ACPI callback the allocating device if it touches allocated ranges,
> > >    and on function return.
> > 
> > By callback, do you mean port forwarding as Rudolf Marek did, or more
> > simple "stop touching the device" and "you can touch the device again"
> > sort of signals? If the latter, this is no different from a mutex.
> 
> It may be the described port emulation callback (which is very nice and
> possibly desired for some devices), but if you add an "I am done tampering
> with your hardware" callback, the driver can also implement the mutex.
> 
> Some drivers will not understand all possible access patterns, they may 
> try to emulate first, and if they fail, fall back to taking the mutex and 
> passing the raw port access.

Good point.

> 
> > > 1 + 2 will replace allocating single ports and freeing them again (so ACPI won't
> > > prevent e.g. the display driver from loading just because it turned on the
> > > backlight) while preventing races with newly allocated ranges, and 3 allows
> > > coexistence with other drivers.
> > 
> > What benefit do you see compared to a lock taken by both AML and the
> > hardware monitoring drivers?
> 
> 1) I asume port allocations or ACPI foreign port acces to be rare, so
>    there would be little impact on (un)registering hardware. Off cause
>    there are some long ACPI calls (like reading the battery?), in these
>    cases it might be beneficial to release the global allocation lock
>    after gaining exclusive access to the driver's port range. (This can
>    only be done if the device driver is loaded.)

The problem is that the rarity of ACPI foreign port access doesn't
matter much, as you have to do all the tests first to find out that it
was a foreign port, and I suspect that these tests will cost more than
taking the lock itself.

> 2) If there is a global lock for accessing ACPI-tampered hardware, each
>    driver that *might* conflict with ACPI _must_ take it on each access.  
>    This would seialize unrelated sensor busses, IDE, graphics ...  (AFAI
>    can imagine.)  A callback allows using a per-(device|bus)-mutex instead.

True, this is the problem with having a single mutex: everything gets
serialized. I have no idea how often we are running AML code though, so
I don't know if this is a real problem in practice.

We may be able to workaround the inter-driver exclusion though, if we
use a semaphore initialized to N, have ACPI take N, but other drivers
take just one. This would let ACPI be exclusive with all the other
drivers, but drivers themselves could otherwise run concurrently. That
being said, we do not appear to have the required primitives to take
more than 1 semaphore resource at once at the moment, so we'd need to
do that first.

OTOH, I certainly hope that not that many drivers are affected on a
given system, so maybe serialization is acceptable. We'll need some
field experimentation to find out.

> Rudolph's implementation is close enough to imagine what would be done,
> the changes would be:
> 
> - Add a "done" call to resource_ops
> 
> in ACPI:
> - Check for own region first and do raw access there
> - get the lock before __request_region
> - s/__request/__check/
> 
> in driver:
> - If emulation fails, get the local lock and switch to passthrough mode
> - if in passthrough mode and done is called, release the lock

Yep, that's a bit complex, but should work, thanks for sketching this.

-- 
Jean Delvare