Re: [PATCH] ACPI: battery: Re-read battery information in the complete callback

[johannes . goede]

Hi Rafael,

On 13-Oct-26 13:58, Rafael J. Wysocki (Intel) wrote:
> On Tue, Oct 13, 2026 at 11:33 AM <johannes.goede@xxxxxxxxxxxxxxxx> wrote:
>>
>> Hi,
>>
>> On 13-Oct-26 00:46, Ali Ahmet Memiş wrote:
>>> Since commit d33e89d12295 ("thermal: core: Suspend thermal zones later
>>> and resume them earlier"), thermal zones are resumed from dpm_complete(),
>>> before the PM_POST_SUSPEND notifier that re-reads _BIX and _BST. On
>>> ThinkPad P1 Gen2 and P53, INT3403 _TMP calls \GTST which switches the EC
>>> page selector (HIID) without taking the BATM mutex used by the battery
>>> methods. ACPICA releases the interpreter lock around EC accesses
>>
>> I'm wondering if this "ACPICA releases the interpreter lock around EC
>> accesses" is not the real problem here.
> 
> Yes, it most likely is.
> 
> What actually happens is that ACPICA drops the interpreter lock and
> the namespace lock in several cases listed in the
> acpi_ex_exit_interpreter() documentation comment, the majority of
> which involve having to wait.
> 
> The bottom line though is that the interpreter and namespace locks
> cannot be expected to be held throughout AML evaluation because it is
> not known in advance if any of the cases in question will appear.
> 
>> Recently I've learned / heard
>> from a trusted source that Windows ACPI subsystem is very much single
>> threaded. So it is nice that ACPI has these mutexes and other stuff
>> to support multi-threading but as long as Windows ACPI interpreter is
>> not multi-threaded I think we may need to consider to copy Windows
>> behavior.
> 
> This is also my conclusion, at least to some extent.
> 
> The problem with it though is that in some cases parallel evaluation
> of AML is actually useful.  For example, say two PCI devices are
> powered up at the same time and AML waits for each of them.  The async
> suspend/resume aligns those waits to take place simultaneously instead
> of serializing them which allows some time to be saved.
> 
>> So maybe the EC accesses should keep the interpreter lock locked during
>> EC accesses.
> 
> The problem here is that it is hard to say in advance whether or not
> _BIX or _BST will access the EC and whether or not that will clash
> with _TMP evaluation for a seemingly unrelated thermal zone.
> 
>> And even go further and maybe we need to introduce a big-acpi-lock,
>> which I guess the interpreter lock more or less is? And just serialize
>> all ACPI accesses since most ACPI tables clearly do not handle
>> multi-threaded access to ACPI methods well ?
> 
> I would be inclined to do so except for device power management
> methods, that is _ON, _OFF, and _PSx, that are not the problematic
> ones, at least as far as I know.

That sounds like a reasonable compromise which should fix both
this case as well as help with bko221065 .

>> Another example of multi-threaded ACPI calls causing problems is:
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=221065
> 
> Right.
> 
> So I guess the idea would be to add locking to acpi_evaluate_object()?

Yes that seems like a nice single point of entry to do this where
all the various helpers eventually end up.

> An alternative might be to introduce an "exclusive" variant of
> acpi_evaluate_object() with built-in locking and use it in the cases
> that are known-problematic.

So here we have _TMP and _BIX / _BST and in bko221065 we have
a bunch of again batteyr methods + UCSI racing. I'm afraid if
we add something like acpi_evaluate_object_exclusive() we'll end
up eventually using that almost everywhere.

Maybe do things the other way around and serializing all the calls
by default and adding an acpi_evaluate_object_unlocked() to be used
in cases where we want the speedup of multi-threading like
the _ON, _OFF and _PSx you mentioned above ?

Regards,

Hans