mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-08-30 16:53:20 -04:00
i3c: master: Fix recursive locking during device registration
i3c_master_register_new_i3c_devs() registers newly discovered devices
while holding i3c_bus_normaluse_lock(), a down_read(). device_register()
can immediately probe the device, and probe callbacks typically invoke
I3C helpers that take i3c_bus_normaluse_lock() again, leading to a
recursive acquisition of the same rwsem. rwsems do not support recursive
read locking and can deadlock when a writer is waiting. See the
"Recursive read locks" section of Documentation/locking/lockdep-design.rst.
For example, with Intel LPSS I3C, LOCKDEP generates a WARNING like:
# echo intel-lpss-i3c.0 > /sys/bus/platform/drivers/mipi-i3c-hci/unbind
# echo intel-lpss-i3c.0 > /sys/bus/platform/drivers/mipi-i3c-hci/bind
WARNING: possible recursive locking detected
kworker/5:1/94 is trying to acquire lock:
ffff88811c810d78 (&i3cbus->lock){++++}-{4:4}, at: i3c_device_match_id+0x45/0x370
but task is already holding lock:
ffff88811c810d78 (&i3cbus->lock){++++}-{4:4}, at: i3c_master_reg_work_fn+0x21/0x5f0
Fix this by separating device creation from device registration.
Populate desc->dev under the maintenance lock, collect the devices that
still need registration into a local list, then release the lock before
calling device_register(). Finally retake the lock and clean up any
devices that failed to register.
Use the maintenance lock rather than the normal-use lock while adding
device objects. A write-side maintenance lock prevents readers from
observing a partially initialized desc->dev during initial device
population, or desc->dev disappearing if registration fails.
The local list requires a list node, so add a list node member to struct
i3c_device.
Fixes: 3a379bbcea ("i3c: Add core I3C infrastructure")
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Link: https://patch.msgid.link/20260807145638.168865-2-adrian.hunter@intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
This commit is contained in:
committed by
Alexandre Belloni
parent
2cdb9c2138
commit
456f832e5f
@@ -2069,12 +2069,21 @@ static int i3c_master_early_i3c_dev_add(struct i3c_master_controller *master,
|
||||
static void
|
||||
i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
|
||||
{
|
||||
struct i3c_device *i3cdev, *tmp;
|
||||
struct i3c_dev_desc *desc;
|
||||
LIST_HEAD(i3c_unreg_devs);
|
||||
int ret;
|
||||
|
||||
if (!master->init_done)
|
||||
return;
|
||||
|
||||
i3c_bus_maintenance_lock(&master->bus);
|
||||
|
||||
if (master->shutting_down) {
|
||||
i3c_bus_maintenance_unlock(&master->bus);
|
||||
return;
|
||||
}
|
||||
|
||||
i3c_bus_for_each_i3cdev(&master->bus, desc) {
|
||||
if (desc->dev || !desc->info.dyn_addr || desc == master->this)
|
||||
continue;
|
||||
@@ -2104,25 +2113,37 @@ i3c_master_register_new_i3c_devs(struct i3c_master_controller *master)
|
||||
if (desc->boardinfo)
|
||||
device_set_node(&desc->dev->dev, desc->boardinfo->fwnode);
|
||||
|
||||
ret = device_register(&desc->dev->dev);
|
||||
if (ret) {
|
||||
dev_err(&master->dev,
|
||||
"Failed to add I3C device (err = %d)\n", ret);
|
||||
desc->dev->desc = NULL;
|
||||
put_device(&desc->dev->dev);
|
||||
desc->dev = NULL;
|
||||
}
|
||||
list_add_tail(&desc->dev->node, &i3c_unreg_devs);
|
||||
}
|
||||
|
||||
i3c_bus_maintenance_unlock(&master->bus);
|
||||
|
||||
list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
|
||||
ret = device_register(&i3cdev->dev);
|
||||
if (ret)
|
||||
dev_err(&master->dev, "Failed to add I3C device (err = %d)\n", ret);
|
||||
else
|
||||
list_del_init(&i3cdev->node);
|
||||
}
|
||||
|
||||
i3c_bus_maintenance_lock(&master->bus);
|
||||
|
||||
list_for_each_entry_safe(i3cdev, tmp, &i3c_unreg_devs, node) {
|
||||
list_del(&i3cdev->node);
|
||||
desc = i3cdev->desc;
|
||||
i3cdev->desc = NULL;
|
||||
put_device(&i3cdev->dev);
|
||||
desc->dev = NULL;
|
||||
}
|
||||
|
||||
i3c_bus_maintenance_unlock(&master->bus);
|
||||
}
|
||||
|
||||
static void i3c_master_reg_work_fn(struct work_struct *work)
|
||||
{
|
||||
struct i3c_master_controller *master = container_of(work, typeof(*master), reg_work);
|
||||
|
||||
i3c_bus_normaluse_lock(&master->bus);
|
||||
if (!master->shutting_down)
|
||||
i3c_master_register_new_i3c_devs(master);
|
||||
i3c_bus_normaluse_unlock(&master->bus);
|
||||
i3c_master_register_new_i3c_devs(master);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -238,6 +238,8 @@ struct i3c_dev_desc {
|
||||
* every time the I3C device is rediscovered with a different dynamic
|
||||
* address assigned
|
||||
* @bus: I3C bus this device is attached to
|
||||
* @node: unregistered device list node, only for use by
|
||||
* i3c_master_register_new_i3c_devs(), it is not protected by a lock
|
||||
*
|
||||
* I3C device object exposed to I3C device drivers. The takes care of linking
|
||||
* this object to the relevant &struct_i3c_dev_desc one.
|
||||
@@ -248,6 +250,7 @@ struct i3c_device {
|
||||
struct device dev;
|
||||
struct i3c_dev_desc *desc;
|
||||
struct i3c_bus *bus;
|
||||
struct list_head node;
|
||||
};
|
||||
|
||||
/*
|
||||
|
||||
Reference in New Issue
Block a user