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

[johannes . goede]

Hi Rafael,

On 13-Oct-26 19:20, Rafael J. Wysocki (Intel) wrote:
> Hi Hans,
> 
> On Tue, Oct 13, 2026 at 3:36 PM <johannes.goede@xxxxxxxxxxxxxxxx> wrote:
>>
>> 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.
> 
> That, of course, would require updating ACPICA code and I'm not sure
> if doing it upstream is useful.  For example, if some hosts already
> use locking around acpi_evaluate_object(), this modification won't
> help them and it will certainly be a change in behavior.
> 
> This means that we'll likely end up with a Linux-specific divergence
> from the upstream which can be lived with, but it won't be nice.

Ah, I see.

>>> 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.
> 
> While I think that your concern is valid, this can be done without
> touching the ACPICA code at all.

Not needing to touch ACPICA is a valid concern. So I agree lets go
with a wrapper for paths which we know need to be serialized for now.

And then add that around the battery methods + _TMP + UCSI methods
for starters I guess ?

###

Should we also stop releasing the interpreter lock in the EC code?

All these things (battery, _TMP, UCSI) seem to have in common that
they might talk to the EC in some cases?

Where as e.g. your powering-up of PCI slots example is unlikely to
talk to the EC ?

Regards,

Hans