lkml.org 
[lkml]   [2009]   [Aug]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: WARNING at: drivers/char/tty_ldisc.c


On Sun, 2 Aug 2009, Sergey Senozhatsky wrote:
>
> Looks like we have one more bug in tty code:
> 'shutdown -r now' in 'single' user mode gives following trace:
>
> WARNING at: drivers/char/tty_ldisc.c:209 tty_ldisc_put+0x95/0xa0
> ---
> warn_slowpath_common+0x78/0xa0
> warn_slowpath_null+0x21/0x40
> tty_ldisc_put+0x95/0xa0
> tty_ldisc_hangup+0xfc/0x1f0
> do_tty_hangup+0x131/0x380
> disassociate_ctty+0x50/0x210
> do_exit+0x6b8/0x700
> do_group_exit+0x45/0xb0
> get_signal_to_deliver+0x226/0x410
> do_notify_resume+0xc1+0xa90
> work_notifysig+0x13/0x19
>
> //There is no trace in syslog, so given one is 'copy-paste' from the paper.

We have that 'refcount' counter for the ldisc, but we don't actually use
it for memory management like we should (ie "free the ldisc when count
goes to zero"), we just decrement it and free the thing.

And it's quite possible that another CPU is doing some tty read thing or
other that does

tty_ldisc_try(..) // increments ld->refcount
...
tty_ldisc_deref(..) // decrements it.
at the same time.

The ldisc refcounts are simply done wrong. They are more debugging aids
(for the case where no races occur), than actual memory management
refcounts.

Now, when you do refcounts wrong like that, the "fix" for it is to wait
for the count to go to zero before releasing things (the problem with that
fix is that it tends to be complicated and prone to deadlocks etc).

And the tty layer really does try to handle this with that

tty_ldisc_halt() - clear TTY_LDISC so that no new refs are taken
tty_ldisc_wait_idle() - wait for all old refs to go away

which it did just before the call to 'tty_ldisc_reinit()' (which is
apparently inlined) that will then call 'tty_ldisc_put()', which is what
complains.

So we've actually just waited for the count to go down to zero, and now
it's not zero again. Which _should_ be impossible since the TTY_LDISC bit
is clear, of course.

But there are things that enable TTY_LDISC, like a 'tty_set_ldisc()' done
on another CPU. Which does try to protect things with 'tty->ldisc_mutex',
but in the case of pty's in particular, there are _two_ tty's involved,
and it does things like

if (o_tty)
tty_ldisc_enable(o_tty);
to re-enable the "other" tty, without holding the lock for that tty.

And I don't think we can just take the lock either, because that's an ABBA
deadlock when you do it trivially.

I dunno. It might not be the above, there might be something else going
on, I'll have to think about/look at it a bit more.

Linus


\
 
 \ /
  Last update: 2009-10-18 23:28    [from the cache]
©2003-2011 Jasper Spaans