From: Paul Fulghum The following patch removes unnecessary disabling of interrupts when processing hangup for tty devices. This was introduced way back in August 1998 with patch 2.1.115 when the original hangup processing was changed from cli/sti to lock_kernel()/unlock_kernel(). Up to now, this did not cause any actual problems. In 2.6.X this causes a warning if any of the called functions (flush_buffer/write_wakeup) call spin_xxx_bh() functions. Warning is in spin_unlock_bh (kernel/softirq.c:122) An example is drivers/net/ppp_synctty.c write_wakeup. This has been reported several times by different people. The flush_buffer/write_wakeup calls are also called when processing ioctl for tty devices, without disabling interrupts using only lock_kernel()/unlock_kernel(). tty->ldisc.flush_buffer() tty_ioctl.c ioctl(TCSETSF) -> set_termios() ioctl(TCSETAF) -> set_termios() ioctl(TCFLSH) tty->driver.flush_buffer() tty_ioctl.c ioctl(TCFLSH) tty->ldisc.write_wakeup() tty_ioctl.c ioctl(TCOON) -> start_tty() ioctl(TCIOFF) -> send_prio_char() -> start_tty() ioctl(TCION) -> send_prio_char() -> start_tty() So removal of the interrupt disable/enable from around these calls in tty_io.c do_tty_hangup(), implements locking the same as the ioctl calls. I have tested this patch on an SMP machine with 2.6.6. I welcome comments and testing by others, particularly those who have seen the softirq.c message when doing ppp (synchronous PPP, PPPoATM etc) that use the ppp_synctty.c Signed-off-by: Andrew Morton --- 25-akpm/drivers/char/tty_io.c | 22 +++++++--------------- 1 files changed, 7 insertions(+), 15 deletions(-) diff -puN drivers/char/tty_io.c~tty_io-hangup-locking drivers/char/tty_io.c --- 25/drivers/char/tty_io.c~tty_io-hangup-locking Fri May 28 15:59:50 2004 +++ 25-akpm/drivers/char/tty_io.c Fri May 28 15:59:50 2004 @@ -445,21 +445,13 @@ void do_tty_hangup(void *data) } file_list_unlock(); - /* FIXME! What are the locking issues here? This may me overdoing things.. - * this question is especially important now that we've removed the irqlock. */ - { - unsigned long flags; - - local_irq_save(flags); // FIXME: is this safe? - if (tty->ldisc.flush_buffer) - tty->ldisc.flush_buffer(tty); - if (tty->driver->flush_buffer) - tty->driver->flush_buffer(tty); - if ((test_bit(TTY_DO_WRITE_WAKEUP, &tty->flags)) && - tty->ldisc.write_wakeup) - (tty->ldisc.write_wakeup)(tty); - local_irq_restore(flags); // FIXME: is this safe? - } + if (tty->ldisc.flush_buffer) + tty->ldisc.flush_buffer(tty); + if (tty->driver->flush_buffer) + tty->driver->flush_buffer(tty); + if ((test_bit(TTY_DO_WRITE_WAKEUP, &tty->flags)) && + tty->ldisc.write_wakeup) + (tty->ldisc.write_wakeup)(tty); wake_up_interruptible(&tty->write_wait); wake_up_interruptible(&tty->read_wait); _