public inbox for linux-i2c@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC] i2c: Fix deadlock on adapter removal
@ 2025-02-04 19:54 Heiner Kallweit
  2025-02-05  8:07 ` Herve Codina
  0 siblings, 1 reply; 5+ messages in thread
From: Heiner Kallweit @ 2025-02-04 19:54 UTC (permalink / raw)
  To: Wolfram Sang, Herve Codina; +Cc: linux-i2c@vger.kernel.org

i2c_del_adapter() can be called recursively if it has an i2c mux on
the bus. This results in a deadlock.
We use the lock to protect from parallel unregistering of clients in
case driver and adapter are removed at the same time.
The fix approach is based on the fact that the used iterators are
klist-based. So it's safe to remove list elements during the iteration,
and we don't have to acquire the core lock.
As a result we just have to prevent that i2c_unregister_device() is
executed in parallel for the same client. Use an atomic bit op for this
purpose.

Fixes: 56a50667cbcf ("i2c: Replace list-based mechanism for handling auto-detected clients")
Reported-by: Herve Codina <herve.codina@bootlin.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/i2c/i2c-core-base.c | 9 +++------
 include/linux/i2c.h         | 3 ++-
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 6eade239f..44b861eb4 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -1059,6 +1059,9 @@ void i2c_unregister_device(struct i2c_client *client)
 	if (IS_ERR_OR_NULL(client))
 		return;
 
+	if (test_and_set_bit(I2C_CLIENT_UNREGISTERING, &client->flags))
+		return;
+
 	if (client->dev.of_node) {
 		of_node_clear_flag(client->dev.of_node, OF_POPULATED);
 		of_node_put(client->dev.of_node);
@@ -1354,7 +1357,6 @@ delete_device_store(struct device *dev, struct device_attribute *attr,
 		return -EINVAL;
 	}
 
-	mutex_lock(&core_lock);
 	/* Make sure the device was added through sysfs */
 	child_dev = device_find_child(&adap->dev, &addr, __i2c_find_user_addr);
 	if (child_dev) {
@@ -1364,7 +1366,6 @@ delete_device_store(struct device *dev, struct device_attribute *attr,
 		dev_err(dev, "Can't find userspace-created device at %#x\n", addr);
 		count = -ENOENT;
 	}
-	mutex_unlock(&core_lock);
 
 	return count;
 }
@@ -1745,10 +1746,8 @@ void i2c_del_adapter(struct i2c_adapter *adap)
 	 * we can't remove the dummy devices during the first pass: they
 	 * could have been instantiated by real devices wishing to clean
 	 * them up properly, so we give them a chance to do that first. */
-	mutex_lock(&core_lock);
 	device_for_each_child(&adap->dev, NULL, __unregister_client);
 	device_for_each_child(&adap->dev, NULL, __unregister_dummy);
-	mutex_unlock(&core_lock);
 
 	/* device name is gone after device_unregister */
 	dev_dbg(&adap->dev, "adapter [%s] unregistered\n", adap->name);
@@ -2002,12 +2001,10 @@ static int __i2c_unregister_detected_client(struct device *dev, void *argp)
  */
 void i2c_del_driver(struct i2c_driver *driver)
 {
-	mutex_lock(&core_lock);
 	/* Satisfy __must_check, function can't fail */
 	if (driver_for_each_device(&driver->driver, NULL, NULL,
 				   __i2c_unregister_detected_client)) {
 	}
-	mutex_unlock(&core_lock);
 
 	driver_unregister(&driver->driver);
 	pr_debug("driver [%s] unregistered\n", driver->driver.name);
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index c31fd1dba..c9b6c0f50 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -325,7 +325,7 @@ struct i2c_driver {
  * managing the device.
  */
 struct i2c_client {
-	unsigned short flags;		/* div., see below		*/
+	unsigned long flags;		/* div., see below		*/
 #define I2C_CLIENT_PEC		0x04	/* Use Packet Error Checking */
 #define I2C_CLIENT_TEN		0x10	/* we have a ten bit chip address */
 					/* Must equal I2C_M_TEN below */
@@ -334,6 +334,7 @@ struct i2c_client {
 #define I2C_CLIENT_WAKE		0x80	/* for board_info; true iff can wake */
 #define I2C_CLIENT_AUTO		0x100	/* client was auto-detected */
 #define I2C_CLIENT_USER		0x200	/* client was userspace-created */
+#define I2C_CLIENT_UNREGISTERING 0x400	/* client is being unregistered */
 #define I2C_CLIENT_SCCB		0x9000	/* Use Omnivision SCCB protocol */
 					/* Must match I2C_M_STOP|IGNORE_NAK */
 
-- 
2.48.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-02-05 14:23 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-04 19:54 [PATCH RFC] i2c: Fix deadlock on adapter removal Heiner Kallweit
2025-02-05  8:07 ` Herve Codina
2025-02-05 13:50   ` Heiner Kallweit
2025-02-05 13:51   ` Wolfram Sang
2025-02-05 14:23     ` Herve Codina

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox