Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 25 Oct 2012 09:00:01 GMT
From:      Jeremy Chadwick <jdc@koitsu.org>
To:        freebsd-bugs@FreeBSD.org
Subject:   Re: kern/173010: Backgrounded processes remain in ttyin state / SIGCONT does not remove sleeping processes from sleep queue
Message-ID:  <201210250900.q9P901AG042806@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/173010; it has been noted by GNATS.

From: Jeremy Chadwick <jdc@koitsu.org>
To: FreeBSD-gnats-submit@FreeBSD.org, freebsd-bugs@FreeBSD.org
Cc:  
Subject: Re: kern/173010: Backgrounded processes remain in ttyin state /
 SIGCONT does not remove sleeping processes from sleep queue
Date: Thu, 25 Oct 2012 01:51:45 -0700

 --ReaqsoxgOBHFXBhH
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 
 Mail thread on -stable at this point, including a patch from ed@
 
 From: Ed Schouten <ed@80386.nl>
 To: Konstantin Belousov <kostikbel@gmail.com>
 Date: Thu, 25 Oct 2012 09:45:11 +0200
 Cc: Jeremy Chadwick <jdc@koitsu.org>, freebsd-stable@freebsd.org, jhb@freebsd.org
 Subject: Re: pty/tty or signal strangeness, or grep/bsdgrep bug?
 
 Hi all,
 
 2012/10/23 Ed Schouten <ed@80386.nl>:
 > Will try to come up with a decent patch tomorrow evening.
 
 Ahem; the day after tomorrow. Jeremy, could you please try the following patch?
 
 http://80386.nl/pub/tty-bg-read.txt
 
 I decomposed the TTY read routine into four separate functions to
 improve clarity. While this was initially true, I think it's a pity
 the four functions are constantly becoming a bit more complex.
 
 The same issue is also present on the output path, but I have no idea
 how realistic/hard it is to fix this issue. Also, it might not really
 be an issue in practice. If you do a large write and become a
 non-foreground process group, you might be able to circumvent TOSTOP
 while the write() is in transit.
 
 Fixing this might be tedious, because we currently enforce that writes
 to a TTY are serialized. Blocking inside the write() might then cause
 a deadlock. But in my opinion, I would prefer the serialization over
 the enforcing of TOSTOP.
 
 Thanks again for reporting the issue!
 
 -- 
 Ed Schouten <ed@80386.nl>
 
 
 
 
 From: Jeremy Chadwick <jdc@koitsu.org>
 To: Ed Schouten <ed@80386.nl>
 Date: Thu, 25 Oct 2012 01:46:03 -0700
 Cc: Konstantin Belousov <kostikbel@gmail.com>, freebsd-stable@freebsd.org, jhb@freebsd.org
 Subject: Re: pty/tty or signal strangeness, or grep/bsdgrep bug?
 
 On Thu, Oct 25, 2012 at 09:45:11AM +0200, Ed Schouten wrote:
 > 2012/10/23 Ed Schouten <ed@80386.nl>:
 > > Will try to come up with a decent patch tomorrow evening.
 >
 > Ahem; the day after tomorrow. Jeremy, could you please try the following patch?
 >
 > http://80386.nl/pub/tty-bg-read.txt
 >
 > I decomposed the TTY read routine into four separate functions to
 > improve clarity. While this was initially true, I think it's a pity
 > the four functions are constantly becoming a bit more complex.
 >
 > The same issue is also present on the output path, but I have no idea
 > how realistic/hard it is to fix this issue. Also, it might not really
 > be an issue in practice. If you do a large write and become a
 > non-foreground process group, you might be able to circumvent TOSTOP
 > while the write() is in transit.
 >
 > Fixing this might be tedious, because we currently enforce that writes
 > to a TTY are serialized. Blocking inside the write() might then cause
 > a deadlock. But in my opinion, I would prefer the serialization over
 > the enforcing of TOSTOP.
 
 After the patch, testing with grep and cat, and checking cv/state for
 the latter case:
 
 
 (01:30:39 jdc@icarus) ~ $ csh
 % grep -r "-2011" . &
 [1] 1964
 % jobs
 [1]  + Suspended (tty input)         grep -r -2011 .
 % fg
 grep -r -2011 .
 ^C
 % jobs
 
 % grep -r "-2011" .
 ^Z
 Suspended
 % bg
 [1]    grep -r -2011 . &
 [1]  + Suspended (tty input)         grep -r -2011 .
 % fg
 grep -r -2011 .
 ^C
 
 % cat &
 [1] 2042
 % jobs
 [1]  + Suspended (tty input)         cat
 % ps -auxwU jdc | grep cat
 jdc  2042  0.0  0.0  9908 1496  1  T     1:34AM 0:00.00 cat
 jdc  2044  0.0  0.0 16268 1864  1  S+    1:34AM 0:00.00 grep cat
 % top -b -U jdc | grep cat
  2042 jdc         1  20    0  9908K  1496K STOP    0   0:00  0.00% cat
  2047 jdc         1  20    0 16268K  1864K piperd  0   0:00  0.00% grep cat
 % fg
 cat
 ^C
 % exit
 
 
 I do not get dropped characters or witness any other anomalies.  I
 tested behaviour with /bin/sh, as well as bash.  All seems good.
 
 > Thanks again for reporting the issue!
 
 No, thank *you* and others for looking + fixing it!  :-)
 
 I assume a commit to HEAD + MFC in 2 weeks is in order?
 
 I'll update the PR with this part of our mail thread.
 
 -- 
 | Jeremy Chadwick                                   jdc@koitsu.org |
 | UNIX Systems Administrator                http://jdc.koitsu.org/ |
 | Mountain View, CA, US                                            |
 | Making life hard for others since 1977.             PGP 4BD6C0CB |
 
 
 --ReaqsoxgOBHFXBhH
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: attachment; filename="tty-bg-read.txt"
 
 Index: sys/kern/tty.c
 ===================================================================
 --- sys/kern/tty.c	(Revision 241962)
 +++ sys/kern/tty.c	(Arbeitskopie)
 @@ -361,7 +361,7 @@
  	return (p->p_session == tp->t_session && p->p_flag & P_CONTROLT);
  }
  
 -static int
 +int
  tty_wait_background(struct tty *tp, struct thread *td, int sig)
  {
  	struct proc *p = td->td_proc;
 @@ -433,13 +433,6 @@
  	error = ttydev_enter(tp);
  	if (error)
  		goto done;
 -
 -	error = tty_wait_background(tp, curthread, SIGTTIN);
 -	if (error) {
 -		tty_unlock(tp);
 -		goto done;
 -	}
 -
  	error = ttydisc_read(tp, uio, ioflag);
  	tty_unlock(tp);
  
 Index: sys/kern/tty_ttydisc.c
 ===================================================================
 --- sys/kern/tty_ttydisc.c	(Revision 241962)
 +++ sys/kern/tty_ttydisc.c	(Arbeitskopie)
 @@ -126,6 +126,10 @@
  	breakc[n] = '\0';
  
  	do {
 +		error = tty_wait_background(tp, curthread, SIGTTIN);
 +		if (error)
 +			return (error);
 +
  		/*
  		 * Quite a tricky case: unlike the old TTY
  		 * implementation, this implementation copies data back
 @@ -192,6 +196,10 @@
  	 */
  
  	for (;;) {
 +		error = tty_wait_background(tp, curthread, SIGTTIN);
 +		if (error)
 +			return (error);
 +
  		error = ttyinq_read_uio(&tp->t_inq, tp, uio,
  		    uio->uio_resid, 0);
  		if (error)
 @@ -229,6 +237,10 @@
  	timevaladd(&end, &now);
  
  	for (;;) {
 +		error = tty_wait_background(tp, curthread, SIGTTIN);
 +		if (error)
 +			return (error);
 +
  		error = ttyinq_read_uio(&tp->t_inq, tp, uio,
  		    uio->uio_resid, 0);
  		if (error)
 @@ -278,6 +290,10 @@
  	 */
  
  	for (;;) {
 +		error = tty_wait_background(tp, curthread, SIGTTIN);
 +		if (error)
 +			return (error);
 +
  		error = ttyinq_read_uio(&tp->t_inq, tp, uio,
  		    uio->uio_resid, 0);
  		if (error)
 Index: sys/sys/tty.h
 ===================================================================
 --- sys/sys/tty.h	(Revision 241962)
 +++ sys/sys/tty.h	(Arbeitskopie)
 @@ -180,6 +180,7 @@
  void	tty_signal_pgrp(struct tty *tp, int signal);
  /* Waking up readers/writers. */
  int	tty_wait(struct tty *tp, struct cv *cv);
 +int	tty_wait_background(struct tty *tp, struct thread *td, int sig);
  int	tty_timedwait(struct tty *tp, struct cv *cv, int timo);
  void	tty_wakeup(struct tty *tp, int flags);
  
 
 --ReaqsoxgOBHFXBhH--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201210250900.q9P901AG042806>