On Fri, Oct 01, 2021 at 07:18:15AM -0300, Fabio Estevam wrote:

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 <redacted>
---
Changes since v2:
- Keep the cast when calling uart_prepare_sysrq_char() - Johan
- Improve commit log and subject - Johan

You changed more things since v2 as that was the broken version that
just dropped the lock.

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.

Johan