[PATCH 1/1] Char: tty_io, fix closecount counting

[PATCH 1/1] Char: tty_io, fix closecount counting

[Jiri Slaby]

Alan, please N/ACK this.

--

filp->f_op->write never equals to tty_write for the console device, so
closecount++ is never reached and we don't close console device so many
times we open it before. (The closecount is used only for /dev/console.)

This is probably a fix for an issue first reported in 2.6.18.1:
http://lkml.org/lkml/2006/10/20/301
and
http://marc.info/?l=linux-mips&m=118797256328587

Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
---
 drivers/char/tty_io.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
index c9e6e24..8c7b6ed 100644
--- a/drivers/char/tty_io.c
+++ b/drivers/char/tty_io.c
@@ -1434,9 +1434,9 @@ static void do_tty_hangup(struct work_struct *work)
 	list_for_each_entry(filp, &tty->tty_files, f_u.fu_list) {
 		if (filp->f_op->write == redirected_tty_write)
 			cons_filp = filp;
+		closecount++;
 		if (filp->f_op->write != tty_write)
 			continue;
-		closecount++;
 		tty_fasync(-1, filp, 0);	/* can't block */
 		filp->f_op = &hung_up_tty_fops;
 	}
-- 
1.5.4.5

Re: [PATCH 1/1] Char: tty_io, fix closecount counting

[Jiri Slaby]

Jiri Slaby napsal(a):
> filp->f_op->write never equals to tty_write for the console device, so
> closecount++ is never reached and we don't close console device so many
> times we open it before. (The closecount is used only for /dev/console.)
[...]
>  drivers/char/tty_io.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c
> index c9e6e24..8c7b6ed 100644
> --- a/drivers/char/tty_io.c
> +++ b/drivers/char/tty_io.c
> @@ -1434,9 +1434,9 @@ static void do_tty_hangup(struct work_struct *work)
>  	list_for_each_entry(filp, &tty->tty_files, f_u.fu_list) {
>  		if (filp->f_op->write == redirected_tty_write)
>  			cons_filp = filp;
> +		closecount++;
>  		if (filp->f_op->write != tty_write)
>  			continue;
> -		closecount++;

Hm, no, this is wrong.

Re: [PATCH 1/1] Char: tty_io, fix closecount counting

[Jiri Slaby]

Jiri Slaby napsal(a):
> This is probably a fix for an issue first reported in 2.6.18.1:
> http://lkml.org/lkml/2006/10/20/301
> and
> http://marc.info/?l=linux-mips&m=118797256328587

Alan, I know, you already stated some time ago, that you don't have a clue 
what could have caused this. Haven't you found anything since that time as I 
think the hangup path is the only possible cause of this problem?

I've found an other one from 2.6.17-rc4:
http://readlist.com/lists/vger.kernel.org/linux-kernel/44/220050.html
and from 2.6.22+suse_stuff:
http://lists.opensuse.org/opensuse-bugs/2008-04/msg05628.html

It is ever vcs1, which is /dev/console and it is the only treated separately 
(otherwise I think we would have more (and distinct) reports like this). 
When the console is HUPped? And what should happen with openers? The 
2.6.22+suse happened when sulogin /dev/console was invoked. Any ideas how to 
track this down?

> --- a/drivers/char/tty_io.c
> +++ b/drivers/char/tty_io.c
> @@ -1434,9 +1434,9 @@ static void do_tty_hangup(struct work_struct *work)
>  	list_for_each_entry(filp, &tty->tty_files, f_u.fu_list) {
>  		if (filp->f_op->write == redirected_tty_write)
>  			cons_filp = filp;
> +		closecount++;
>  		if (filp->f_op->write != tty_write)
>  			continue;
> -		closecount++;
>  		tty_fasync(-1, filp, 0);	/* can't block */
>  		filp->f_op = &hung_up_tty_fops;
>  	}

Re: [PATCH 1/1] Char: tty_io, fix closecount counting

[Jiri Slaby]

Jiri Slaby napsal(a):
> Jiri Slaby napsal(a):
>> This is probably a fix for an issue first reported in 2.6.18.1:
>> http://lkml.org/lkml/2006/10/20/301
>> and
>> http://marc.info/?l=linux-mips&m=118797256328587
> 
> Alan, I know, you already stated some time ago, that you don't have a 
> clue what could have caused this. Haven't you found anything since that 
> time as I think the hangup path is the only possible cause of this problem?
> 
> I've found an other one from 2.6.17-rc4:
> http://readlist.com/lists/vger.kernel.org/linux-kernel/44/220050.html
> and from 2.6.22+suse_stuff:
> http://lists.opensuse.org/opensuse-bugs/2008-04/msg05628.html
> 
> It is ever vcs1, which is /dev/console and it is the only treated 
> separately (otherwise I think we would have more (and distinct) reports 
> like this). When the console is HUPped? And what should happen with 
> openers? The 2.6.22+suse happened when sulogin /dev/console was invoked. 
> Any ideas how to track this down?
> 
>> --- a/drivers/char/tty_io.c
>> +++ b/drivers/char/tty_io.c
>> @@ -1434,9 +1434,9 @@ static void do_tty_hangup(struct work_struct *work)
>>      list_for_each_entry(filp, &tty->tty_files, f_u.fu_list) {
>>          if (filp->f_op->write == redirected_tty_write)
>>              cons_filp = filp;
>> +        closecount++;
>>          if (filp->f_op->write != tty_write)
>>              continue;
>> -        closecount++;
>>          tty_fasync(-1, filp, 0);    /* can't block */
>>          filp->f_op = &hung_up_tty_fops;
>>      }

(Ah, btw, I've uttered this change as wrong.)

Re: [PATCH 1/1] Char: tty_io, fix closecount counting

[Alan Cox]

> It is ever vcs1, which is /dev/console and it is the only treated separately 
> (otherwise I think we would have more (and distinct) reports like this). 
> When the console is HUPped? And what should happen with openers? The 
> 2.6.22+suse happened when sulogin /dev/console was invoked. Any ideas how to 
> track this down?

Its a complete and total mess is the obvious answer. In fact its far
worse than it first looks and I don't believe it can be fixed without
reworking the whole printk/console stuff.

I spent several days this week mapping out how the tty open/close/hangup
logic works and to be honest what I've learned is

- It doesn't
- It can't be easily fixed to
- There are numerous races
- The whole "tty/console" abstraction for printk type stuff is totally
broken by design and replicated in several places for good measure.

At this point I have to say I can't even see a way to get from the
existing tty open/close/hangup logic to a working one which does not
involve changes to every driver with a 'flag day' type switch between the
two implementations.

For the existing codebase I think the best we can do in this area is label
it "condemned" and not touch it, because any change will be unpredictable
in its effect and not a real fix.

I'd love to be proved wrong. Doing a grand reimplementation of large
chunks of the tty layer isn't what I want to do, I just see no
alternative at this point.

Alan

Re: [PATCH 1/1] Char: tty_io, fix closecount counting

[Jiri Slaby]

On 05/31/2008 03:43 PM, Alan Cox wrote:
>> It is ever vcs1, which is /dev/console and it is the only treated separately 
>> (otherwise I think we would have more (and distinct) reports like this). 
>> When the console is HUPped? And what should happen with openers? The 
>> 2.6.22+suse happened when sulogin /dev/console was invoked. Any ideas how to 
>> track this down?
> 
> Its a complete and total mess is the obvious answer. In fact its far
> worse than it first looks and I don't believe it can be fixed without
> reworking the whole printk/console stuff.

Thanks a heap for the answer.