Messages in this thread | | | Date | Thu, 25 Nov 2010 17:51:11 +0100 | From | Jiri Slaby <> | Subject | Re: mmotm 2010-11-23 - WARNING: at drivers/tty/tty_io.c:1331 |
| |
On 11/25/2010 05:44 PM, Jiri Slaby wrote: > On 11/25/2010 04:14 PM, Kyle McMartin wrote: >> On Tue, Nov 23, 2010 at 11:55:39PM -0500, Valdis.Kletnieks@vt.edu wrote: >>> On Tue, 23 Nov 2010 16:13:06 PST, akpm@linux-foundation.org said: >>>> The mm-of-the-moment snapshot 2010-11-23-16-12 has been uploaded to >>>> >>>> http://userweb.kernel.org/~akpm/mmotm/ >>> >>> Seen during boot: >>> >>> [ 23.015448] Modules linked in: >>> [ 23.015453] Pid: 1207, comm: plymouthd Not tainted 2.6.37-rc3-mmotm1123 #3 >>> [ 23.015455] Call Trace: >> >> I've been trying to figure this one out for a while, without much luck. >> (Users are seeing it in 2.6.36 as well.) >> >> I *think* (I added a rawhide debugging patch to print the tty->name) >> that plymouth is always opening tty7 to cause this. My guess is the BKL >> removal has exposed some kind of race, but it's not obvious to me (and >> there's many other bugs to sort through too. :( >> >> CC-ing Jiri since he seems to be the poor guy who's been poking this >> recently (there's a good few threads about this (though the others look >> like an ldisc attach race...)) I wouldn't think that's the case here >> since N_TTY is the default... > > Ok, tty_reopen is called without TTY_LDISC set. For further > considerations, note tty_lock is held in tty_open. TTY_LDISC is cleared in: > > 1) __tty_hangup from tty_ldisc_hangup to tty_ldisc_enable. During this > section tty_lock is held. > > 2) tty_release via tty_ldisc_release till the end of tty existence. If > tty->count <= 1, tty_lock is taken, TTY_CLOSING bit set and then > tty_ldisc_release called. tty_reopen checks TTY_CLOSING before checking > TTY_LDISC. > > 3) tty_set_ldisc from tty_ldisc_halt to tty_ldisc_enable. We take > tty_lock, set TTY_LDISC_CHANGING, put tty_lock, do some other work, take > tty_lock, call tty_ldisc_enable, put tty_lock.
Oh, "do some other work" includes tty_ldisc_halt where TTY_LDISC is cleared and tty_lock is _not_ held.
> So the only option I see is 3) and we should do: > --- a/drivers/tty/tty_io.c > +++ b/drivers/tty/tty_io.c > @@ -1310,7 +1310,8 @@ static int tty_reopen(struct tty_struct *tty) > { > struct tty_driver *driver = tty->driver; > > - if (test_bit(TTY_CLOSING, &tty->flags)) > + if (test_bit(TTY_CLOSING, &tty->flags) || > + test_bit(TTY_LDISC_CHANGING, &tty->flags)) > return -EIO; > > if (driver->type == TTY_DRIVER_TYPE_PTY && > > Alan, Greg? > > thanks, -- js suse labs
| |