linux-serial.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Fabio Estevam <festevam@denx.de>
To: gregkh@linuxfoundation.org
Cc: michael@walle.cc, linux-serial@vger.kernel.org, johan@kernel.org,
	marex@denx.de, u.kleine-koenig@pengutronix.de,
	Fabio Estevam <festevam@denx.de>
Subject: [PATCH v3]  serial: imx: Suppress false positive sysrq lockdep warning
Date: Fri,  1 Oct 2021 07:18:15 -0300	[thread overview]
Message-ID: <20211001101815.729648-1-festevam@denx.de> (raw)

The following sysrq command causes the following lockdep warning:

 # echo t > /proc/sysrq-trigger
 ....
[   20.325246] ======================================================
[   20.325252] WARNING: possible circular locking dependency detected
[   20.325260] 5.15.0-rc2-next-20210924-00004-gd2d6e664f29f-dirty #163
Not tainted
[   20.325273] ------------------------------------------------------
[   20.325279] sh/236 is trying to acquire lock:
[   20.325293] c1618614 (console_owner){-...}-{0:0}, at:
console_unlock+0x180/0x5bc
[   20.325361]
[   20.325361] but task is already holding lock:
[   20.325368] eefccc90 (&pool->lock){-.-.}-{2:2}, at:
show_workqueue_state+0x104/0x3c8
[   20.325432]
[   20.325432] which lock already depends on the new lock.

...

[   20.325657] -> #2 (&pool->lock/1){-.-.}-{2:2}:
[   20.325690]        __queue_work+0x114/0x810
[   20.325710]        queue_work_on+0x54/0x94
[   20.325727]        __imx_uart_rxint.constprop.0+0x1b4/0x2e0
[   20.325760]        imx_uart_int+0x270/0x310

This problem happens because uart_handle_sysrq_char() is called
with the lock held.

Fix this by using the same approach done in commit 5697df7322fe ("serial:
fsl_lpuart: split sysrq handling"), which calls uart_prepare_sysrq_char()
and uart_unlock_and_check_sysrq() instead.

Its commit log says:

"Instead of uart_handle_sysrq_char() use uart_prepare_sysrq_char() and
uart_unlock_and_check_sysrq(). This will call handle_sysrq() without
holding the port lock, which in turn let us drop the spin_trylock hack."

Do the same here to suppress the false positive lockdep warning.

As __imx_uart_rxint() drops the lock now, remove the spin_unlock()
inside imx_uart_rxint(), which is only used on i.MX1.

Tested on a i.MX7D board via 'echo t > /proc/sysrq-trigger' in the
command line and also by passing the "<break> + t" keys in
the serial console.

Signed-off-by: Fabio Estevam <festevam@denx.de>
---
Changes since v2:
- Keep the cast when calling uart_prepare_sysrq_char() - Johan
- Improve commit log and subject - Johan

 drivers/tty/serial/imx.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 8b121cd869e9..a0135718c588 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -803,7 +803,7 @@ static irqreturn_t __imx_uart_rxint(int irq, void *dev_id)
 				continue;
 		}
 
-		if (uart_handle_sysrq_char(&sport->port, (unsigned char)rx))
+		if (uart_prepare_sysrq_char(&sport->port, (unsigned char)rx))
 			continue;
 
 		if (unlikely(rx & URXD_ERR)) {
@@ -844,6 +844,7 @@ static irqreturn_t __imx_uart_rxint(int irq, void *dev_id)
 	}
 
 out:
+	uart_unlock_and_check_sysrq(&sport->port);
 	tty_flip_buffer_push(port);
 
 	return IRQ_HANDLED;
@@ -852,15 +853,10 @@ static irqreturn_t __imx_uart_rxint(int irq, void *dev_id)
 static irqreturn_t imx_uart_rxint(int irq, void *dev_id)
 {
 	struct imx_port *sport = dev_id;
-	irqreturn_t ret;
 
 	spin_lock(&sport->port.lock);
 
-	ret = __imx_uart_rxint(irq, dev_id);
-
-	spin_unlock(&sport->port.lock);
-
-	return ret;
+	return __imx_uart_rxint(irq, dev_id);
 }
 
 static void imx_uart_clear_rx_errors(struct imx_port *sport);
@@ -959,6 +955,7 @@ static irqreturn_t imx_uart_int(int irq, void *dev_id)
 		imx_uart_writel(sport, USR1_AGTIM, USR1);
 
 		__imx_uart_rxint(irq, dev_id);
+		spin_lock(&sport->port.lock);
 		ret = IRQ_HANDLED;
 	}
 
@@ -1977,9 +1974,7 @@ imx_uart_console_write(struct console *co, const char *s, unsigned int count)
 	unsigned int ucr1;
 	int locked = 1;
 
-	if (sport->port.sysrq)
-		locked = 0;
-	else if (oops_in_progress)
+	if (oops_in_progress)
 		locked = spin_trylock_irqsave(&sport->port.lock, flags);
 	else
 		spin_lock_irqsave(&sport->port.lock, flags);
-- 
2.25.1


             reply	other threads:[~2021-10-01 10:18 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-01 10:18 Fabio Estevam [this message]
2021-10-01 13:56 ` [PATCH v3] serial: imx: Suppress false positive sysrq lockdep warning Johan Hovold
2021-10-01 14:48   ` Fabio Estevam
2021-10-02  2:15   ` Fabio Estevam
2021-10-06  8:10     ` Johan Hovold
2021-10-06  8:11       ` [PATCH] workqueue: fix state-dump console deadlock Johan Hovold
2021-10-06  9:19         ` Petr Mladek
2021-10-06 10:07           ` Johan Hovold
2021-10-06 10:49         ` Fabio Estevam
2021-10-06 10:52       ` [PATCH v3] serial: imx: Suppress false positive sysrq lockdep warning Fabio Estevam
2021-10-06 12:02         ` Johan Hovold

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211001101815.729648-1-festevam@denx.de \
    --to=festevam@denx.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=johan@kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=marex@denx.de \
    --cc=michael@walle.cc \
    --cc=u.kleine-koenig@pengutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).