kern/173010: Backgrounded processes remain in ttyin state / SIGCONT does not remove sleeping processes from sleep queue
Jeremy Chadwick
jdc at koitsu.org
Thu Oct 25 09:00:02 UTC 2012
The following reply was made to PR kern/173010; it has been noted by GNATS.
From: Jeremy Chadwick <jdc at koitsu.org>
To: FreeBSD-gnats-submit at FreeBSD.org, freebsd-bugs at 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 at 80386.nl>
To: Konstantin Belousov <kostikbel at gmail.com>
Date: Thu, 25 Oct 2012 09:45:11 +0200
Cc: Jeremy Chadwick <jdc at koitsu.org>, freebsd-stable at freebsd.org, jhb at freebsd.org
Subject: Re: pty/tty or signal strangeness, or grep/bsdgrep bug?
Hi all,
2012/10/23 Ed Schouten <ed at 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 at 80386.nl>
From: Jeremy Chadwick <jdc at koitsu.org>
To: Ed Schouten <ed at 80386.nl>
Date: Thu, 25 Oct 2012 01:46:03 -0700
Cc: Konstantin Belousov <kostikbel at gmail.com>, freebsd-stable at freebsd.org, jhb at 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 at 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 at 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 at 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--
More information about the freebsd-bugs
mailing list