Mail thread on -stable at this point, including a patch from ed@
From: Ed Schouten <[email protected]>
To: Konstantin Belousov <[email protected]>
Date: Thu, 25 Oct 2012 09:45:11 +0200
Cc: Jeremy Chadwick <[email protected]>, [email protected],
[email protected]
Subject: Re: pty/tty or signal strangeness, or grep/bsdgrep bug?
Hi all,
2012/10/23 Ed Schouten <[email protected]>:
> 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 <[email protected]>
From: Jeremy Chadwick <[email protected]>
To: Ed Schouten <[email protected]>
Date: Thu, 25 Oct 2012 01:46:03 -0700
Cc: Konstantin Belousov <[email protected]>, [email protected],
[email protected]
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 <[email protected]>:
> > 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 [email protected] |
| UNIX Systems Administrator http://jdc.koitsu.org/ |
| Mountain View, CA, US |
| Making life hard for others since 1977. PGP 4BD6C0CB |
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);
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-bugs
To unsubscribe, send any mail to "[email protected]"
