public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
To: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Sergey Senozhatsky <sergey.senozhatsky@mail.by>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Greg KH <greg@kroah.com>
Subject: Re: WARNING at: drivers/char/tty_ldisc.c
Date: Tue, 04 Aug 2009 01:26:29 +0900	[thread overview]
Message-ID: <87vdl4kgne.fsf@devron.myhome.or.jp> (raw)
In-Reply-To: <20090803103746.264c3acf@lxorguk.ukuu.org.uk> (Alan Cox's message of "Mon, 3 Aug 2009 10:37:46 +0100")

Alan Cox <alan@lxorguk.ukuu.org.uk> writes:

>> So I do get the feeling that it may be just old code that simply nobody 
>> has dared remove - it may have made more sense back when then it does 
>> now, and has just been carried around.
>
> Alas not: eg
>
> ->hangup
> uart_hangup
> uart_flush_buffer
> uart_shutdown
> 	drops carrier and dtr
> 	shuts down the port
> 	frees the IRQ handler
> 	kills off the transmit buffer & tasklet
>
> whether the resulting mess happens to still work with printk rather
> depends upon what ->ops->shutdown does. Even on x86 that will drop a
> remote console if the carrier is being used so you won't see printk
> messages after that point as you got a modem hangup.
>
> A re-open as a device isn't a problem (we cleared ASYNCB_INITIALIZED) -
> its the printk side that kills you, and someone is going to have to read
> all the embedded device consoles as they are both the most likely to
> break and most used 8(

I see. I guess it is explaining the following part

	if (cons_filp) {
		if (tty->ops->close)
			for (n = 0; n < closecount; n++)
				tty->ops->close(tty, cons_filp);
	} else if (tty->ops->hangup)
		(tty->ops->hangup)(tty);

the above seem to avoid serial hangup... Luckily, I found the patch of
it. But, even before the patch, it seems we didn't

	filp->f_op = &hung_up_tty_fops;

for console stuff. But, I'm still not found why we avoid to the above...

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

commit a972c49404c18ed15db2c1cce5f414293f73c8d9
Author: Linus Torvalds <torvalds@linuxfoundation.org>
Date:   Fri Nov 23 15:16:55 2007 -0500

    Import 2.1.125pre2

diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index e8b116f..2a2f2de 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -390,7 +390,9 @@ void do_tty_hangup(void *data)
 {
 	struct tty_struct *tty = (struct tty_struct *) data;
 	struct file * filp;
+	struct file * cons_filp = NULL;
 	struct task_struct *p;
+	int    closecount = 0, n;
 
 	if (!tty)
 		return;
@@ -407,10 +409,13 @@ void do_tty_hangup(void *data)
 		if (!filp->f_dentry->d_inode)
 			continue;
 		if (filp->f_dentry->d_inode->i_rdev == CONSOLE_DEV ||
-		    filp->f_dentry->d_inode->i_rdev == SYSCONS_DEV)
+		    filp->f_dentry->d_inode->i_rdev == SYSCONS_DEV) {
+			cons_filp = filp;
 			continue;
+		}
 		if (filp->f_op != &tty_fops)
 			continue;
+		closecount++;
 		tty_fasync(-1, filp, 0);
 		filp->f_op = &hung_up_tty_fops;
 	}
@@ -470,7 +475,17 @@ void do_tty_hangup(void *data)
 	tty->session = 0;
 	tty->pgrp = -1;
 	tty->ctrl_status = 0;
-	if (tty->driver.hangup)
+	/*
+	 *	If one of the devices matches a console pointer, we
+	 *	cannot just call hangup() because that will cause
+	 *	tty->count and state->count to go out of sync.
+	 *	So we just call close() the right number of times.
+	 */
+	if (cons_filp) {
+		if (tty->driver.close)
+			for (n = 0; n < closecount; n++)
+				tty->driver.close(tty, cons_filp);
+	} else if (tty->driver.hangup)
 		(tty->driver.hangup)(tty);
 	unlock_kernel();
 }
