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

Cyrill Gorcunov gorcunov at virtuozzo.com
Fri Jan 29 02:08:32 PST 2016


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.

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

	Cyrill


More information about the Devel mailing list