Re: [PATCH] ACPI: SBS: Serialize battery state refreshes

[Rafael J. Wysocki (Intel)]

On Tue, Oct 20, 2026 at 12:58 PM Penkacelsus via B4 Relay
<devnull+balthasar978.gmail.com@xxxxxxxxxx> wrote:
>
> From: Penkacelsus <balthasar978@xxxxxxxxx>
>
> acpi_battery_get_state() refreshes the battery state at most once per
> cache_time (1 s), but the check and the update are not serialized:
> callers that arrive together all see a stale cache and each read all
> seven state registers over SMBus.
>
> On T2 MacBooks (MacBookPro16,1: SBS battery behind the EC's SMBus host
> controller) every SMBus word read takes about two dozen EC GPEs. A
> desktop shell that reloads ten BAT0 attributes in parallel every 2 s
> drove the EC GPE to ~680/s, which kept irq/9-acpi near 16% of a CPU.
> Readers of a single attribute file each get their own thread in such
> shells, so the race is the common case, not a corner case.
>
> Serialize the refresh with a per-battery mutex taken in
> acpi_battery_get_state(), so that the power supply readers and the
> SMBus alarm callback share it. The first caller does the SMBus reads;
> the others wait and then find a fresh cache. Use a new lock rather than
> sbs->lock: acpi_sbs_remove() holds sbs->lock while it unregisters the
> power supplies, which waits for sysfs readers to drain.
>
> With ten parallel readers of state attributes every 200 ms, SMBus reads
> drop from 26-29/s to 6/s (the cache floor) and EC GPEs from 600-670/s
> to ~140/s.
>
> Signed-off-by: Penkacelsus <balthasar978@xxxxxxxxx>
> ---
>  drivers/acpi/sbs.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
> index f10bbf13c..861739b18 100644
> --- a/drivers/acpi/sbs.c
> +++ b/drivers/acpi/sbs.c
> @@ -55,6 +55,7 @@ struct acpi_battery {
>         struct power_supply_desc bat_desc;
>         struct acpi_sbs *sbs;
>         unsigned long update_time;
> +       struct mutex state_lock;        /* serializes state refreshes */
>         char name[8];
>         char manufacturer_name[ACPI_SBS_BLOCK_MAX];
>         char device_name[ACPI_SBS_BLOCK_MAX];
> @@ -355,6 +356,13 @@ static int acpi_battery_get_state(struct acpi_battery *battery)
>  {
>         int i, result = 0;
>
> +       /*
> +        * Callers that find the cache stale at the same time would each
> +        * re-read the whole state over SMBus. Serialize them so that only
> +        * the first one does; the others then find a fresh cache.
> +        */
> +       guard(mutex)(&battery->state_lock);
> +
>         if (battery->update_time &&
>             time_before(jiffies, battery->update_time +
>                                 msecs_to_jiffies(cache_time)))
> @@ -367,9 +375,8 @@ static int acpi_battery_get_state(struct acpi_battery *battery)
>                                          (u8 *)battery +
>                                                 state_readers[i].offset);
>                 if (result)
> -                       goto end;
> +                       break;
>         }
> -      end:
>         battery->update_time = jiffies;
>         return result;
>  }
> @@ -530,6 +537,7 @@ static int acpi_battery_add(struct acpi_sbs *sbs, int id)
>
>         battery->id = id;
>         battery->sbs = sbs;
> +       mutex_init(&battery->state_lock);
>         result = acpi_battery_read(battery);
>         if (result)
>                 return result;
>
> ---

Applied as 7.5 material, thanks!