@@ -1243,6 +1258,7 @@ retry_open:
 		if (!c)
                         return -ENODEV;
                 device = c->device(c);
+		filp->f_flags |= O_NONBLOCK; /* Don't let /dev/console block */
 		noctty = 1;
 	}
 #ifdef CONFIG_UNIX98_PTYS


  reply	other threads:[~2009-08-03 16:26 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-02 12:01 WARNING at: drivers/char/tty_ldisc.c Sergey Senozhatsky
2009-08-02 16:05 ` Greg KH
2009-08-02 17:01   ` Sergey Senozhatsky
2009-08-02 17:07   ` Sergey Senozhatsky
2009-08-02 17:16 ` Linus Torvalds
2009-08-02 19:05   ` Sergey Senozhatsky
2009-08-02 20:20     ` Linus Torvalds
2009-08-02 21:17       ` OGAWA Hirofumi
2009-08-02 21:33         ` Linus Torvalds
2009-08-02 22:46           ` Sergey Senozhatsky
2009-08-02 22:48           ` Alan Cox
2009-08-03  0:40             ` Linus Torvalds
2009-08-03  1:44               ` Linus Torvalds
2009-08-03  9:37               ` Alan Cox
2009-08-03 16:26                 ` OGAWA Hirofumi [this message]
2009-08-03 16:59                   ` Alan Cox
2009-08-03 17:55               ` [PATCH 0/2] proper tty-ldisc refcounting (was Re: WARNING at: drivers/char/tty_ldisc.c) Linus Torvalds
2009-08-03 17:58                 ` [PATCH 1/2] tty-ldisc: make refcount be atomic_t 'users' count Linus Torvalds
2009-08-03 18:11                   ` [PATCH 2/2] tty-ldisc: turn ldisc user count into a proper refcount Linus Torvalds
2009-08-03 18:39                     ` Alan Cox
2009-08-03 20:00                     ` OGAWA Hirofumi
2009-08-03 18:18                 ` [PATCH 0/2] proper tty-ldisc refcounting (was Re: WARNING at: drivers/char/tty_ldisc.c) Greg KH
2009-08-03 18:53                   ` Linus Torvalds
2009-08-03 22:16                   ` Sergey Senozhatsky
2009-08-03 22:25                     ` Linus Torvalds
2009-08-03 22:58                       ` [PATCH 3/2] tty-ldisc: be more careful in 'put_ldisc' locking Linus Torvalds
2009-08-03 23:00                         ` [PATCH 4/2] tty-ldisc: make /proc/tty/ldiscs use ldisc_ops instead of ldiscs Linus Torvalds
2009-08-03 23:01                           ` [PATCH 5/2] tty-ldisc: get rid of tty_ldisc_try_get() helper function Linus Torvalds
2009-08-04  0:30                   ` proper tty-ldisc refcounting (was Re: WARNING at: drivers/char/tty_ldisc.c) Sergey Senozhatsky
2009-08-04  0:56                     ` Linus Torvalds
2009-08-04  3:53                       ` Greg KH
2009-08-04  4:08                         ` Greg KH
2009-08-04  6:19                           ` Linus Torvalds
2009-08-04  7:23                             ` Greg KH
2009-08-04  9:12                               ` Sergey Senozhatsky
2009-08-04 14:53                                 ` Greg KH
2009-08-04 15:40                               ` Linus Torvalds
2009-08-04 16:00                                 ` Greg KH
2009-08-02 22:15       ` WARNING at: drivers/char/tty_ldisc.c Sergey Senozhatsky

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=87vdl4kgne.fsf@devron.myhome.or.jp \
    --to=hirofumi@mail.parknet.co.jp \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=greg@kroah.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sergey.senozhatsky@mail.by \
    --cc=torvalds@linux-foundation.org \
    /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