Thread (11 messages) 11 messages, 3 authors, 2021-10-06

Re: [PATCH v3] serial: imx: Suppress false positive sysrq lockdep warning

From: Johan Hovold <johan@kernel.org>
Date: 2021-10-06 08:10:27
Also in: lkml

On Fri, Oct 01, 2021 at 11:15:33PM -0300, Fabio Estevam wrote:
quoted hunk ↗ jump to hunk
On 01/10/2021 10:56, Johan Hovold wrote:
quoted
No, no, no.

Just replace this unlock with uart_unlock_and_check_sysrq() and do the
corresponding change in imx_uart_int(). The result is an even smaller
diff than what you're currently proposing and without any performance
penalty from dropping and reacquiring the lock.
Just to be clear, this is something that I have also tried:
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 8b121cd869e9..b652908f0bf1 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)) {
@@ -858,7 +858,7 @@ static irqreturn_t imx_uart_rxint(int irq, void 
*dev_id)

  	ret = __imx_uart_rxint(irq, dev_id);

-	spin_unlock(&sport->port.lock);
+	uart_unlock_and_check_sysrq(&sport->port);

  	return ret;
  }
@@ -991,7 +991,7 @@ static irqreturn_t imx_uart_int(int irq, void 
*dev_id)
  		ret = IRQ_HANDLED;
  	}

-	spin_unlock(&sport->port.lock);
+	uart_unlock_and_check_sysrq(&sport->port);

  	return ret;
  }
, but still get the lockdep warning in this case.
Ok, thanks for testing. The above is what I meant and it does fix the
false-positive lockdep splat which motivated
uart_unlock_and_check_sysrq() to be added in the first place.

Looking closer at the splat you reported (which you've edited quite
heavily), it becomes apparent that you are now hitting a different
locking issue. And it's not a false positive this time.

There a problem with the workqueue debugging code, which unless fixed at
the source, would prevent any console driver from queueing work while
holding a lock also taken in their write paths. And
tty_flip_buffer_push() is just one example of many.

I can easily reproduce the splat with another serial driver, and I've
also been able to trigger the actual deadlock.

I've prepared a patch that takes care of the workqueue state dumping,
which I'll send as a reply to this mail. Would you mind giving it a spin
with the imx driver as well?

Note that you may hit the other, false-positive, lockdep splat when
running with the workqueue fix, but the above diff should then address
that.

Johan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help