* [PATCH AUTOSEL 5.14 20/26] spi: Fix deadlock when adding SPI controllers on SPI buses
[not found] <20211021002023.1128949-1-sashal@kernel.org>
@ 2021-10-21 0:20 ` Sasha Levin
2021-10-21 0:20 ` [PATCH AUTOSEL 5.14 21/26] spi-mux: Fix false-positive lockdep splats Sasha Levin
1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2021-10-21 0:20 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Mark Brown, Uwe Kleine-König, Sasha Levin, linux-spi
From: Mark Brown <broonie@kernel.org>
[ Upstream commit 6098475d4cb48d821bdf453c61118c56e26294f0 ]
Currently we have a global spi_add_lock which we take when adding new
devices so that we can check that we're not trying to reuse a chip
select that's already controlled. This means that if the SPI device is
itself a SPI controller and triggers the instantiation of further SPI
devices we trigger a deadlock as we try to register and instantiate
those devices while in the process of doing so for the parent controller
and hence already holding the global spi_add_lock. Since we only care
about concurrency within a single SPI bus move the lock to be per
controller, avoiding the deadlock.
This can be easily triggered in the case of spi-mux.
Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/spi/spi.c | 17 ++++++-----------
include/linux/spi/spi.h | 3 +++
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index f95f7666cb5b..2c342bded058 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -480,12 +480,6 @@ static LIST_HEAD(spi_controller_list);
*/
static DEFINE_MUTEX(board_lock);
-/*
- * Prevents addition of devices with same chip select and
- * addition of devices below an unregistering controller.
- */
-static DEFINE_MUTEX(spi_add_lock);
-
/**
* spi_alloc_device - Allocate a new SPI device
* @ctlr: Controller to which device is connected
@@ -638,9 +632,9 @@ int spi_add_device(struct spi_device *spi)
* chipselect **BEFORE** we call setup(), else we'll trash
* its configuration. Lock against concurrent add() calls.
*/
- mutex_lock(&spi_add_lock);
+ mutex_lock(&ctlr->add_lock);
status = __spi_add_device(spi);
- mutex_unlock(&spi_add_lock);
+ mutex_unlock(&ctlr->add_lock);
return status;
}
EXPORT_SYMBOL_GPL(spi_add_device);
@@ -660,7 +654,7 @@ static int spi_add_device_locked(struct spi_device *spi)
/* Set the bus ID string */
spi_dev_set_name(spi);
- WARN_ON(!mutex_is_locked(&spi_add_lock));
+ WARN_ON(!mutex_is_locked(&ctlr->add_lock));
return __spi_add_device(spi);
}
@@ -2832,6 +2826,7 @@ int spi_register_controller(struct spi_controller *ctlr)
spin_lock_init(&ctlr->bus_lock_spinlock);
mutex_init(&ctlr->bus_lock_mutex);
mutex_init(&ctlr->io_mutex);
+ mutex_init(&ctlr->add_lock);
ctlr->bus_lock_flag = 0;
init_completion(&ctlr->xfer_completion);
if (!ctlr->max_dma_len)
@@ -2968,7 +2963,7 @@ void spi_unregister_controller(struct spi_controller *ctlr)
/* Prevent addition of new devices, unregister existing ones */
if (IS_ENABLED(CONFIG_SPI_DYNAMIC))
- mutex_lock(&spi_add_lock);
+ mutex_lock(&ctlr->add_lock);
device_for_each_child(&ctlr->dev, NULL, __unregister);
@@ -2999,7 +2994,7 @@ void spi_unregister_controller(struct spi_controller *ctlr)
mutex_unlock(&board_lock);
if (IS_ENABLED(CONFIG_SPI_DYNAMIC))
- mutex_unlock(&spi_add_lock);
+ mutex_unlock(&ctlr->add_lock);
}
EXPORT_SYMBOL_GPL(spi_unregister_controller);
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 97b8d12b5f2b..5d80c6fd2a22 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -527,6 +527,9 @@ struct spi_controller {
/* I/O mutex */
struct mutex io_mutex;
+ /* Used to avoid adding the same CS twice */
+ struct mutex add_lock;
+
/* lock and mutex for SPI bus locking */
spinlock_t bus_lock_spinlock;
struct mutex bus_lock_mutex;
--
2.33.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [PATCH AUTOSEL 5.14 21/26] spi-mux: Fix false-positive lockdep splats
[not found] <20211021002023.1128949-1-sashal@kernel.org>
2021-10-21 0:20 ` [PATCH AUTOSEL 5.14 20/26] spi: Fix deadlock when adding SPI controllers on SPI buses Sasha Levin
@ 2021-10-21 0:20 ` Sasha Levin
1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2021-10-21 0:20 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Uwe Kleine-König, Mark Brown, Sasha Levin, linux-spi
From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
[ Upstream commit 16a8e2fbb2d49111004efc1c7342e083eafabeb0 ]
io_mutex is taken by spi_setup() and spi-mux's .setup() callback calls
spi_setup() which results in a nested lock of io_mutex.
add_lock is taken by spi_add_device(). The device_add() call in there
can result in calling spi-mux's .probe() callback which registers its
own spi controller which in turn results in spi_add_device() being
called again.
To fix this initialize the controller's locks already in
spi_alloc_controller() to give spi_mux_probe() a chance to set the
lockdep subclass.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Link: https://lore.kernel.org/r/20211013133710.2679703-2-u.kleine-koenig@pengutronix.de
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/spi/spi-mux.c | 7 +++++++
drivers/spi/spi.c | 12 ++++++------
2 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/spi/spi-mux.c b/drivers/spi/spi-mux.c
index 9708b7827ff7..f5d32ec4634e 100644
--- a/drivers/spi/spi-mux.c
+++ b/drivers/spi/spi-mux.c
@@ -137,6 +137,13 @@ static int spi_mux_probe(struct spi_device *spi)
priv = spi_controller_get_devdata(ctlr);
priv->spi = spi;
+ /*
+ * Increase lockdep class as these lock are taken while the parent bus
+ * already holds their instance's lock.
+ */
+ lockdep_set_subclass(&ctlr->io_mutex, 1);
+ lockdep_set_subclass(&ctlr->add_lock, 1);
+
priv->mux = devm_mux_control_get(&spi->dev, NULL);
if (IS_ERR(priv->mux)) {
ret = dev_err_probe(&spi->dev, PTR_ERR(priv->mux),
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 2c342bded058..3093e0041158 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -2549,6 +2549,12 @@ struct spi_controller *__spi_alloc_controller(struct device *dev,
return NULL;
device_initialize(&ctlr->dev);
+ INIT_LIST_HEAD(&ctlr->queue);
+ spin_lock_init(&ctlr->queue_lock);
+ spin_lock_init(&ctlr->bus_lock_spinlock);
+ mutex_init(&ctlr->bus_lock_mutex);
+ mutex_init(&ctlr->io_mutex);
+ mutex_init(&ctlr->add_lock);
ctlr->bus_num = -1;
ctlr->num_chipselect = 1;
ctlr->slave = slave;
@@ -2821,12 +2827,6 @@ int spi_register_controller(struct spi_controller *ctlr)
return id;
ctlr->bus_num = id;
}
- INIT_LIST_HEAD(&ctlr->queue);
- spin_lock_init(&ctlr->queue_lock);
- spin_lock_init(&ctlr->bus_lock_spinlock);
- mutex_init(&ctlr->bus_lock_mutex);
- mutex_init(&ctlr->io_mutex);
- mutex_init(&ctlr->add_lock);
ctlr->bus_lock_flag = 0;
init_completion(&ctlr->xfer_completion);
if (!ctlr->max_dma_len)
--
2.33.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-10-21 0:21 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20211021002023.1128949-1-sashal@kernel.org>
2021-10-21 0:20 ` [PATCH AUTOSEL 5.14 20/26] spi: Fix deadlock when adding SPI controllers on SPI buses Sasha Levin
2021-10-21 0:20 ` [PATCH AUTOSEL 5.14 21/26] spi-mux: Fix false-positive lockdep splats Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).