From patchwork Wed Oct 13 13:37:09 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= X-Patchwork-Id: 12555977 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F288CC433EF for ; Wed, 13 Oct 2021 14:23:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DE2B260E96 for ; Wed, 13 Oct 2021 14:23:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237339AbhJMOZw (ORCPT ); Wed, 13 Oct 2021 10:25:52 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48234 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237619AbhJMOZq (ORCPT ); Wed, 13 Oct 2021 10:25:46 -0400 Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [IPv6:2001:67c:670:201:290:27ff:fe1d:cc33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A674AC061746 for ; Wed, 13 Oct 2021 07:23:35 -0700 (PDT) Received: from drehscheibe.grey.stw.pengutronix.de ([2a0a:edc0:0:c01:1d::a2]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mafAM-0007eC-1P; Wed, 13 Oct 2021 16:23:34 +0200 Received: from [2a0a:edc0:0:900:1d::77] (helo=ptz.office.stw.pengutronix.de) by drehscheibe.grey.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1mafAK-0005d6-Fh; Wed, 13 Oct 2021 16:23:32 +0200 Received: from ukl by ptz.office.stw.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1mafAK-0007jS-Eq; Wed, 13 Oct 2021 16:23:32 +0200 From: =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= To: Mark Brown Cc: kernel@pengutronix.de, linux-spi@vger.kernel.org Subject: [PATCH 1/2] spi: Make spi_add_lock a per-controller lock Date: Wed, 13 Oct 2021 15:37:09 +0200 Message-Id: <20211013133710.2679703-1-u.kleine-koenig@pengutronix.de> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 X-Patch-Hashes: v=1; h=sha256; i=PNzZVIW0rJ/qRfe9wHF3FK/hHw6FosZGzO9EiHHeiH0=; m=QyKS/VkEwAN9+U5rEhQJG9lIqsLBTMARDoO6FgRw0wY=; p=0KreRDaOiTwyFokVK7Cwuq+zx8iQL3JbmYXP/jZWC0U=; g=869edb4edd311c511a0bbadc2ca6de5d9cbd0714 X-Patch-Sig: m=pgp; i=u.kleine-koenig@pengutronix.de; s=0x0D2511F322BFAB1C1580266BE2DCDD9132669BD6; b=iQEzBAABCgAdFiEEfnIqFpAYrP8+dKQLwfwUeK3K7AkFAmFm4P8ACgkQwfwUeK3K7AkA/Qf/ZQQ Smk9T6UZR5qBQ+TGhqRjGXEREvGw2p/1tKH/U+Whk4olHkoCOF2vw2V95rlAIjd/ZmM0XbW1XVI/e Wn+KVV2GZmpedlYcFwKtne5zLdttN4l+2jy7WHLgG1QiQzlrf/CiUI0FUKVJ99NnIPN9cvBBmMoGq ZU4UMKYq61ig8U8fz3/DgnIwseIo0w+xrPvZKhvwLIh4OZWm4/jgAr7oA0Bs6L0DfEneyba1Px+lN pFRs47Ri1FM0PsS4r8W13ai4QXg7ut2pVRcQNkz3YfjN71/94uv07ouav4m84p/lx2U+0zA16KtLj NgwR8ndNOeNio5WoeoGkM5TIBouhBAQ== X-SA-Exim-Connect-IP: 2a0a:edc0:0:c01:1d::a2 X-SA-Exim-Mail-From: ukl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-spi@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org The spi_add_lock that is removed with this change was held when spi_add_device() called device_add() (via __spi_add_device()). In the case where the added device is an spi-mux calling device_add() might result in calling the spi-mux's probe function which adds another controller and for that spi_add_device() might be called. This results in a dead-lock. To circumvent this deadlock replace the global spi_add_lock with a lock per controller. The biggest part of this patch was authored by Mark Brown. Signed-off-by: Uwe Kleine-König --- drivers/spi/spi.c | 17 ++++++----------- include/linux/spi/spi.h | 4 ++++ 2 files changed, 10 insertions(+), 11 deletions(-) base-commit: da21fde0fdb393c2fbe0ae0735cc826cd55fd46f diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 50591de16e0f..193e3264f7eb 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -477,12 +477,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 @@ -635,9 +629,9 @@ static int spi_add_device(struct spi_device *spi) /* Set the bus ID string */ spi_dev_set_name(spi); - 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; } @@ -656,7 +650,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); } @@ -2909,6 +2903,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) @@ -3045,7 +3040,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); @@ -3076,7 +3071,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 29e21d49aafc..5b61abe679f1 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -349,6 +349,7 @@ extern struct spi_device *spi_new_ancillary_device(struct spi_device *spi, u8 ch * @max_message_size: function that returns the max message size for * a &spi_device; may be %NULL, so the default %SIZE_MAX will be used. * @io_mutex: mutex for physical bus access + * @add_lock: mutex to avoid adding two devices on the same CS * @bus_lock_spinlock: spinlock for SPI bus locking * @bus_lock_mutex: mutex for exclusion of multiple callers * @bus_lock_flag: indicates that the SPI bus is locked for exclusive use @@ -527,6 +528,9 @@ struct spi_controller { /* I/O mutex */ struct mutex io_mutex; + /* Used to avoid adding two devices on the same CS line */ + struct mutex add_lock; + /* lock and mutex for SPI bus locking */ spinlock_t bus_lock_spinlock; struct mutex bus_lock_mutex; From patchwork Wed Oct 13 13:37:10 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= X-Patchwork-Id: 12555975 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 86C9BC433F5 for ; Wed, 13 Oct 2021 14:23:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6B559610C8 for ; Wed, 13 Oct 2021 14:23:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S237387AbhJMOZv (ORCPT ); Wed, 13 Oct 2021 10:25:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48282 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237307AbhJMOZq (ORCPT ); Wed, 13 Oct 2021 10:25:46 -0400 Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [IPv6:2001:67c:670:201:290:27ff:fe1d:cc33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A66FAC061570 for ; Wed, 13 Oct 2021 07:23:35 -0700 (PDT) Received: from drehscheibe.grey.stw.pengutronix.de ([2a0a:edc0:0:c01:1d::a2]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mafAM-0007eD-1P; Wed, 13 Oct 2021 16:23:34 +0200 Received: from [2a0a:edc0:0:900:1d::77] (helo=ptz.office.stw.pengutronix.de) by drehscheibe.grey.stw.pengutronix.de with esmtp (Exim 4.92) (envelope-from ) id 1mafAK-0005d9-Oo; Wed, 13 Oct 2021 16:23:32 +0200 Received: from ukl by ptz.office.stw.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1mafAK-0007jZ-O4; Wed, 13 Oct 2021 16:23:32 +0200 From: =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= To: Mark Brown Cc: kernel@pengutronix.de, linux-spi@vger.kernel.org Subject: [PATCH 2/2] spi-mux: Fix false-positive lockdep splats Date: Wed, 13 Oct 2021 15:37:10 +0200 Message-Id: <20211013133710.2679703-2-u.kleine-koenig@pengutronix.de> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20211013133710.2679703-1-u.kleine-koenig@pengutronix.de> References: <20211013133710.2679703-1-u.kleine-koenig@pengutronix.de> MIME-Version: 1.0 X-Patch-Hashes: v=1; h=sha256; i=GmV54BYZllLKsz7VkfghoOiGcoTv0LJ4fEcz/X0yKXw=; m=0dPnly2CxBt+MmNXnYpm2iVb63Kl480qW3iOirV/2vo=; p=xVK2ofTkqv137h/cJDorRBFEdRbDk87NQip2N6JtYNs=; g=bb34e4727a2f3720e1e64ab56c8766185ece3e56 X-Patch-Sig: m=pgp; i=u.kleine-koenig@pengutronix.de; s=0x0D2511F322BFAB1C1580266BE2DCDD9132669BD6; b=iQEzBAABCgAdFiEEfnIqFpAYrP8+dKQLwfwUeK3K7AkFAmFm4QMACgkQwfwUeK3K7AmHVwgAg+E KsqEy5riOT1rXkJ2iZG01WwvKyyqtC6pdIP/eIQZ8qe38W1UuexnDJ4qtkKsvdFwKZfDPi/REVCaO YAcK7OUSIRV0wwpqbDW/vqJgJKStWI8Y7xZa3j9CHmECQpbg7yAvO9aM82OLNI+BYh1XGEAUO4OSh 8sOG3ALMzMZf00Rz340ZjSWhWQSvwkZepIGWnLrdDeQxztO8K7Kv43Cbw4f3ynWC6S+wOUyzSISg4 +5qHRRZsXVdG+ZoeOwbCzNt91KoIDEDHwjbzwPoWzTS0wtKAmwXgrPA8actCj/DFDxxQzIL6MrO4x 4t2yBmlxIttNRT4qdM46F7a3KODsXFw== X-SA-Exim-Connect-IP: 2a0a:edc0:0:c01:1d::a2 X-SA-Exim-Mail-From: ukl@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-spi@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-spi@vger.kernel.org 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 --- 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 193e3264f7eb..72826bdab270 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -2626,6 +2626,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; @@ -2898,12 +2904,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)