v2: - Fix missing mutex_unlock in acpi_battery_update() (Reported-by: kernel test robot)
When removing and reinserting the laptop battery, ACPI can trigger two notifications in quick succession:
- ACPI_BATTERY_NOTIFY_STATUS (0x80) - ACPI_BATTERY_NOTIFY_INFO (0x81)
Both notifications call acpi_battery_update(). Because the events happen very close in time, sysfs_add_battery() can be re-entered before battery->bat is set, causing a duplicate sysfs entry error.
This patch ensures that sysfs_add_battery() is not re-entered when battery->bat is already non-NULL, preventing the duplicate sysfs creation and stabilizing battery hotplug handling.
[ 476.117945] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' [ 476.118896] CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 [ 476.118903] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 [ 476.118906] Workqueue: kacpi_notify acpi_os_execute_deferred [ 476.118917] Call Trace: [ 476.118922] <TASK> [ 476.118929] dump_stack_lvl+0x5d/0x80 [ 476.118938] sysfs_warn_dup.cold+0x17/0x23 [ 476.118943] sysfs_create_dir_ns+0xce/0xe0 [ 476.118952] kobject_add_internal+0xba/0x250 [ 476.118959] kobject_add+0x96/0xc0 [ 476.118964] ? get_device_parent+0xde/0x1e0 [ 476.118970] device_add+0xe2/0x870 [ 476.118975] __power_supply_register.part.0+0x20f/0x3f0 [ 476.118981] ? wake_up_q+0x4e/0x90 [ 476.118990] sysfs_add_battery+0xa4/0x1d0 [battery] [ 476.118998] acpi_battery_update+0x19e/0x290 [battery] [ 476.119002] acpi_battery_notify+0x50/0x120 [battery] [ 476.119006] acpi_ev_notify_dispatch+0x49/0x70 [ 476.119012] acpi_os_execute_deferred+0x1a/0x30 [ 476.119015] process_one_work+0x177/0x330 [ 476.119022] worker_thread+0x251/0x390 [ 476.119026] ? __pfx_worker_thread+0x10/0x10 [ 476.119030] kthread+0xd2/0x100 [ 476.119033] ? __pfx_kthread+0x10/0x10 [ 476.119035] ret_from_fork+0x34/0x50 [ 476.119040] ? __pfx_kthread+0x10/0x10 [ 476.119042] ret_from_fork_asm+0x1a/0x30 [ 476.119049] </TASK> [ 476.142552] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory. [ 476.415022] ata1.00: unexpected _GTF length (8) [ 476.428076] sd 0:0:0:0: [sda] Starting disk [ 476.835035] ata1.00: unexpected _GTF length (8) [ 476.839720] ata1.00: configured for UDMA/133 [ 491.328831] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' [ 491.329720] CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 [ 491.329727] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 [ 491.329731] Workqueue: kacpi_notify acpi_os_execute_deferred [ 491.329741] Call Trace: [ 491.329745] <TASK> [ 491.329751] dump_stack_lvl+0x5d/0x80 [ 491.329758] sysfs_warn_dup.cold+0x17/0x23 [ 491.329762] sysfs_create_dir_ns+0xce/0xe0 [ 491.329770] kobject_add_internal+0xba/0x250 [ 491.329775] kobject_add+0x96/0xc0 [ 491.329779] ? get_device_parent+0xde/0x1e0 [ 491.329784] device_add+0xe2/0x870 [ 491.329790] __power_supply_register.part.0+0x20f/0x3f0 [ 491.329797] sysfs_add_battery+0xa4/0x1d0 [battery] [ 491.329805] acpi_battery_update+0x19e/0x290 [battery] [ 491.329809] acpi_battery_notify+0x50/0x120 [battery] [ 491.329812] acpi_ev_notify_dispatch+0x49/0x70 [ 491.329817] acpi_os_execute_deferred+0x1a/0x30 [ 491.329820] process_one_work+0x177/0x330 [ 491.329826] worker_thread+0x251/0x390 [ 491.329830] ? __pfx_worker_thread+0x10/0x10 [ 491.329833] kthread+0xd2/0x100 [ 491.329836] ? __pfx_kthread+0x10/0x10 [ 491.329838] ret_from_fork+0x34/0x50 [ 491.329842] ? __pfx_kthread+0x10/0x10 [ 491.329844] ret_from_fork_asm+0x1a/0x30 [ 491.329850] </TASK> [ 491.329855] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory.
Reported-by: kernel test robot lkp@intel.com Closes: https://lore.kernel.org/r/202509101620.yI0HZ5gT-lkp@intel.com/ Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lore.kernel.org/r/202509101620.yI0HZ5gT-lkp@intel.com/ Fixes: 508df92d1f8d ("ACPI: battery: register power_supply subdevice only when battery is present") Signed-off-by: GuangFei Luo luogf2025@163.com Cc: stable@vger.kernel.org --- drivers/acpi/battery.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 6905b56bf3e4..649185f7a3b1 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -1026,11 +1026,15 @@ static int acpi_battery_update(struct acpi_battery *battery, bool resume) return result; acpi_battery_quirks(battery);
+ mutex_lock(&battery->sysfs_lock); if (!battery->bat) { result = sysfs_add_battery(battery); - if (result) + if (result) { + mutex_unlock(&battery->sysfs_lock); return result; + } } + mutex_unlock(&battery->sysfs_lock);
/* * Wakeup the system if battery is critical low
v3: - Modified the earlier approach: since sysfs_add_battery() is invoked from multiple places, the most reliable way is to add the lock inside the function itself. - sysfs_remove_battery() had a similar race issue in the past, which was fixed by adding a lock as well. Reference: https://lore.kernel.org/all/9c921c22a7f33397a6774d7fa076db9b6a0fd669 .1312318300.git.len.brown@intel.com/
v2: - Fix missing mutex_unlock in acpi_battery_update() (Reported-by: kernel test robot)
v1: When removing and reinserting the laptop battery, ACPI can trigger two notifications in quick succession:
- ACPI_BATTERY_NOTIFY_STATUS (0x80) - ACPI_BATTERY_NOTIFY_INFO (0x81)
Both notifications call acpi_battery_update(). Because the events happen very close in time, sysfs_add_battery() can be re-entered before battery->bat is set, causing a duplicate sysfs entry error.
This patch ensures that sysfs_add_battery() is not re-entered when battery->bat is already non-NULL, preventing the duplicate sysfs creation and stabilizing battery hotplug handling.
[ 476.117945] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' [ 476.118896] CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 [ 476.118903] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 [ 476.118906] Workqueue: kacpi_notify acpi_os_execute_deferred [ 476.118917] Call Trace: [ 476.118922] <TASK> [ 476.118929] dump_stack_lvl+0x5d/0x80 [ 476.118938] sysfs_warn_dup.cold+0x17/0x23 [ 476.118943] sysfs_create_dir_ns+0xce/0xe0 [ 476.118952] kobject_add_internal+0xba/0x250 [ 476.118959] kobject_add+0x96/0xc0 [ 476.118964] ? get_device_parent+0xde/0x1e0 [ 476.118970] device_add+0xe2/0x870 [ 476.118975] __power_supply_register.part.0+0x20f/0x3f0 [ 476.118981] ? wake_up_q+0x4e/0x90 [ 476.118990] sysfs_add_battery+0xa4/0x1d0 [battery] [ 476.118998] acpi_battery_update+0x19e/0x290 [battery] [ 476.119002] acpi_battery_notify+0x50/0x120 [battery] [ 476.119006] acpi_ev_notify_dispatch+0x49/0x70 [ 476.119012] acpi_os_execute_deferred+0x1a/0x30 [ 476.119015] process_one_work+0x177/0x330 [ 476.119022] worker_thread+0x251/0x390 [ 476.119026] ? __pfx_worker_thread+0x10/0x10 [ 476.119030] kthread+0xd2/0x100 [ 476.119033] ? __pfx_kthread+0x10/0x10 [ 476.119035] ret_from_fork+0x34/0x50 [ 476.119040] ? __pfx_kthread+0x10/0x10 [ 476.119042] ret_from_fork_asm+0x1a/0x30 [ 476.119049] </TASK> [ 476.142552] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory. [ 476.415022] ata1.00: unexpected _GTF length (8) [ 476.428076] sd 0:0:0:0: [sda] Starting disk [ 476.835035] ata1.00: unexpected _GTF length (8) [ 476.839720] ata1.00: configured for UDMA/133 [ 491.328831] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' [ 491.329720] CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 [ 491.329727] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 [ 491.329731] Workqueue: kacpi_notify acpi_os_execute_deferred [ 491.329741] Call Trace: [ 491.329745] <TASK> [ 491.329751] dump_stack_lvl+0x5d/0x80 [ 491.329758] sysfs_warn_dup.cold+0x17/0x23 [ 491.329762] sysfs_create_dir_ns+0xce/0xe0 [ 491.329770] kobject_add_internal+0xba/0x250 [ 491.329775] kobject_add+0x96/0xc0 [ 491.329779] ? get_device_parent+0xde/0x1e0 [ 491.329784] device_add+0xe2/0x870 [ 491.329790] __power_supply_register.part.0+0x20f/0x3f0 [ 491.329797] sysfs_add_battery+0xa4/0x1d0 [battery] [ 491.329805] acpi_battery_update+0x19e/0x290 [battery] [ 491.329809] acpi_battery_notify+0x50/0x120 [battery] [ 491.329812] acpi_ev_notify_dispatch+0x49/0x70 [ 491.329817] acpi_os_execute_deferred+0x1a/0x30 [ 491.329820] process_one_work+0x177/0x330 [ 491.329826] worker_thread+0x251/0x390 [ 491.329830] ? __pfx_worker_thread+0x10/0x10 [ 491.329833] kthread+0xd2/0x100 [ 491.329836] ? __pfx_kthread+0x10/0x10 [ 491.329838] ret_from_fork+0x34/0x50 [ 491.329842] ? __pfx_kthread+0x10/0x10 [ 491.329844] ret_from_fork_asm+0x1a/0x30 [ 491.329850] </TASK> [ 491.329855] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory.
Reported-by: kernel test robot lkp@intel.com Closes: https://lore.kernel.org/r/202509101620.yI0HZ5gT-lkp@intel.com/ Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lore.kernel.org/r/202509101620.yI0HZ5gT-lkp@intel.com/ Fixes: 508df92d1f8d ("ACPI: battery: register power_supply subdevice only when battery is present") Signed-off-by: GuangFei Luo luogf2025@163.com Cc: stable@vger.kernel.org --- drivers/acpi/battery.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 6905b56bf3e4..2ee498f0595a 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -850,6 +850,12 @@ static void __exit battery_hook_exit(void)
static int sysfs_add_battery(struct acpi_battery *battery) { + mutex_lock(&battery->sysfs_lock); + if (!battery->bat) { + mutex_unlock(&battery->sysfs_lock); + return 0; + } + struct power_supply_config psy_cfg = { .drv_data = battery, .attr_grp = acpi_battery_groups, @@ -896,9 +902,11 @@ static int sysfs_add_battery(struct acpi_battery *battery) int result = PTR_ERR(battery->bat);
battery->bat = NULL; + mutex_unlock(&battery->sysfs_lock); return result; } battery_hook_add_battery(battery); + mutex_unlock(&battery->sysfs_lock); return 0; }
@@ -1026,11 +1034,9 @@ static int acpi_battery_update(struct acpi_battery *battery, bool resume) return result; acpi_battery_quirks(battery);
- if (!battery->bat) { - result = sysfs_add_battery(battery); - if (result) - return result; - } + result = sysfs_add_battery(battery); + if (result) + return result;
/* * Wakeup the system if battery is critical low
v3: - Modified the earlier approach: since sysfs_add_battery() is invoked from multiple places, the most reliable way is to add the lock inside the function itself. - sysfs_remove_battery() had a similar race issue in the past, which was fixed by adding a lock as well. Reference: https://lore.kernel.org/all/9c921c22a7f33397a6774d7fa076db9b6a0fd669 .1312318300.git.len.brown@intel.com/
v2: - Fix missing mutex_unlock in acpi_battery_update() (Reported-by: kernel test robot)
v1: When removing and reinserting the laptop battery, ACPI can trigger two notifications in quick succession:
- ACPI_BATTERY_NOTIFY_STATUS (0x80) - ACPI_BATTERY_NOTIFY_INFO (0x81)
Both notifications call acpi_battery_update(). Because the events happen very close in time, sysfs_add_battery() can be re-entered before battery->bat is set, causing a duplicate sysfs entry error.
This patch ensures that sysfs_add_battery() is not re-entered when battery->bat is already non-NULL, preventing the duplicate sysfs creation and stabilizing battery hotplug handling.
[ 476.117945] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' [ 476.118896] CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 [ 476.118903] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 [ 476.118906] Workqueue: kacpi_notify acpi_os_execute_deferred [ 476.118917] Call Trace: [ 476.118922] <TASK> [ 476.118929] dump_stack_lvl+0x5d/0x80 [ 476.118938] sysfs_warn_dup.cold+0x17/0x23 [ 476.118943] sysfs_create_dir_ns+0xce/0xe0 [ 476.118952] kobject_add_internal+0xba/0x250 [ 476.118959] kobject_add+0x96/0xc0 [ 476.118964] ? get_device_parent+0xde/0x1e0 [ 476.118970] device_add+0xe2/0x870 [ 476.118975] __power_supply_register.part.0+0x20f/0x3f0 [ 476.118981] ? wake_up_q+0x4e/0x90 [ 476.118990] sysfs_add_battery+0xa4/0x1d0 [battery] [ 476.118998] acpi_battery_update+0x19e/0x290 [battery] [ 476.119002] acpi_battery_notify+0x50/0x120 [battery] [ 476.119006] acpi_ev_notify_dispatch+0x49/0x70 [ 476.119012] acpi_os_execute_deferred+0x1a/0x30 [ 476.119015] process_one_work+0x177/0x330 [ 476.119022] worker_thread+0x251/0x390 [ 476.119026] ? __pfx_worker_thread+0x10/0x10 [ 476.119030] kthread+0xd2/0x100 [ 476.119033] ? __pfx_kthread+0x10/0x10 [ 476.119035] ret_from_fork+0x34/0x50 [ 476.119040] ? __pfx_kthread+0x10/0x10 [ 476.119042] ret_from_fork_asm+0x1a/0x30 [ 476.119049] </TASK> [ 476.142552] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory. [ 476.415022] ata1.00: unexpected _GTF length (8) [ 476.428076] sd 0:0:0:0: [sda] Starting disk [ 476.835035] ata1.00: unexpected _GTF length (8) [ 476.839720] ata1.00: configured for UDMA/133 [ 491.328831] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' [ 491.329720] CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 [ 491.329727] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 [ 491.329731] Workqueue: kacpi_notify acpi_os_execute_deferred [ 491.329741] Call Trace: [ 491.329745] <TASK> [ 491.329751] dump_stack_lvl+0x5d/0x80 [ 491.329758] sysfs_warn_dup.cold+0x17/0x23 [ 491.329762] sysfs_create_dir_ns+0xce/0xe0 [ 491.329770] kobject_add_internal+0xba/0x250 [ 491.329775] kobject_add+0x96/0xc0 [ 491.329779] ? get_device_parent+0xde/0x1e0 [ 491.329784] device_add+0xe2/0x870 [ 491.329790] __power_supply_register.part.0+0x20f/0x3f0 [ 491.329797] sysfs_add_battery+0xa4/0x1d0 [battery] [ 491.329805] acpi_battery_update+0x19e/0x290 [battery] [ 491.329809] acpi_battery_notify+0x50/0x120 [battery] [ 491.329812] acpi_ev_notify_dispatch+0x49/0x70 [ 491.329817] acpi_os_execute_deferred+0x1a/0x30 [ 491.329820] process_one_work+0x177/0x330 [ 491.329826] worker_thread+0x251/0x390 [ 491.329830] ? __pfx_worker_thread+0x10/0x10 [ 491.329833] kthread+0xd2/0x100 [ 491.329836] ? __pfx_kthread+0x10/0x10 [ 491.329838] ret_from_fork+0x34/0x50 [ 491.329842] ? __pfx_kthread+0x10/0x10 [ 491.329844] ret_from_fork_asm+0x1a/0x30 [ 491.329850] </TASK> [ 491.329855] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory.
Reported-by: kernel test robot lkp@intel.com Closes: https://lore.kernel.org/r/202509101620.yI0HZ5gT-lkp@intel.com/ Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lore.kernel.org/r/202509101620.yI0HZ5gT-lkp@intel.com/ Fixes: 508df92d1f8d ("ACPI: battery: register power_supply subdevice only when battery is present") Signed-off-by: GuangFei Luo luogf2025@163.com Cc: stable@vger.kernel.org --- drivers/acpi/battery.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 6905b56bf3e4..f6d4a8b39a9c 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -850,6 +850,12 @@ static void __exit battery_hook_exit(void)
static int sysfs_add_battery(struct acpi_battery *battery) { + mutex_lock(&battery->sysfs_lock); + if (battery->bat) { + mutex_unlock(&battery->sysfs_lock); + return 0; + } + struct power_supply_config psy_cfg = { .drv_data = battery, .attr_grp = acpi_battery_groups, @@ -896,9 +902,11 @@ static int sysfs_add_battery(struct acpi_battery *battery) int result = PTR_ERR(battery->bat);
battery->bat = NULL; + mutex_unlock(&battery->sysfs_lock); return result; } battery_hook_add_battery(battery); + mutex_unlock(&battery->sysfs_lock); return 0; }
@@ -1026,11 +1034,9 @@ static int acpi_battery_update(struct acpi_battery *battery, bool resume) return result; acpi_battery_quirks(battery);
- if (!battery->bat) { - result = sysfs_add_battery(battery); - if (result) - return result; - } + result = sysfs_add_battery(battery); + if (result) + return result;
/* * Wakeup the system if battery is critical low
On Fri, Sep 12, 2025 at 11:48:59PM +0800, GuangFei Luo wrote:
v3:
- Modified the earlier approach: since sysfs_add_battery() is invoked from multiple places, the most reliable way is to add the lock inside the function itself.
- sysfs_remove_battery() had a similar race issue in the past, which was fixed by adding a lock as well. Reference: https://lore.kernel.org/all/9c921c22a7f33397a6774d7fa076db9b6a0fd669
.1312318300.git.len.brown@intel.com/
v2:
- Fix missing mutex_unlock in acpi_battery_update() (Reported-by: kernel test robot)
v1: When removing and reinserting the laptop battery, ACPI can trigger two notifications in quick succession:
- ACPI_BATTERY_NOTIFY_STATUS (0x80)
- ACPI_BATTERY_NOTIFY_INFO (0x81)
Changelog should go below the --- line.
static int sysfs_add_battery(struct acpi_battery *battery) {
- mutex_lock(&battery->sysfs_lock);
Why not just use a guard() here? That would make the logic elsewhere much simpler.
thanks,
greg k-h
Hi Greg,
Thanks for your review and suggestion.
I've updated sysfs_add_battery() to address your comment. The locking is now applied explicitly inside the function to prevent re-entry issues, while keeping the function self-contained for all call sites.
Patch version: v3
Thanks, GuangFei
v3: - Modified the earlier approach: since sysfs_add_battery() is invoked from multiple places, the most reliable way is to add the lock inside the function itself. - sysfs_remove_battery() had a similar race issue in the past, which was fixed by adding a lock as well. Reference: https://lore.kernel.org/all/9c921c22a7f33397a6774d7fa076db9b6a0fd669 .1312318300.git.len.brown@intel.com/
v2: - Fix missing mutex_unlock in acpi_battery_update() (Reported-by: kernel test robot)
v1: When removing and reinserting the laptop battery, ACPI can trigger two notifications in quick succession:
- ACPI_BATTERY_NOTIFY_STATUS (0x80) - ACPI_BATTERY_NOTIFY_INFO (0x81)
Both notifications call acpi_battery_update(). Because the events happen very close in time, sysfs_add_battery() can be re-entered before battery->bat is set, causing a duplicate sysfs entry error.
This patch ensures that sysfs_add_battery() is not re-entered when battery->bat is already non-NULL, preventing the duplicate sysfs creation and stabilizing battery hotplug handling.
[ 476.117945] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' [ 476.118896] CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 [ 476.118903] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 [ 476.118906] Workqueue: kacpi_notify acpi_os_execute_deferred [ 476.118917] Call Trace: [ 476.118922] <TASK> [ 476.118929] dump_stack_lvl+0x5d/0x80 [ 476.118938] sysfs_warn_dup.cold+0x17/0x23 [ 476.118943] sysfs_create_dir_ns+0xce/0xe0 [ 476.118952] kobject_add_internal+0xba/0x250 [ 476.118959] kobject_add+0x96/0xc0 [ 476.118964] ? get_device_parent+0xde/0x1e0 [ 476.118970] device_add+0xe2/0x870 [ 476.118975] __power_supply_register.part.0+0x20f/0x3f0 [ 476.118981] ? wake_up_q+0x4e/0x90 [ 476.118990] sysfs_add_battery+0xa4/0x1d0 [battery] [ 476.118998] acpi_battery_update+0x19e/0x290 [battery] [ 476.119002] acpi_battery_notify+0x50/0x120 [battery] [ 476.119006] acpi_ev_notify_dispatch+0x49/0x70 [ 476.119012] acpi_os_execute_deferred+0x1a/0x30 [ 476.119015] process_one_work+0x177/0x330 [ 476.119022] worker_thread+0x251/0x390 [ 476.119026] ? __pfx_worker_thread+0x10/0x10 [ 476.119030] kthread+0xd2/0x100 [ 476.119033] ? __pfx_kthread+0x10/0x10 [ 476.119035] ret_from_fork+0x34/0x50 [ 476.119040] ? __pfx_kthread+0x10/0x10 [ 476.119042] ret_from_fork_asm+0x1a/0x30 [ 476.119049] </TASK> [ 476.142552] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory. [ 476.415022] ata1.00: unexpected _GTF length (8) [ 476.428076] sd 0:0:0:0: [sda] Starting disk [ 476.835035] ata1.00: unexpected _GTF length (8) [ 476.839720] ata1.00: configured for UDMA/133 [ 491.328831] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' [ 491.329720] CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 [ 491.329727] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 [ 491.329731] Workqueue: kacpi_notify acpi_os_execute_deferred [ 491.329741] Call Trace: [ 491.329745] <TASK> [ 491.329751] dump_stack_lvl+0x5d/0x80 [ 491.329758] sysfs_warn_dup.cold+0x17/0x23 [ 491.329762] sysfs_create_dir_ns+0xce/0xe0 [ 491.329770] kobject_add_internal+0xba/0x250 [ 491.329775] kobject_add+0x96/0xc0 [ 491.329779] ? get_device_parent+0xde/0x1e0 [ 491.329784] device_add+0xe2/0x870 [ 491.329790] __power_supply_register.part.0+0x20f/0x3f0 [ 491.329797] sysfs_add_battery+0xa4/0x1d0 [battery] [ 491.329805] acpi_battery_update+0x19e/0x290 [battery] [ 491.329809] acpi_battery_notify+0x50/0x120 [battery] [ 491.329812] acpi_ev_notify_dispatch+0x49/0x70 [ 491.329817] acpi_os_execute_deferred+0x1a/0x30 [ 491.329820] process_one_work+0x177/0x330 [ 491.329826] worker_thread+0x251/0x390 [ 491.329830] ? __pfx_worker_thread+0x10/0x10 [ 491.329833] kthread+0xd2/0x100 [ 491.329836] ? __pfx_kthread+0x10/0x10 [ 491.329838] ret_from_fork+0x34/0x50 [ 491.329842] ? __pfx_kthread+0x10/0x10 [ 491.329844] ret_from_fork_asm+0x1a/0x30 [ 491.329850] </TASK> [ 491.329855] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory.
Reported-by: kernel test robot lkp@intel.com Closes: https://lore.kernel.org/r/202509101620.yI0HZ5gT-lkp@intel.com/ Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lore.kernel.org/r/202509101620.yI0HZ5gT-lkp@intel.com/ Fixes: 508df92d1f8d ("ACPI: battery: register power_supply subdevice only when battery is present") Signed-off-by: GuangFei Luo luogf2025@163.com Cc: stable@vger.kernel.org --- drivers/acpi/battery.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 6905b56bf3e4..f6d4a8b39a9c 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -850,6 +850,12 @@ static void __exit battery_hook_exit(void)
static int sysfs_add_battery(struct acpi_battery *battery) { + mutex_lock(&battery->sysfs_lock); + if (battery->bat) { + mutex_unlock(&battery->sysfs_lock); + return 0; + } + struct power_supply_config psy_cfg = { .drv_data = battery, .attr_grp = acpi_battery_groups, @@ -896,9 +902,11 @@ static int sysfs_add_battery(struct acpi_battery *battery) int result = PTR_ERR(battery->bat);
battery->bat = NULL; + mutex_unlock(&battery->sysfs_lock); return result; } battery_hook_add_battery(battery); + mutex_unlock(&battery->sysfs_lock); return 0; }
@@ -1026,11 +1034,9 @@ static int acpi_battery_update(struct acpi_battery *battery, bool resume) return result; acpi_battery_quirks(battery);
- if (!battery->bat) { - result = sysfs_add_battery(battery); - if (result) - return result; - } + result = sysfs_add_battery(battery); + if (result) + return result;
/* * Wakeup the system if battery is critical low
On Sat, Sep 13, 2025 at 12:25:16AM +0800, GuangFei Luo wrote:
Hi Greg,
Thanks for your review and suggestion.
I've updated sysfs_add_battery() to address your comment. The locking is now applied explicitly inside the function to prevent re-entry issues, while keeping the function self-contained for all call sites.
Patch version: v3
Thanks, GuangFei
v3:
- Modified the earlier approach: since sysfs_add_battery() is invoked from multiple places, the most reliable way is to add the lock inside the function itself.
- sysfs_remove_battery() had a similar race issue in the past, which was fixed by adding a lock as well. Reference: https://lore.kernel.org/all/9c921c22a7f33397a6774d7fa076db9b6a0fd669
.1312318300.git.len.brown@intel.com/
v2:
- Fix missing mutex_unlock in acpi_battery_update() (Reported-by: kernel test robot)
v1: When removing and reinserting the laptop battery, ACPI can trigger two notifications in quick succession:
Note, none of the above should be here in the changelog body, it should be below the --- line.
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 6905b56bf3e4..f6d4a8b39a9c 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -850,6 +850,12 @@ static void __exit battery_hook_exit(void) static int sysfs_add_battery(struct acpi_battery *battery) {
- mutex_lock(&battery->sysfs_lock);
Again, can you use guard() to make this logic simpler? That would turn this into a much smaller patch.
thanks,
greg k-h
v4: - Uses guard(mutex) for battery->sysfs_lock in sysfs_add_battery(). - Since sysfs_add_battery() now handles the battery->bat check with proper locking,the extra if (!battery->bat) check at the call site has become redundant.
v3: - Modified the earlier approach: since sysfs_add_battery() is invoked from multiple places, the most reliable way is to add the lock inside the function itself. - sysfs_remove_battery() had a similar race issue in the past, which was fixed by adding a lock as well. Reference: https://lore.kernel.org/all/9c921c22a7f33397a6774d7fa076db9b6a0fd669 .1312318300.git.len.brown@intel.com/
v2: - Fix missing mutex_unlock in acpi_battery_update() (Reported-by: kernel test robot)
v1: When removing and reinserting the laptop battery, ACPI can trigger two notifications in quick succession:
- ACPI_BATTERY_NOTIFY_STATUS (0x80) - ACPI_BATTERY_NOTIFY_INFO (0x81)
Both notifications call acpi_battery_update(). Because the events happen very close in time, sysfs_add_battery() can be re-entered before battery->bat is set, causing a duplicate sysfs entry error.
This patch ensures that sysfs_add_battery() is not re-entered when battery->bat is already non-NULL, preventing the duplicate sysfs creation and stabilizing battery hotplug handling.
[ 476.117945] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' [ 476.118896] CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 [ 476.118903] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 [ 476.118906] Workqueue: kacpi_notify acpi_os_execute_deferred [ 476.118917] Call Trace: [ 476.118922] <TASK> [ 476.118929] dump_stack_lvl+0x5d/0x80 [ 476.118938] sysfs_warn_dup.cold+0x17/0x23 [ 476.118943] sysfs_create_dir_ns+0xce/0xe0 [ 476.118952] kobject_add_internal+0xba/0x250 [ 476.118959] kobject_add+0x96/0xc0 [ 476.118964] ? get_device_parent+0xde/0x1e0 [ 476.118970] device_add+0xe2/0x870 [ 476.118975] __power_supply_register.part.0+0x20f/0x3f0 [ 476.118981] ? wake_up_q+0x4e/0x90 [ 476.118990] sysfs_add_battery+0xa4/0x1d0 [battery] [ 476.118998] acpi_battery_update+0x19e/0x290 [battery] [ 476.119002] acpi_battery_notify+0x50/0x120 [battery] [ 476.119006] acpi_ev_notify_dispatch+0x49/0x70 [ 476.119012] acpi_os_execute_deferred+0x1a/0x30 [ 476.119015] process_one_work+0x177/0x330 [ 476.119022] worker_thread+0x251/0x390 [ 476.119026] ? __pfx_worker_thread+0x10/0x10 [ 476.119030] kthread+0xd2/0x100 [ 476.119033] ? __pfx_kthread+0x10/0x10 [ 476.119035] ret_from_fork+0x34/0x50 [ 476.119040] ? __pfx_kthread+0x10/0x10 [ 476.119042] ret_from_fork_asm+0x1a/0x30 [ 476.119049] </TASK> [ 476.142552] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory. [ 476.415022] ata1.00: unexpected _GTF length (8) [ 476.428076] sd 0:0:0:0: [sda] Starting disk [ 476.835035] ata1.00: unexpected _GTF length (8) [ 476.839720] ata1.00: configured for UDMA/133 [ 491.328831] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' [ 491.329720] CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 [ 491.329727] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 [ 491.329731] Workqueue: kacpi_notify acpi_os_execute_deferred [ 491.329741] Call Trace: [ 491.329745] <TASK> [ 491.329751] dump_stack_lvl+0x5d/0x80 [ 491.329758] sysfs_warn_dup.cold+0x17/0x23 [ 491.329762] sysfs_create_dir_ns+0xce/0xe0 [ 491.329770] kobject_add_internal+0xba/0x250 [ 491.329775] kobject_add+0x96/0xc0 [ 491.329779] ? get_device_parent+0xde/0x1e0 [ 491.329784] device_add+0xe2/0x870 [ 491.329790] __power_supply_register.part.0+0x20f/0x3f0 [ 491.329797] sysfs_add_battery+0xa4/0x1d0 [battery] [ 491.329805] acpi_battery_update+0x19e/0x290 [battery] [ 491.329809] acpi_battery_notify+0x50/0x120 [battery] [ 491.329812] acpi_ev_notify_dispatch+0x49/0x70 [ 491.329817] acpi_os_execute_deferred+0x1a/0x30 [ 491.329820] process_one_work+0x177/0x330 [ 491.329826] worker_thread+0x251/0x390 [ 491.329830] ? __pfx_worker_thread+0x10/0x10 [ 491.329833] kthread+0xd2/0x100 [ 491.329836] ? __pfx_kthread+0x10/0x10 [ 491.329838] ret_from_fork+0x34/0x50 [ 491.329842] ? __pfx_kthread+0x10/0x10 [ 491.329844] ret_from_fork_asm+0x1a/0x30 [ 491.329850] </TASK> [ 491.329855] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory.
Reported-by: kernel test robot lkp@intel.com Closes: https://lore.kernel.org/r/202509101620.yI0HZ5gT-lkp@intel.com/ Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lore.kernel.org/r/202509101620.yI0HZ5gT-lkp@intel.com/ Fixes: 508df92d1f8d ("ACPI: battery: register power_supply subdevice only when battery is present") Signed-off-by: GuangFei Luo luogf2025@163.com Cc: stable@vger.kernel.org --- drivers/acpi/battery.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 6905b56bf3e4..20d68f3e881f 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -850,6 +850,10 @@ static void __exit battery_hook_exit(void)
static int sysfs_add_battery(struct acpi_battery *battery) { + guard(mutex)(&battery->sysfs_lock); + if (battery->bat) + return 0; + struct power_supply_config psy_cfg = { .drv_data = battery, .attr_grp = acpi_battery_groups, @@ -1026,11 +1030,9 @@ static int acpi_battery_update(struct acpi_battery *battery, bool resume) return result; acpi_battery_quirks(battery);
- if (!battery->bat) { - result = sysfs_add_battery(battery); - if (result) - return result; - } + result = sysfs_add_battery(battery); + if (result) + return result;
/* * Wakeup the system if battery is critical low @@ -1112,12 +1114,12 @@ static int battery_notify(struct notifier_block *nb, result = acpi_battery_get_info(battery); if (result) return result; - - result = sysfs_add_battery(battery); - if (result) - return result; }
+ result = sysfs_add_battery(battery); + if (result) + return result; + acpi_battery_init_alarm(battery); acpi_battery_get_state(battery); break;
On Sat, Sep 13, 2025 at 03:19:25PM +0800, GuangFei Luo wrote:
v4:
- Uses guard(mutex) for battery->sysfs_lock in sysfs_add_battery().
- Since sysfs_add_battery() now handles the battery->bat check with proper locking,the extra if (!battery->bat) check at the call site has become redundant.
v3:
- Modified the earlier approach: since sysfs_add_battery() is invoked from multiple places, the most reliable way is to add the lock inside the function itself.
- sysfs_remove_battery() had a similar race issue in the past, which was fixed by adding a lock as well. Reference: https://lore.kernel.org/all/9c921c22a7f33397a6774d7fa076db9b6a0fd669
.1312318300.git.len.brown@intel.com/
v2:
- Fix missing mutex_unlock in acpi_battery_update() (Reported-by: kernel test robot)
v1: When removing and reinserting the laptop battery, ACPI can trigger two notifications in quick succession:
Again, all of this goes below the --- line, not above it.
When removing and reinserting the laptop battery, ACPI can trigger two notifications in quick succession: - ACPI_BATTERY_NOTIFY_STATUS (0x80) - ACPI_BATTERY_NOTIFY_INFO (0x81)
Both notifications call acpi_battery_update(). Because the events happen very close in time, sysfs_add_battery() can be re-entered before battery->bat is set, causing a duplicate sysfs entry error.
This patch ensures that sysfs_add_battery() is not re-entered when battery->bat is already non-NULL, preventing the duplicate sysfs creation and stabilizing battery hotplug handling.
[ 476.117945] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' [ 476.118896] CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 [ 476.118903] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 [ 476.118906] Workqueue: kacpi_notify acpi_os_execute_deferred [ 476.118917] Call Trace: [ 476.118922] <TASK> [ 476.118929] dump_stack_lvl+0x5d/0x80 [ 476.118938] sysfs_warn_dup.cold+0x17/0x23 [ 476.118943] sysfs_create_dir_ns+0xce/0xe0 [ 476.118952] kobject_add_internal+0xba/0x250 [ 476.118959] kobject_add+0x96/0xc0 [ 476.118964] ? get_device_parent+0xde/0x1e0 [ 476.118970] device_add+0xe2/0x870 [ 476.118975] __power_supply_register.part.0+0x20f/0x3f0 [ 476.118981] ? wake_up_q+0x4e/0x90 [ 476.118990] sysfs_add_battery+0xa4/0x1d0 [battery] [ 476.118998] acpi_battery_update+0x19e/0x290 [battery] [ 476.119002] acpi_battery_notify+0x50/0x120 [battery] [ 476.119006] acpi_ev_notify_dispatch+0x49/0x70 [ 476.119012] acpi_os_execute_deferred+0x1a/0x30 [ 476.119015] process_one_work+0x177/0x330 [ 476.119022] worker_thread+0x251/0x390 [ 476.119026] ? __pfx_worker_thread+0x10/0x10 [ 476.119030] kthread+0xd2/0x100 [ 476.119033] ? __pfx_kthread+0x10/0x10 [ 476.119035] ret_from_fork+0x34/0x50 [ 476.119040] ? __pfx_kthread+0x10/0x10 [ 476.119042] ret_from_fork_asm+0x1a/0x30 [ 476.119049] </TASK> [ 476.142552] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory. [ 476.415022] ata1.00: unexpected _GTF length (8) [ 476.428076] sd 0:0:0:0: [sda] Starting disk [ 476.835035] ata1.00: unexpected _GTF length (8) [ 476.839720] ata1.00: configured for UDMA/133 [ 491.328831] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' [ 491.329720] CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 [ 491.329727] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 [ 491.329731] Workqueue: kacpi_notify acpi_os_execute_deferred [ 491.329741] Call Trace: [ 491.329745] <TASK> [ 491.329751] dump_stack_lvl+0x5d/0x80 [ 491.329758] sysfs_warn_dup.cold+0x17/0x23 [ 491.329762] sysfs_create_dir_ns+0xce/0xe0 [ 491.329770] kobject_add_internal+0xba/0x250 [ 491.329775] kobject_add+0x96/0xc0 [ 491.329779] ? get_device_parent+0xde/0x1e0 [ 491.329784] device_add+0xe2/0x870 [ 491.329790] __power_supply_register.part.0+0x20f/0x3f0 [ 491.329797] sysfs_add_battery+0xa4/0x1d0 [battery] [ 491.329805] acpi_battery_update+0x19e/0x290 [battery] [ 491.329809] acpi_battery_notify+0x50/0x120 [battery] [ 491.329812] acpi_ev_notify_dispatch+0x49/0x70 [ 491.329817] acpi_os_execute_deferred+0x1a/0x30 [ 491.329820] process_one_work+0x177/0x330 [ 491.329826] worker_thread+0x251/0x390 [ 491.329830] ? __pfx_worker_thread+0x10/0x10 [ 491.329833] kthread+0xd2/0x100 [ 491.329836] ? __pfx_kthread+0x10/0x10 [ 491.329838] ret_from_fork+0x34/0x50 [ 491.329842] ? __pfx_kthread+0x10/0x10 [ 491.329844] ret_from_fork_asm+0x1a/0x30 [ 491.329850] </TASK> [ 491.329855] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory.
Reported-by: kernel test robot lkp@intel.com Closes: https://lore.kernel.org/r/202509101620.yI0HZ5gT-lkp@intel.com/ Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lore.kernel.org/r/202509101620.yI0HZ5gT-lkp@intel.com/ Fixes: 508df92d1f8d ("ACPI: battery: register power_supply subdevice only when battery is present") Signed-off-by: GuangFei Luo luogf2025@163.com Cc: stable@vger.kernel.org --- v5: - Move changelog above the '---' line as per submission guidelines.
v4: - Uses guard(mutex) for battery->sysfs_lock in sysfs_add_battery(). - Since sysfs_add_battery() now handles the battery->bat check with proper locking,the extra if (!battery->bat) check at the call site has become redundant.
v3: - Modified the earlier approach: since sysfs_add_battery() is invoked from multiple places, the most reliable way is to add the lock inside the function itself. - sysfs_remove_battery() had a similar race issue in the past, which was fixed by adding a lock as well. Reference: https://lore.kernel.org/all/9c921c22a7f33397a6774d7fa076db9b6a0fd669 .1312318300.git.len.brown@intel.com/
v2: - Fix missing mutex_unlock in acpi_battery_update() (Reported-by: kernel test robot)
v1: - Initial patch to handle race when hotplugging battery, preventing duplicate sysfs entries. --- drivers/acpi/battery.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 6905b56bf3e4..20d68f3e881f 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -850,6 +850,10 @@ static void __exit battery_hook_exit(void)
static int sysfs_add_battery(struct acpi_battery *battery) { + guard(mutex)(&battery->sysfs_lock); + if (battery->bat) + return 0; + struct power_supply_config psy_cfg = { .drv_data = battery, .attr_grp = acpi_battery_groups, @@ -1026,11 +1030,9 @@ static int acpi_battery_update(struct acpi_battery *battery, bool resume) return result; acpi_battery_quirks(battery);
- if (!battery->bat) { - result = sysfs_add_battery(battery); - if (result) - return result; - } + result = sysfs_add_battery(battery); + if (result) + return result;
/* * Wakeup the system if battery is critical low @@ -1112,12 +1114,12 @@ static int battery_notify(struct notifier_block *nb, result = acpi_battery_get_info(battery); if (result) return result; - - result = sysfs_add_battery(battery); - if (result) - return result; }
+ result = sysfs_add_battery(battery); + if (result) + return result; + acpi_battery_init_alarm(battery); acpi_battery_get_state(battery); break;
When removing and reinserting the laptop battery, ACPI can trigger two notifications in quick succession: - ACPI_BATTERY_NOTIFY_STATUS (0x80) - ACPI_BATTERY_NOTIFY_INFO (0x81)
Both notifications call acpi_battery_update(). Because the events happen very close in time, sysfs_add_battery() can be re-entered before battery->bat is set, causing a duplicate sysfs entry error.
This patch ensures that sysfs_add_battery() is not re-entered when battery->bat is already non-NULL, preventing the duplicate sysfs creation and stabilizing battery hotplug handling.
[ 476.117945] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' [ 476.118896] CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 [ 476.118903] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 [ 476.118906] Workqueue: kacpi_notify acpi_os_execute_deferred [ 476.118917] Call Trace: [ 476.118922] <TASK> [ 476.118929] dump_stack_lvl+0x5d/0x80 [ 476.118938] sysfs_warn_dup.cold+0x17/0x23 [ 476.118943] sysfs_create_dir_ns+0xce/0xe0 [ 476.118952] kobject_add_internal+0xba/0x250 [ 476.118959] kobject_add+0x96/0xc0 [ 476.118964] ? get_device_parent+0xde/0x1e0 [ 476.118970] device_add+0xe2/0x870 [ 476.118975] __power_supply_register.part.0+0x20f/0x3f0 [ 476.118981] ? wake_up_q+0x4e/0x90 [ 476.118990] sysfs_add_battery+0xa4/0x1d0 [battery] [ 476.118998] acpi_battery_update+0x19e/0x290 [battery] [ 476.119002] acpi_battery_notify+0x50/0x120 [battery] [ 476.119006] acpi_ev_notify_dispatch+0x49/0x70 [ 476.119012] acpi_os_execute_deferred+0x1a/0x30 [ 476.119015] process_one_work+0x177/0x330 [ 476.119022] worker_thread+0x251/0x390 [ 476.119026] ? __pfx_worker_thread+0x10/0x10 [ 476.119030] kthread+0xd2/0x100 [ 476.119033] ? __pfx_kthread+0x10/0x10 [ 476.119035] ret_from_fork+0x34/0x50 [ 476.119040] ? __pfx_kthread+0x10/0x10 [ 476.119042] ret_from_fork_asm+0x1a/0x30 [ 476.119049] </TASK> [ 476.142552] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory. [ 476.415022] ata1.00: unexpected _GTF length (8) [ 476.428076] sd 0:0:0:0: [sda] Starting disk [ 476.835035] ata1.00: unexpected _GTF length (8) [ 476.839720] ata1.00: configured for UDMA/133 [ 491.328831] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' [ 491.329720] CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 [ 491.329727] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 [ 491.329731] Workqueue: kacpi_notify acpi_os_execute_deferred [ 491.329741] Call Trace: [ 491.329745] <TASK> [ 491.329751] dump_stack_lvl+0x5d/0x80 [ 491.329758] sysfs_warn_dup.cold+0x17/0x23 [ 491.329762] sysfs_create_dir_ns+0xce/0xe0 [ 491.329770] kobject_add_internal+0xba/0x250 [ 491.329775] kobject_add+0x96/0xc0 [ 491.329779] ? get_device_parent+0xde/0x1e0 [ 491.329784] device_add+0xe2/0x870 [ 491.329790] __power_supply_register.part.0+0x20f/0x3f0 [ 491.329797] sysfs_add_battery+0xa4/0x1d0 [battery] [ 491.329805] acpi_battery_update+0x19e/0x290 [battery] [ 491.329809] acpi_battery_notify+0x50/0x120 [battery] [ 491.329812] acpi_ev_notify_dispatch+0x49/0x70 [ 491.329817] acpi_os_execute_deferred+0x1a/0x30 [ 491.329820] process_one_work+0x177/0x330 [ 491.329826] worker_thread+0x251/0x390 [ 491.329830] ? __pfx_worker_thread+0x10/0x10 [ 491.329833] kthread+0xd2/0x100 [ 491.329836] ? __pfx_kthread+0x10/0x10 [ 491.329838] ret_from_fork+0x34/0x50 [ 491.329842] ? __pfx_kthread+0x10/0x10 [ 491.329844] ret_from_fork_asm+0x1a/0x30 [ 491.329850] </TASK> [ 491.329855] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory.
Reported-by: kernel test robot lkp@intel.com Closes: https://lore.kernel.org/r/202509101620.yI0HZ5gT-lkp@intel.com/ Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lore.kernel.org/r/202509101620.yI0HZ5gT-lkp@intel.com/ Fixes: 10666251554c ("ACPI: battery: Install Notify() handler directly") Signed-off-by: GuangFei Luo luogf2025@163.com Cc: stable@vger.kernel.org --- v6: - Update Fixes tag: point to commit 10666251554c ("ACPI: battery: Install Notify() handler directly"), which introduced the sysfs_add_battery() re-entry issue when acpi_battery_notify is registered via acpi_dev_install_notify_handler(). The problem does not occur with acpi_bus_register_driver().
v5: - Move changelog above the '---' line as per submission guidelines.
v4: - Uses guard(mutex) for battery->sysfs_lock in sysfs_add_battery(). - Since sysfs_add_battery() now handles the battery->bat check with proper locking,the extra if (!battery->bat) check at the call site has become redundant.
v3: - Modified the earlier approach: since sysfs_add_battery() is invoked from multiple places, the most reliable way is to add the lock inside the function itself. - sysfs_remove_battery() had a similar race issue in the past, which was fixed by adding a lock as well. Reference: https://lore.kernel.org/all/9c921c22a7f33397a6774d7fa076db9b6a0fd669 .1312318300.git.len.brown@intel.com/
v2: - Fix missing mutex_unlock in acpi_battery_update() (Reported-by: kernel test robot)
v1: - Initial patch to handle race when hotplugging battery, preventing duplicate sysfs entries. --- drivers/acpi/battery.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 6905b56bf3e4..20d68f3e881f 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -850,6 +850,10 @@ static void __exit battery_hook_exit(void)
static int sysfs_add_battery(struct acpi_battery *battery) { + guard(mutex)(&battery->sysfs_lock); + if (battery->bat) + return 0; + struct power_supply_config psy_cfg = { .drv_data = battery, .attr_grp = acpi_battery_groups, @@ -1026,11 +1030,9 @@ static int acpi_battery_update(struct acpi_battery *battery, bool resume) return result; acpi_battery_quirks(battery);
- if (!battery->bat) { - result = sysfs_add_battery(battery); - if (result) - return result; - } + result = sysfs_add_battery(battery); + if (result) + return result;
/* * Wakeup the system if battery is critical low @@ -1112,12 +1114,12 @@ static int battery_notify(struct notifier_block *nb, result = acpi_battery_get_info(battery); if (result) return result; - - result = sysfs_add_battery(battery); - if (result) - return result; }
+ result = sysfs_add_battery(battery); + if (result) + return result; + acpi_battery_init_alarm(battery); acpi_battery_get_state(battery); break;
From: Guofeng Luo luogf2025@163.com
On Thu, Sep 18, 2025 at 3:56 PM GuangFei Luo luogf2025@163.com wrote:
When removing and reinserting the laptop battery, ACPI can trigger two notifications in quick succession:
- ACPI_BATTERY_NOTIFY_STATUS (0x80)
- ACPI_BATTERY_NOTIFY_INFO (0x81)
Both notifications call acpi_battery_update(). Because the events happen very close in time, sysfs_add_battery() can be re-entered before battery->bat is set, causing a duplicate sysfs entry error.
When the ACPI battery driver uses acpi_dev_install_notify_handler() to register acpi_battery_notify, the callback may be triggered twice in a very short period of time.
This patch ensures that sysfs_add_battery() is not re-entered when battery->bat is already non-NULL, preventing the duplicate sysfs creation and stabilizing battery hotplug handling.
[ 476.117945] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' [ 476.118896] CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 [ 476.118903] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 [ 476.118906] Workqueue: kacpi_notify acpi_os_execute_deferred [ 476.118917] Call Trace: [ 476.118922] <TASK> [ 476.118929] dump_stack_lvl+0x5d/0x80 [ 476.118938] sysfs_warn_dup.cold+0x17/0x23 [ 476.118943] sysfs_create_dir_ns+0xce/0xe0 [ 476.118952] kobject_add_internal+0xba/0x250 [ 476.118959] kobject_add+0x96/0xc0 [ 476.118964] ? get_device_parent+0xde/0x1e0 [ 476.118970] device_add+0xe2/0x870 [ 476.118975] __power_supply_register.part.0+0x20f/0x3f0 [ 476.118981] ? wake_up_q+0x4e/0x90 [ 476.118990] sysfs_add_battery+0xa4/0x1d0 [battery] [ 476.118998] acpi_battery_update+0x19e/0x290 [battery] [ 476.119002] acpi_battery_notify+0x50/0x120 [battery] [ 476.119006] acpi_ev_notify_dispatch+0x49/0x70 [ 476.119012] acpi_os_execute_deferred+0x1a/0x30 [ 476.119015] process_one_work+0x177/0x330 [ 476.119022] worker_thread+0x251/0x390 [ 476.119026] ? __pfx_worker_thread+0x10/0x10 [ 476.119030] kthread+0xd2/0x100 [ 476.119033] ? __pfx_kthread+0x10/0x10 [ 476.119035] ret_from_fork+0x34/0x50 [ 476.119040] ? __pfx_kthread+0x10/0x10 [ 476.119042] ret_from_fork_asm+0x1a/0x30 [ 476.119049] </TASK> [ 476.142552] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory. [ 476.415022] ata1.00: unexpected _GTF length (8) [ 476.428076] sd 0:0:0:0: [sda] Starting disk [ 476.835035] ata1.00: unexpected _GTF length (8) [ 476.839720] ata1.00: configured for UDMA/133 [ 491.328831] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' [ 491.329720] CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 [ 491.329727] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 [ 491.329731] Workqueue: kacpi_notify acpi_os_execute_deferred [ 491.329741] Call Trace: [ 491.329745] <TASK> [ 491.329751] dump_stack_lvl+0x5d/0x80 [ 491.329758] sysfs_warn_dup.cold+0x17/0x23 [ 491.329762] sysfs_create_dir_ns+0xce/0xe0 [ 491.329770] kobject_add_internal+0xba/0x250 [ 491.329775] kobject_add+0x96/0xc0 [ 491.329779] ? get_device_parent+0xde/0x1e0 [ 491.329784] device_add+0xe2/0x870 [ 491.329790] __power_supply_register.part.0+0x20f/0x3f0 [ 491.329797] sysfs_add_battery+0xa4/0x1d0 [battery] [ 491.329805] acpi_battery_update+0x19e/0x290 [battery] [ 491.329809] acpi_battery_notify+0x50/0x120 [battery] [ 491.329812] acpi_ev_notify_dispatch+0x49/0x70 [ 491.329817] acpi_os_execute_deferred+0x1a/0x30 [ 491.329820] process_one_work+0x177/0x330 [ 491.329826] worker_thread+0x251/0x390 [ 491.329830] ? __pfx_worker_thread+0x10/0x10 [ 491.329833] kthread+0xd2/0x100 [ 491.329836] ? __pfx_kthread+0x10/0x10 [ 491.329838] ret_from_fork+0x34/0x50 [ 491.329842] ? __pfx_kthread+0x10/0x10 [ 491.329844] ret_from_fork_asm+0x1a/0x30 [ 491.329850] </TASK> [ 491.329855] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory.
Fixes: 10666251554c ("ACPI: battery: Install Notify() handler directly") Signed-off-by: GuangFei Luo luogf2025@163.com Cc: stable@vger.kernel.org
v6:
- Update Fixes tag: point to commit 10666251554c ("ACPI: battery: Install Notify() handler directly"), which introduced the sysfs_add_battery() re-entry issue when acpi_battery_notify is registered via acpi_dev_install_notify_handler(). The problem does not occur with acpi_bus_register_driver().
v5:
- Move changelog above the '---' line as per submission guidelines.
v4:
- Uses guard(mutex) for battery->sysfs_lock in sysfs_add_battery().
- Since sysfs_add_battery() now handles the battery->bat check with proper locking,the extra if (!battery->bat) check at the call site has become redundant.
v3:
- Modified the earlier approach: since sysfs_add_battery() is invoked from multiple places, the most reliable way is to add the lock inside the function itself.
- sysfs_remove_battery() had a similar race issue in the past, which was fixed by adding a lock as well. Reference: https://lore.kernel.org/all/9c921c22a7f33397a6774d7fa076db9b6a0fd669 .1312318300.git.len.brown@intel.com/
v2:
- Fix missing mutex_unlock in acpi_battery_update() (Reported-by: kernel test robot)
v1:
- Initial patch to handle race when hotplugging battery, preventing duplicate sysfs entries.
drivers/acpi/battery.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 6905b56bf3e4..20d68f3e881f 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -850,6 +850,10 @@ static void __exit battery_hook_exit(void)
static int sysfs_add_battery(struct acpi_battery *battery) {
guard(mutex)(&battery->sysfs_lock);if (battery->bat)return 0;struct power_supply_config psy_cfg = { .drv_data = battery, .attr_grp = acpi_battery_groups,@@ -1026,11 +1030,9 @@ static int acpi_battery_update(struct acpi_battery *battery, bool resume) return result; acpi_battery_quirks(battery);
if (!battery->bat) {result = sysfs_add_battery(battery);if (result)return result;}
result = sysfs_add_battery(battery);if (result)return result; /* * Wakeup the system if battery is critical low@@ -1112,12 +1114,12 @@ static int battery_notify(struct notifier_block *nb, result = acpi_battery_get_info(battery); if (result) return result;
result = sysfs_add_battery(battery);if (result)return result; }
result = sysfs_add_battery(battery);if (result)return result;Why is this change necessary?
Hi Rafael,
In the previous code:
if (battery->bat) { acpi_battery_refresh(battery); } else { result = acpi_battery_get_info(battery); if (result) return result;
result = sysfs_add_battery(battery); if (result) return result; }
the `if (!battery->bat)` check was done at the call site. However, this check is not atomic: two threads can both see `battery->bat == NULL` and then call `sysfs_add_battery()` concurrently, leading to duplicate sysfs registration and `-EEXIST` errors.
By moving the check and mutex into `sysfs_add_battery()`, the race is handled atomically. All call sites can now invoke it unconditionally, and the function itself will no-op if the battery is already registered. This avoids duplicated `if (!battery->bat)` logic and ensures consistent protection everywhere.
Thanks, GuangFei
acpi_battery_init_alarm(battery); acpi_battery_get_state(battery); break;--
On Tue, Sep 23, 2025 at 4:39 AM GuangFei Luo luogf2025@163.com wrote:
From: Guofeng Luo luogf2025@163.com
On Thu, Sep 18, 2025 at 3:56 PM GuangFei Luo luogf2025@163.com wrote:
When removing and reinserting the laptop battery, ACPI can trigger two notifications in quick succession:
- ACPI_BATTERY_NOTIFY_STATUS (0x80)
- ACPI_BATTERY_NOTIFY_INFO (0x81)
Both notifications call acpi_battery_update(). Because the events happen very close in time, sysfs_add_battery() can be re-entered before battery->bat is set, causing a duplicate sysfs entry error.
When the ACPI battery driver uses acpi_dev_install_notify_handler() to register acpi_battery_notify, the callback may be triggered twice in a very short period of time.
This patch ensures that sysfs_add_battery() is not re-entered when battery->bat is already non-NULL, preventing the duplicate sysfs creation and stabilizing battery hotplug handling.
[ 476.117945] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' [ 476.118896] CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 [ 476.118903] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 [ 476.118906] Workqueue: kacpi_notify acpi_os_execute_deferred [ 476.118917] Call Trace: [ 476.118922] <TASK> [ 476.118929] dump_stack_lvl+0x5d/0x80 [ 476.118938] sysfs_warn_dup.cold+0x17/0x23 [ 476.118943] sysfs_create_dir_ns+0xce/0xe0 [ 476.118952] kobject_add_internal+0xba/0x250 [ 476.118959] kobject_add+0x96/0xc0 [ 476.118964] ? get_device_parent+0xde/0x1e0 [ 476.118970] device_add+0xe2/0x870 [ 476.118975] __power_supply_register.part.0+0x20f/0x3f0 [ 476.118981] ? wake_up_q+0x4e/0x90 [ 476.118990] sysfs_add_battery+0xa4/0x1d0 [battery] [ 476.118998] acpi_battery_update+0x19e/0x290 [battery] [ 476.119002] acpi_battery_notify+0x50/0x120 [battery] [ 476.119006] acpi_ev_notify_dispatch+0x49/0x70 [ 476.119012] acpi_os_execute_deferred+0x1a/0x30 [ 476.119015] process_one_work+0x177/0x330 [ 476.119022] worker_thread+0x251/0x390 [ 476.119026] ? __pfx_worker_thread+0x10/0x10 [ 476.119030] kthread+0xd2/0x100 [ 476.119033] ? __pfx_kthread+0x10/0x10 [ 476.119035] ret_from_fork+0x34/0x50 [ 476.119040] ? __pfx_kthread+0x10/0x10 [ 476.119042] ret_from_fork_asm+0x1a/0x30 [ 476.119049] </TASK> [ 476.142552] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory. [ 476.415022] ata1.00: unexpected _GTF length (8) [ 476.428076] sd 0:0:0:0: [sda] Starting disk [ 476.835035] ata1.00: unexpected _GTF length (8) [ 476.839720] ata1.00: configured for UDMA/133 [ 491.328831] sysfs: cannot create duplicate filename '/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0C0A:00/power_supply/BAT1' [ 491.329720] CPU: 1 UID: 0 PID: 185 Comm: kworker/1:4 Kdump: loaded Not tainted 6.12.38+deb13-amd64 #1 Debian 6.12.38-1 [ 491.329727] Hardware name: Gateway NV44 /SJV40-MV , BIOS V1.3121 04/08/2009 [ 491.329731] Workqueue: kacpi_notify acpi_os_execute_deferred [ 491.329741] Call Trace: [ 491.329745] <TASK> [ 491.329751] dump_stack_lvl+0x5d/0x80 [ 491.329758] sysfs_warn_dup.cold+0x17/0x23 [ 491.329762] sysfs_create_dir_ns+0xce/0xe0 [ 491.329770] kobject_add_internal+0xba/0x250 [ 491.329775] kobject_add+0x96/0xc0 [ 491.329779] ? get_device_parent+0xde/0x1e0 [ 491.329784] device_add+0xe2/0x870 [ 491.329790] __power_supply_register.part.0+0x20f/0x3f0 [ 491.329797] sysfs_add_battery+0xa4/0x1d0 [battery] [ 491.329805] acpi_battery_update+0x19e/0x290 [battery] [ 491.329809] acpi_battery_notify+0x50/0x120 [battery] [ 491.329812] acpi_ev_notify_dispatch+0x49/0x70 [ 491.329817] acpi_os_execute_deferred+0x1a/0x30 [ 491.329820] process_one_work+0x177/0x330 [ 491.329826] worker_thread+0x251/0x390 [ 491.329830] ? __pfx_worker_thread+0x10/0x10 [ 491.329833] kthread+0xd2/0x100 [ 491.329836] ? __pfx_kthread+0x10/0x10 [ 491.329838] ret_from_fork+0x34/0x50 [ 491.329842] ? __pfx_kthread+0x10/0x10 [ 491.329844] ret_from_fork_asm+0x1a/0x30 [ 491.329850] </TASK> [ 491.329855] kobject: kobject_add_internal failed for BAT1 with -EEXIST, don't try to register things with the same name in the same directory.
Fixes: 10666251554c ("ACPI: battery: Install Notify() handler directly") Signed-off-by: GuangFei Luo luogf2025@163.com Cc: stable@vger.kernel.org
v6:
- Update Fixes tag: point to commit 10666251554c ("ACPI: battery: Install Notify() handler directly"), which introduced the sysfs_add_battery() re-entry issue when acpi_battery_notify is registered via acpi_dev_install_notify_handler(). The problem does not occur with acpi_bus_register_driver().
v5:
- Move changelog above the '---' line as per submission guidelines.
v4:
- Uses guard(mutex) for battery->sysfs_lock in sysfs_add_battery().
- Since sysfs_add_battery() now handles the battery->bat check with proper locking,the extra if (!battery->bat) check at the call site has become redundant.
v3:
- Modified the earlier approach: since sysfs_add_battery() is invoked from multiple places, the most reliable way is to add the lock inside the function itself.
- sysfs_remove_battery() had a similar race issue in the past, which was fixed by adding a lock as well. Reference: https://lore.kernel.org/all/9c921c22a7f33397a6774d7fa076db9b6a0fd669 .1312318300.git.len.brown@intel.com/
v2:
- Fix missing mutex_unlock in acpi_battery_update() (Reported-by: kernel test robot)
v1:
- Initial patch to handle race when hotplugging battery, preventing duplicate sysfs entries.
drivers/acpi/battery.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 6905b56bf3e4..20d68f3e881f 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -850,6 +850,10 @@ static void __exit battery_hook_exit(void)
static int sysfs_add_battery(struct acpi_battery *battery) {
guard(mutex)(&battery->sysfs_lock);if (battery->bat)return 0;struct power_supply_config psy_cfg = { .drv_data = battery, .attr_grp = acpi_battery_groups,@@ -1026,11 +1030,9 @@ static int acpi_battery_update(struct acpi_battery *battery, bool resume) return result; acpi_battery_quirks(battery);
if (!battery->bat) {result = sysfs_add_battery(battery);if (result)return result;}
result = sysfs_add_battery(battery);if (result)return result; /* * Wakeup the system if battery is critical low@@ -1112,12 +1114,12 @@ static int battery_notify(struct notifier_block *nb, result = acpi_battery_get_info(battery); if (result) return result;
result = sysfs_add_battery(battery);if (result)return result; }
result = sysfs_add_battery(battery);if (result)return result;Why is this change necessary?
Hi Rafael,
In the previous code:
if (battery->bat) { acpi_battery_refresh(battery); } else { result = acpi_battery_get_info(battery); if (result) return result; result = sysfs_add_battery(battery); if (result) return result; }the `if (!battery->bat)` check was done at the call site. However, this check is not atomic: two threads can both see `battery->bat == NULL` and then call `sysfs_add_battery()` concurrently, leading to duplicate sysfs registration and `-EEXIST` errors.
By moving the check and mutex into `sysfs_add_battery()`, the race is handled atomically. All call sites can now invoke it unconditionally, and the function itself will no-op if the battery is already registered. This avoids duplicated `if (!battery->bat)` logic and ensures consistent protection everywhere.
Since the lock is acquired in sysfs_add_battery(), the race is addressed regardless of the change in question. The only difference is when the previous battery->bat check passes (that is, battery->bat is not NULL to start with), and if battery->bat becomes NULL between the two checks, the entire section of the code checking battery->bat needs to go under the lock.
I think that all battery->bat accesses need to be done under the lock now, don't they?
The functions battery_hook_add_battery(), battery_hook_remove_battery(), and sysfs_remove_battery() already acquire locks, so their internal accesses are safe.
acpi_battery_refresh() does check battery->bat, but its child functions (sysfs_add_battery() and sysfs_remove_battery()) already handle locking.
In acpi_battery_notify(), battery->bat has no lock. However, the check of battery->bat is at the very end of the function. During earlier calls, battery->bat has already been protected by locks, so re-entry will not cause issues.
On Tue, Sep 23, 2025 at 6:14 PM GuangFei Luo luogf2025@163.com wrote:
The functions battery_hook_add_battery(), battery_hook_remove_battery(), and sysfs_remove_battery() already acquire locks, so their internal accesses are safe.
In fact, there are two locks in use, battery->sysfs_lock and hook_mutex. The latter is used for managing hooks and the former is only used by sysfs_remove_battery(), so it only prevents that function from racing with another instance of itself.
I would suggest using battery->sysfs_lock for protecting battery->bat in general.
acpi_battery_refresh() does check battery->bat, but its child functions (sysfs_add_battery() and sysfs_remove_battery()) already handle locking.
What locking? Before the $subject patch, sysfs_add_battery() doesn't do any locking at all AFAICS.
In acpi_battery_notify(), battery->bat has no lock. However, the check of battery->bat is at the very end of the function. During earlier calls, battery->bat has already been protected by locks, so re-entry will not cause issues.
All of the battery->bat checks and the code depending on them need to go under the same lock. I'd use battery->sysfs_lock for this as already mentioned above.
Yes, in my tests with battery hot-plug, I observed that acpi_battery_notify() can be triggered multiple times in quick succession. Only adding a lock inside acpi_battery_notify() is sufficient.
The functions battery_hook_add_battery(), battery_hook_remove_battery(), and sysfs_remove_battery() already acquire locks, so their internal accesses are safe.
acpi_battery_refresh() does check battery->bat, but its child functions (sysfs_add_battery() and sysfs_remove_battery()) already handle locking.
In acpi_battery_notify(), battery->bat has no lock. However, the check of battery->bat is at the very end of the function. During earlier calls, battery->bat has already been protected by locks, so re-entry will not cause issues.
linux-stable-mirror@lists.linaro.org
-
Greg KH -
GuangFei Luo -
Rafael J. Wysocki