[Devel] [PATCH rh7] tty: Fix task hang if one of peers is sitting in read

[Vladimir Davydov]

On Fri, Jan 29, 2016 at 01:08:32PM +0300, Cyrill Gorcunov wrote:
> On Fri, Jan 29, 2016 at 12:55:11PM +0300, Vladimir Davydov wrote:
> > On Thu, Jan 28, 2016 at 06:30:58PM +0300, Cyrill Gorcunov wrote:
> > 
> > > @@ -679,16 +679,13 @@ void tty_ldisc_hangup(struct tty_struct
> > >  	wake_up_interruptible_poll(&tty->write_wait, POLLOUT);
> > >  	wake_up_interruptible_poll(&tty->read_wait, POLLIN);
> > >  
> > > -	tty_unlock(tty);
> > > -
> > >  	/*
> > >  	 * Shutdown the current line discipline, and reset it to
> > >  	 * N_TTY if need be.
> > >  	 *
> > >  	 * Avoid racing set_ldisc or tty_ldisc_release
> > >  	 */
> > > -	tty_ldisc_lock_pair(tty, tty->link);
> > > -	tty_lock(tty);
> > > +	tty_ldisc_lock(tty, MAX_SCHEDULE_TIMEOUT);
> > 
> > Before this patch tty_lock nested into tty_ldisc_lock, not it's
> > vice-versa in this function, but e.g. tty_set_ldisc still follows the
> > former locking order. Can't it result in a deadlock or, at least,
> > lockdep complains?
> 
> Nope, as far as I can tell. Previously we've the following
> tty_lock -> tty_unlock-> tty_ldisc_lock_pair
> which was needed for tty_ldisc_release. And such locking
> end up in hanging task when one peer is closed but other
> waiting for input. It was reworked in vanilla kernel so
> tty_lock/unlock no longer used. I made the same because
> it's enough I think. Note I dropped tty_lock/unlock
> from _both_ tty_ldisc_hangup and ldisk release.

Yeah, but in vanilla kernel, they changed locking order in tty_set_ldisc
either, while you didn't. I guess you should backport one more commit:

commit c8483bc9deff9bf9118aaab2e840b973b75cac3e
Author: Peter Hurley <peter at hurleysoftware.com>
Date:   Wed Nov 5 12:12:45 2014 -0500

    tty: Invert tty_lock/ldisc_sem lock order

> 
> I run the tests and run container manually with all debug
> turned on and I think we're safe. But lets continue testing.