Re: svn commit: r294778 - in head: lib/libc/sys sys/kern
On 26/01/2016 10:38 PM, Bruce Evans wrote: > On Tue, 26 Jan 2016, Kubilay Kocak wrote: > >>> 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. >>> ... >>> This was first broken then fixed in 1995. I changed only the tty >>> ... >> >> Seems like >> >>> This was next broken in 1997 then fixed in 1998. Importing Lite2 made >>> ... >> >> A fantastic >> >>> This was next broken in 2008 by replacing everything in tty.c and not >>> ... >> >> Regression test candidate :) >> >>> It is now possible to fix this better using the new FREVOKE flag. > > Regression tests for devices are difficult to write and more difficult > to run. Simpler for ttys than for networking or disks, but you still > need at least 2 generic tty ports just to test things that are not > very related to hardware. Of course, though perhaps mocking external subsystems/components is something we could do, as is relatively standard for testing only the code you want to cover, rather than (requiring) the entire integration. > Bugs in flushing and draining are sometimes obvious by observing if > echo 123 >/dev/ttyXx works when it should fail or fails when it should > work. > > For more arcane bugs, I use the old NIST POSIX test suite. This is > badly written and hard to use and not very complete, but it finds about > 30 regressions between FreeBSD-5 and FreeBSD-9. 30 over-counts for error > cascades but undercounts for blocking and some other timing bugs, and > of course strict POSIX tests don't get near FreeBSD features like > revoke() or bidrectional devices. > > Bruce ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r294778 - in head: lib/libc/sys sys/kern
On Tue, 26 Jan 2016, Kubilay Kocak wrote: 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. ... This was first broken then fixed in 1995. I changed only the tty ... Seems like This was next broken in 1997 then fixed in 1998. Importing Lite2 made ... A fantastic This was next broken in 2008 by replacing everything in tty.c and not ... Regression test candidate :) It is now possible to fix this better using the new FREVOKE flag. Regression tests for devices are difficult to write and more difficult to run. Simpler for ttys than for networking or disks, but you still need at least 2 generic tty ports just to test things that are not very related to hardware. Bugs in flushing and draining are sometimes obvious by observing if echo 123 >/dev/ttyXx works when it should fail or fails when it should work. For more arcane bugs, I use the old NIST POSIX test suite. This is badly written and hard to use and not very complete, but it finds about 30 regressions between FreeBSD-5 and FreeBSD-9. 30 over-counts for error cascades but undercounts for blocking and some other timing bugs, and of course strict POSIX tests don't get near FreeBSD features like revoke() or bidrectional devices. Bruce ___ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
Re: svn commit: r294778 - in head: lib/libc/sys sys/kern
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.2Tue Jan 26 07:49:11 2016
> (r294777)
> +++ head/lib/libc/sys/revoke.2Tue 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, >t_outwait, hz);
> - switch (error) {
> - case ERESTART:
> - if (revokecnt != tp->t_revokecnt)
> - error = 0;
> - break;
> - case EWOULDBLOCK:
> - if (ttyoutq_bytesused(>t_outq) < bytesused)
> - error = 0;
> - break;
> - }
> + if (error == EWOULDBLOCK &&
> + ttyoutq_bytesused(>t_outq) < bytesused)
> + error
svn commit: r294778 - in head: lib/libc/sys sys/kern
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, >t_outwait, hz);
- switch (error) {
- case ERESTART:
- if (revokecnt != tp->t_revokecnt)
- error = 0;
- break;
- case EWOULDBLOCK:
- if (ttyoutq_bytesused(>t_outq) < bytesused)
- error = 0;
- break;
- }
+ if (error == EWOULDBLOCK &&
+ ttyoutq_bytesused(>t_outq) < bytesused)
+ error = 0;
} else
error = tty_wait(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)
+
