Date: Tue, 26 Jan 2016 20:10:30 +1100 From: Kubilay Kocak <koobs@FreeBSD.org> To: Konstantin Belousov <kib@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r294778 - in head: lib/libc/sys sys/kern Message-ID: <56A73806.4080800@FreeBSD.org> In-Reply-To: <201601260757.u0Q7viGW029949@repo.freebsd.org> References: <201601260757.u0Q7viGW029949@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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); > + } > } > } >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?56A73806.4080800>