svn commit: r294778 - in head: lib/libc/sys sys/kern

[Kubilay Kocak]

On 26/01/2016 6:57 PM, Konstantin Belousov wrote:
> 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.

Seems like

>   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.

A fantastic

>   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.

Regression test candidate :)

>   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);
> +		}
>  	}
>  }
>