While writing a virtio-console driver, I have found a bug in ttwrite()
that can cause hangs. Below is a fix and after talking to Theo, I would
like to know if the patch causes regressions for anyone, for example hangs
in pty input/output, serial ports, etc. Thanks in advance.
Cheers,
Stefan
- Introduce new defines TTHIWATMINSPACE, TTMINHIWAT for some magic values
that are used in tty.c.
- Remove hiwat adjustments in ttwrite(). This fixes the missing spltty().
- The above alone causs deadlocks with ptys. Change ttysetwater() to keep
at least TTHIWATMINSPACE space above the high water mark. This makes it
consistent with ttycheckoutq() and seems to fix the pty deadlocks.
--- sys/kern/tty.c
+++ sys/kern/tty.c
@@ -1688,7 +1688,7 @@ ttycheckoutq(struct tty *tp, int wait)
hiwat = tp->t_hiwat;
s = spltty();
oldsig = wait ? curproc->p_siglist : 0;
- if (tp->t_outq.c_cc > hiwat + 200)
+ if (tp->t_outq.c_cc > hiwat + TTHIWATMINSPACE)
while (tp->t_outq.c_cc > hiwat) {
ttstart(tp);
if (wait == 0 || curproc->p_siglist != oldsig) {
@@ -1823,7 +1823,7 @@ loop:
tp->t_rocount = 0;
if (ttyoutput(*cp, tp) >= 0) {
/* out of space */
- goto overfull;
+ goto ovhiwat;
}
cp++;
cc--;
@@ -1849,7 +1849,7 @@ loop:
tp->t_outcc += ce;
if (i > 0) {
/* out of space */
- goto overfull;
+ goto ovhiwat;
}
if (ISSET(tp->t_lflag, FLUSHO) ||
tp->t_outq.c_cc > hiwat)
@@ -1869,15 +1869,6 @@ done:
explicit_bzero(obuf, obufcc);
return (error);
-overfull:
- /*
- * Since we are using ring buffers, if we can't insert any more into
- * the output queue, we can assume the ring is full and that someone
- * forgot to set the high water mark correctly. We set it and then
- * proceed as normal.
- */
- hiwat = tp->t_outq.c_cc - 1;
-
ovhiwat:
ttstart(tp);
s = spltty();
@@ -2114,7 +2105,7 @@ ttsetwater(struct tty *tp)
cps = tp->t_ospeed / 10;
tp->t_lowat = x = CLAMP(cps / 2, TTMAXLOWAT, TTMINLOWAT);
x += cps;
- tp->t_hiwat = CLAMP(x, tp->t_outq.c_cn, 100);
+ tp->t_hiwat = CLAMP(x, tp->t_outq.c_cn - TTHIWATMINSPACE, TTMINHIWAT);
#undef CLAMP
}
--- sys/sys/tty.h
+++ sys/sys/tty.h
@@ -171,6 +171,8 @@ struct itty {
#ifdef _KERNEL
#define TTMAXLOWAT 256
#define TTMINLOWAT 32
+#define TTMINHIWAT 100
+#define TTHIWATMINSPACE 200 /* Min space above hiwat */
#endif
/* These flags are kept in t_state. */
