| Message ID | 20240730063104.179553-1-herve.codina@bootlin.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c4d6a347ba7babdf9d90a0eb24048c266cae0532 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v1] net: wan: fsl_qmc_hdlc: Convert carrier_lock spinlock to a mutex | expand |
On Tue, Jul 30, 2024 at 08:31:04AM +0200, Herve Codina wrote: > The carrier_lock spinlock protects the carrier detection. While it is > hold, framer_get_status() is called witch in turn takes a mutex. > This is not correct and can lead to a deadlock. > > A run with PROVE_LOCKING enabled detected the issue: > [ BUG: Invalid wait context ] > ... > c204ddbc (&framer->mutex){+.+.}-{3:3}, at: framer_get_status+0x40/0x78 > other info that might help us debug this: > context-{4:4} > 2 locks held by ifconfig/146: > #0: c0926a38 (rtnl_mutex){+.+.}-{3:3}, at: devinet_ioctl+0x12c/0x664 > #1: c2006a40 (&qmc_hdlc->carrier_lock){....}-{2:2}, at: qmc_hdlc_framer_set_carrier+0x30/0x98 > > Avoid the spinlock usage and convert carrier_lock to a mutex. > > Fixes: 54762918ca85 ("net: wan: fsl_qmc_hdlc: Add framer support") > Cc: stable@vger.kernel.org > Signed-off-by: Herve Codina <herve.codina@bootlin.com> Reviewed-by: Simon Horman <horms@kernel.org>
…
> hold, framer_get_status() is called witch in turn takes a mutex.
…
which?
Regards,
Markus
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Tue, 30 Jul 2024 08:31:04 +0200 you wrote: > The carrier_lock spinlock protects the carrier detection. While it is > hold, framer_get_status() is called witch in turn takes a mutex. > This is not correct and can lead to a deadlock. > > A run with PROVE_LOCKING enabled detected the issue: > [ BUG: Invalid wait context ] > ... > c204ddbc (&framer->mutex){+.+.}-{3:3}, at: framer_get_status+0x40/0x78 > other info that might help us debug this: > context-{4:4} > 2 locks held by ifconfig/146: > #0: c0926a38 (rtnl_mutex){+.+.}-{3:3}, at: devinet_ioctl+0x12c/0x664 > #1: c2006a40 (&qmc_hdlc->carrier_lock){....}-{2:2}, at: qmc_hdlc_framer_set_carrier+0x30/0x98 > > [...] Here is the summary with links: - [net,v1] net: wan: fsl_qmc_hdlc: Convert carrier_lock spinlock to a mutex https://git.kernel.org/netdev/net/c/c4d6a347ba7b You are awesome, thank you!
diff --git a/drivers/net/wan/fsl_qmc_hdlc.c b/drivers/net/wan/fsl_qmc_hdlc.c index c5e7ca793c43..64b4bfa6fea7 100644 --- a/drivers/net/wan/fsl_qmc_hdlc.c +++ b/drivers/net/wan/fsl_qmc_hdlc.c @@ -18,6 +18,7 @@ #include <linux/hdlc.h> #include <linux/mod_devicetable.h> #include <linux/module.h> +#include <linux/mutex.h> #include <linux/platform_device.h> #include <linux/slab.h> #include <linux/spinlock.h> @@ -37,7 +38,7 @@ struct qmc_hdlc { struct qmc_chan *qmc_chan; struct net_device *netdev; struct framer *framer; - spinlock_t carrier_lock; /* Protect carrier detection */ + struct mutex carrier_lock; /* Protect carrier detection */ struct notifier_block nb; bool is_crc32; spinlock_t tx_lock; /* Protect tx descriptors */ @@ -60,7 +61,7 @@ static int qmc_hdlc_framer_set_carrier(struct qmc_hdlc *qmc_hdlc) if (!qmc_hdlc->framer) return 0; - guard(spinlock_irqsave)(&qmc_hdlc->carrier_lock); + guard(mutex)(&qmc_hdlc->carrier_lock); ret = framer_get_status(qmc_hdlc->framer, &framer_status); if (ret) { @@ -706,7 +707,7 @@ static int qmc_hdlc_probe(struct platform_device *pdev) qmc_hdlc->dev = dev; spin_lock_init(&qmc_hdlc->tx_lock); - spin_lock_init(&qmc_hdlc->carrier_lock); + mutex_init(&qmc_hdlc->carrier_lock); qmc_hdlc->qmc_chan = devm_qmc_chan_get_bychild(dev, dev->of_node); if (IS_ERR(qmc_hdlc->qmc_chan))
The carrier_lock spinlock protects the carrier detection. While it is hold, framer_get_status() is called witch in turn takes a mutex. This is not correct and can lead to a deadlock. A run with PROVE_LOCKING enabled detected the issue: [ BUG: Invalid wait context ] ... c204ddbc (&framer->mutex){+.+.}-{3:3}, at: framer_get_status+0x40/0x78 other info that might help us debug this: context-{4:4} 2 locks held by ifconfig/146: #0: c0926a38 (rtnl_mutex){+.+.}-{3:3}, at: devinet_ioctl+0x12c/0x664 #1: c2006a40 (&qmc_hdlc->carrier_lock){....}-{2:2}, at: qmc_hdlc_framer_set_carrier+0x30/0x98 Avoid the spinlock usage and convert carrier_lock to a mutex. Fixes: 54762918ca85 ("net: wan: fsl_qmc_hdlc: Add framer support") Cc: stable@vger.kernel.org Signed-off-by: Herve Codina <herve.codina@bootlin.com> --- drivers/net/wan/fsl_qmc_hdlc.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)