Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 26 Jan 2016 07:57:44 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r294778 - in head: lib/libc/sys sys/kern
Message-ID:  <201601260757.u0Q7viGW029949@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kib
Date: Tue Jan 26 07:57:44 2016
New Revision: 294778
URL: https://svnweb.freebsd.org/changeset/base/294778

Log:
  Restore flushing of output for revoke(2) again.  Document revoke()'s
  intended behaviour in its man page.  Simplify tty_drain() to match.
  Don't call ttydevsw methods in tty_flush() if the device is gone
  since we now sometimes call it then.
  
  The flushing was supposed to be implemented by passing the FNONBLOCK
  flag to VOP_CLOSE() for revoke().  The tty driver is one of the few
  that can block in close and was one of the fewer that knew about this.
  
  This almost worked in FreeBSD-1 and similarly in Net/2.  These
  versions only almost worked because there was and is considerable
  confusion between IO_NDELAY and FNONBLOCK (aka O_NONBLOCK).  IO_NDELAY
  is only valid for VOP_READ() and VOP_WRITE().  For other VOPs it has
  the same value as O_SHLOCK.  But since vfs_subr.c and tty.c
  consistently used the wrong flag and the O_SHLOCK flag is rarely set,
  this mostly worked.  It also gave the feature than applications could
  get the non-blocking close by abusing O_SHLOCK.
  
  This was first broken then fixed in 1995.  I changed only the tty
  driver to use FNONBLOCK, as a hack to get non-blocking via the normal
  flag FNONBLOCK for last closes.  I didn't know about revoke()'s use
  of IO_NDELAY or change it to be consistent, so revoke() was broken.
  Then I changed revoke() to match.
  
  This was next broken in 1997 then fixed in 1998.  Importing Lite2 made
  the flags inconsistent again by undoing the fix only in vfs_subr.c.
  
  This was next broken in 2008 by replacing everything in tty.c and not
  checking any flags in last close.  Other bugs in draining limited the
  resulting unbounded waits to drain in some cases.
  
  It is now possible to fix this better using the new FREVOKE flag.
  Just restore flushing for revoke() for now.  Don't restore or undo any
  hacks for ordinary last closes yet.  But remove dead code in the
  1-second relative timeout (r272789).  This did extra work to extend
  the buggy draining for revoke() for as long as possible.  The 1-second
  timeout made this not very long by usually flushing after 1 second.
  
  Submitted by:	bde
  MFC after:	2 weeks

Modified:
  head/lib/libc/sys/revoke.2
  head/sys/kern/tty.c

Modified: head/lib/libc/sys/revoke.2
==============================================================================
--- head/lib/libc/sys/revoke.2	Tue Jan 26 07:49:11 2016	(r294777)
+++ head/lib/libc/sys/revoke.2	Tue Jan 26 07:57:44 2016	(r294778)
@@ -31,7 +31,7 @@
 .\"     @(#)revoke.2	8.1 (Berkeley) 6/4/93
 .\" $FreeBSD$
 .\"
-.Dd June 4, 1993
+.Dd Jan 25, 2016
 .Dt REVOKE 2
 .Os
 .Sh NAME
@@ -59,7 +59,8 @@ and a
 system call will succeed.
 If the file is a special file for a device which is open,
 the device close function
-is called as if all open references to the file had been closed.
+is called as if all open references to the file had been closed
+using a special close method which does not block.
 .Pp
 Access to a file may be revoked only by its owner or the super user.
 The
@@ -104,3 +105,6 @@ The
 .Fn revoke
 system call first appeared in
 .Bx 4.3 Reno .
+.Sh BUGS
+The non-blocking close method is only correctly implemented for
+terminal devices.

Modified: head/sys/kern/tty.c
==============================================================================
--- head/sys/kern/tty.c	Tue Jan 26 07:49:11 2016	(r294777)
+++ head/sys/kern/tty.c	Tue Jan 26 07:57:44 2016	(r294778)
@@ -126,7 +126,7 @@ static int
 tty_drain(struct tty *tp, int leaving)
 {
 	size_t bytesused;
-	int error, revokecnt;
+	int error;
 
 	if (ttyhook_hashook(tp, getc_inject))
 		/* buffer is inaccessible */
@@ -141,18 +141,10 @@ tty_drain(struct tty *tp, int leaving)
 
 		/* Wait for data to be drained. */
 		if (leaving) {
-			revokecnt = tp->t_revokecnt;
 			error = tty_timedwait(tp, &tp->t_outwait, hz);
-			switch (error) {
-			case ERESTART:
-				if (revokecnt != tp->t_revokecnt)
-					error = 0;
-				break;
-			case EWOULDBLOCK:
-				if (ttyoutq_bytesused(&tp->t_outq) < bytesused)
-					error = 0;
-				break;
-			}
+			if (error == EWOULDBLOCK &&
+			    ttyoutq_bytesused(&tp->t_outq) < bytesused)
+				error = 0;
 		} else
 			error = tty_wait(tp, &tp->t_outwait);
 
@@ -356,6 +348,10 @@ ttydev_close(struct cdev *dev, int fflag
 		return (0);
 	}
 
+	/* If revoking, flush output now to avoid draining it later. */
+	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.
@@ -1460,13 +1456,16 @@ tty_flush(struct tty *tp, int flags)
 		tp->t_flags &= ~TF_HIWAT_OUT;
 		ttyoutq_flush(&tp->t_outq);
 		tty_wakeup(tp, FWRITE);
-		ttydevsw_pktnotify(tp, TIOCPKT_FLUSHWRITE);
+		if (!tty_gone(tp))
+			ttydevsw_pktnotify(tp, TIOCPKT_FLUSHWRITE);
 	}
 	if (flags & FREAD) {
 		tty_hiwat_in_unblock(tp);
 		ttyinq_flush(&tp->t_inq);
-		ttydevsw_inwakeup(tp);
-		ttydevsw_pktnotify(tp, TIOCPKT_FLUSHREAD);
+		if (!tty_gone(tp)) {
+			ttydevsw_inwakeup(tp);
+			ttydevsw_pktnotify(tp, TIOCPKT_FLUSHREAD);
+		}
 	}
 }
 



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