Re: [PATCH] tty: vt: make do_con_write() no-op if IRQ is disabled

From: Jiri Slaby
Date: Fri Dec 03 2021 - 00:03:56 EST


On 02. 12. 21, 19:35, Linus Torvalds wrote:
On Thu, Dec 2, 2021 at 7:41 AM Tetsuo Handa
<penguin-kernel@xxxxxxxxxxxxxxxxxxx> wrote:

Looking at the backtrace, I see

n_hdlc_send_frames+0x24b/0x490 drivers/tty/n_hdlc.c:290
tty_wakeup+0xe1/0x120 drivers/tty/tty_io.c:534
__start_tty drivers/tty/tty_io.c:806 [inline]
__start_tty+0xfb/0x130 drivers/tty/tty_io.c:799

and apparently it's that hdlc line discipline (and
n_hdlc_send_frames() in particular) that is the problem here.

I think that's where the fix should be.

Do you mean that we should change the behavior of n_hdlc_send_frames()
rather than trying to make __start_tty() schedulable again?

I wouldn't change n_hdlc_send_frames() itself. It does what it says it does.

But n_hdlc_tty_wakeup() probably shouldn't call it directly. Other tty
line disciplines don't do that kind of thing - although I only looked
at a couple. They all seem to just set bits and prepare things. Like a
wakeup function should do.

So I think n_hdlc_tty_wakeup() should perhaps only do a
"schedule_work()" or similar to get that n_hdlc_send_frames() started,
rather than doing it itself.

Concurred, this is even documented:
write_wakeup
[DRV] void ()(struct tty_struct *tty)
This function is called by the low-level tty driver to signal that line discpline should try to send more characters to the low-level driver for transmission. If the line discpline does not have any more data to send, it can just return. If the line discipline does have some data to send, please arise a tasklet or workqueue to do the real data transfer. Do not send data in this hook, it may lead to a deadlock.

thanks,
--
js
suse labs