[PATCH] ACPI: SBS: Serialize battery state refreshes

[PATCH] ACPI: SBS: Serialize battery state refreshes

[Penkacelsus via B4 Relay]

From: Penkacelsus <balthasar978@gmail.com>

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@gmail.com>
---
 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;

---
base-commit: a7b81d2593fc90a93e108441a04e3b258d19f074
change-id: 20261020-acpi-sbs-serialize-4e984398cf5c

Best regards,
--  
Penkacelsus <balthasar978@gmail.com>