Re: commit af32cc7b causes stalls on 3.18.23

[Sasha Levin <sasha.levin@oracle.com>]

On 10/31/2015 01:37 AM, Corey Wright wrote:
> On Fri, 30 Oct 2015 22:07:24 -0400
> Sasha Levin <sasha.levin@oracle.com> wrote:
> 
>> > On 10/30/2015 09:56 PM, Corey Wright wrote:
>>> > > Running applications under GNU Screen causes the 3.18.23 kernel to stall and
>>> > > the application to hang.  Reverting commit af32cc7b, ie "tty: fix stall caused
>>> > > by missing memory barrier in drivers/tty/n_tty.c", fixes the problem.
>> > 
>> > Hi Corey,
>> > 
>> > Does doing the same on mainline work for you?
> is 4.1.12 close enough (which i already had "on-hand")?
>  
> it contains a similar commit 614ea4ea and tests successfully (ie no bug).
> 
> so the problem appears to be limited to 3.18.23.
> 
> thanks for getting me looking at and comparing the two versions!
> 
> in 3.18.23 we add a spin_unlock_irqrestore inside the "if (tty->link->packet)"
> block, but we already have one outside of the if block.
> 
> i'm thinking it accidentally got dragged along when porting the commit from
> 4.1 to 3.18 (because the one that was added in 3.18.23 is in the same location
> as the one already in 4.1).
> 
> the below patch fixes the problem and passes the
> apt-and-lxc-start-run-under-screen-without-causing-a-stall test.
> 
> ---------- >8 ----- cut here ----- 8< ----------
> Remove extraneous, unmatched spin_unlock_irqrestore() introduced by commit
> af32cc7b.
> 
> This prevents stalls when running command-line applications under GNU Screen.
> 
> Fixes: af32cc7bde63 ("tty: fix stall caused by missing memory barrier in drivers/tty/n_tty.c")
> Signed-off-by: Corey Wright <undefined@pobox.com>
> 
> --- linux-3.18.23~/drivers/tty/n_tty.c	2015-10-30 23:46:48.000000000 -0500
> +++ linux-3.18.23/drivers/tty/n_tty.c	2015-10-30 23:46:53.821376173 -0500
> @@ -364,7 +364,6 @@ static void n_tty_packet_mode_flush(stru
>  	spin_lock_irqsave(&tty->ctrl_lock, flags);
>  	if (tty->link->packet) {
>  		tty->ctrl_status |= TIOCPKT_FLUSHREAD;
> -		spin_unlock_irqrestore(&tty->ctrl_lock, flags);
>  		wake_up_interruptible(&tty->link->read_wait);
>  	}
>  	spin_unlock_irqrestore(&tty->ctrl_lock, flags);

Hey Corey,

Thanks for looking into it!

I've pushed a corrected backport to the stable queue, if you could please confirm
that it works for you I'll ship it as 3.18.24.

The tree is available at git://git.kernel.org/pub/scm/linux/kernel/git/sashal/linux-stable.git linux-3.18.y-queue

Thanks again!