From owner-svn-src-all@freebsd.org Tue Jan 26 14:46:41 2016 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 5AC1AA466DE; Tue, 26 Jan 2016 14:46:41 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 31ABB109; Tue, 26 Jan 2016 14:46:41 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id u0QEkeCD051440; Tue, 26 Jan 2016 14:46:40 GMT (envelope-from kib@FreeBSD.org) Received: (from kib@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id u0QEkeGF051438; Tue, 26 Jan 2016 14:46:40 GMT (envelope-from kib@FreeBSD.org) Message-Id: <201601261446.u0QEkeGF051438@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kib set sender to kib@FreeBSD.org using -f From: Konstantin Belousov Date: Tue, 26 Jan 2016 14:46:40 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r294836 - head/sys/kern X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Jan 2016 14:46:41 -0000 Author: kib Date: Tue Jan 26 14:46:39 2016 New Revision: 294836 URL: https://svnweb.freebsd.org/changeset/base/294836 Log: Don't clear the software flow control flag before draining for last close or assert the bug that it is clear when leaving. Remove an unrelated rotted comment that was attached to the buggy clearing. Since draining is not done in more cases, flushing is needed in more cases, so start fixing flushing: - do a full flush in ttydisc_close(). State what POSIX requires more clearly. This was missing ttydevsw_pktnotify() calls to tell the devsw layer to flush. Hardware tty drivers don't actually flush since they don't understand this API. - fix 2 missing wakeups in tty_flush(). Most of the wakeups here are unnecessary for last close. But ttydisc_close() did one of the missing ones. This flow control bug ameliorated the design bug of requiring potentially unbounded waits in draining. Software flow control is the easiest way to get an unbounded wait, and a long wait is sometimes actually useful. Users can type the xoff character on the receiver and (if ixon is set on the sender) expect the output to be held until the user is ready for more. Hardware flow control can also give the unbounded wait, and this bug didn't affect hardware flow control. Unbounded waits from hardware flow control take a more unusual configuration. E.g., a terminal program that controls the modem status lines, or unplugging the cable in a configuration where this doesn't break the connection. The design bug is still ameliorated by a newer bug in draining for last close -- the 1 second timeout. E.g., if the user types the xoff character and the sender reaches last close, then output is not resumed and the wait times out after just 1 second. This is broken, but preferable to an unbounded wait. Before this change, the output was resumed immediately and usually completed. Submitted by: bde MFC after: 2 weeks Modified: head/sys/kern/tty.c head/sys/kern/tty_ttydisc.c Modified: head/sys/kern/tty.c ============================================================================== --- head/sys/kern/tty.c Tue Jan 26 14:45:25 2016 (r294835) +++ head/sys/kern/tty.c Tue Jan 26 14:46:39 2016 (r294836) @@ -201,7 +201,6 @@ ttydev_leave(struct tty *tp) constty_clear(); /* Drain any output. */ - MPASS((tp->t_flags & TF_STOPPED) == 0); if (!tty_gone(tp)) tty_drain(tp, 1); @@ -352,11 +351,7 @@ ttydev_close(struct cdev *dev, int fflag if (fflag & FREVOKE) tty_flush(tp, FWRITE); - /* - * This can only be called once. The callin and the callout - * devices cannot be opened at the same time. - */ - tp->t_flags &= ~(TF_EXCLUDE|TF_STOPPED); + tp->t_flags &= ~TF_EXCLUDE; /* Properly wake up threads that are stuck - revoke(). */ tp->t_revokecnt++; @@ -1456,12 +1451,15 @@ tty_flush(struct tty *tp, int flags) tp->t_flags &= ~TF_HIWAT_OUT; ttyoutq_flush(&tp->t_outq); tty_wakeup(tp, FWRITE); - if (!tty_gone(tp)) + if (!tty_gone(tp)) { + ttydevsw_outwakeup(tp); ttydevsw_pktnotify(tp, TIOCPKT_FLUSHWRITE); + } } if (flags & FREAD) { tty_hiwat_in_unblock(tp); ttyinq_flush(&tp->t_inq); + tty_wakeup(tp, FREAD); if (!tty_gone(tp)) { ttydevsw_inwakeup(tp); ttydevsw_pktnotify(tp, TIOCPKT_FLUSHREAD); Modified: head/sys/kern/tty_ttydisc.c ============================================================================== --- head/sys/kern/tty_ttydisc.c Tue Jan 26 14:45:25 2016 (r294835) +++ head/sys/kern/tty_ttydisc.c Tue Jan 26 14:46:39 2016 (r294836) @@ -94,14 +94,11 @@ ttydisc_close(struct tty *tp) /* Clean up our flags when leaving the discipline. */ tp->t_flags &= ~(TF_STOPPED|TF_HIWAT|TF_ZOMBIE); - /* POSIX states we should flush when close() is called. */ - ttyinq_flush(&tp->t_inq); - ttyoutq_flush(&tp->t_outq); - - if (!tty_gone(tp)) { - ttydevsw_inwakeup(tp); - ttydevsw_outwakeup(tp); - } + /* + * POSIX states that we must drain output and flush input on + * last close. Draining has already been done if possible. + */ + tty_flush(tp, FREAD | FWRITE); if (ttyhook_hashook(tp, close)) ttyhook_close(tp);