From owner-svn-src-all@freebsd.org Tue Jan 26 09:10:39 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 CB4C0A47DF2; Tue, 26 Jan 2016 09:10:39 +0000 (UTC) (envelope-from koobs.freebsd@gmail.com) Received: from mail-pa0-x231.google.com (mail-pa0-x231.google.com [IPv6:2607:f8b0:400e:c03::231]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 9CBC51130; Tue, 26 Jan 2016 09:10:39 +0000 (UTC) (envelope-from koobs.freebsd@gmail.com) Received: by mail-pa0-x231.google.com with SMTP id cy9so95246038pac.0; Tue, 26 Jan 2016 01:10:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:reply-to:subject:references:to:from:message-id:date :user-agent:mime-version:in-reply-to:content-type :content-transfer-encoding; bh=NPw4+8QZijRdxrTP/oQtQqKPIRIq9P8u1aH+ssQkOAA=; b=HygSFB3K93wWWFF8jKLtzIea7kGKXqfXRrqDf/TFoB57pNbLMOjyHSXFazLPWsg+q/ Y20orZrHquN2K/xD6ZqOITVtzQp+i3HKkAy5S0LljOwiB8sjThS0PavQXnSE+cdlyncw NJfqgCASmoHXP4RnweEQqaIszWCzqMoBbwJXD2o5x1ozXoLYa6Vm5zYcmu+zj/wtqpyu 3pozs0J09f2ZGUxB0zhzHqJOA2DYHKr2Somaj/Eql9Q40JvB0IgalAv7Z2jOHfpDdBB8 bwyKCLpWOiI57YnqlJsJFne6TtJ+iaAWxkbtWo3BMFjhgK5BggoT3EusQ9mTQS0c+tQA g8sQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:reply-to:subject:references:to:from :message-id:date:user-agent:mime-version:in-reply-to:content-type :content-transfer-encoding; bh=NPw4+8QZijRdxrTP/oQtQqKPIRIq9P8u1aH+ssQkOAA=; b=AcdFuGGOVFYZZDmi+UJzTY5jJ4hd0+KjfTaVJjmfa+9vdUqVqA7Fi0jMKrbscgJMOi nawrB9Ssud2hfuokhP6mtjHbUYpw80sAlPfR7gQd9k+jnjblo1YNO1bavPjxCDtEL5SO HmR4IP2B2WJCXi1LQyEAS0BvtwX63rDsSG6rhq8a/AFu8O5aRlzV1eKD20R1Jv/3XZjG yr9kgTEMoqSaMIvh1PThXlB9sp/Us47gpqYfHhQo49YQCDUYwaXu6wGLHBoZqUonNZUZ s55vlXjZHepufBHwJOZGrEThJ181wYJixR6cXh4EyAENHXrTPSkzZMuLiRi/lzWhFWva W8WA== X-Gm-Message-State: AG10YOTDC/qH8+8sjXas7fBV2eYxmzPYIglL73D/A4ArN0ta05HKt8hlwA0djY0yAocmAw== X-Received: by 10.66.194.230 with SMTP id hz6mr21774015pac.70.1453799439028; Tue, 26 Jan 2016 01:10:39 -0800 (PST) Received: from ?IPv6:2001:44b8:31ae:7b01:e955:70fa:3edd:219e? (2001-44b8-31ae-7b01-e955-70fa-3edd-219e.static.ipv6.internode.on.net. [2001:44b8:31ae:7b01:e955:70fa:3edd:219e]) by smtp.gmail.com with ESMTPSA id m87sm634467pfi.47.2016.01.26.01.10.36 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 26 Jan 2016 01:10:38 -0800 (PST) Sender: Kubilay Kocak Reply-To: koobs@FreeBSD.org Subject: Re: svn commit: r294778 - in head: lib/libc/sys sys/kern References: <201601260757.u0Q7viGW029949@repo.freebsd.org> To: Konstantin Belousov , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org From: Kubilay Kocak Message-ID: <56A73806.4080800@FreeBSD.org> Date: Tue, 26 Jan 2016 20:10:30 +1100 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:44.0) Gecko/20100101 Thunderbird/44.0 MIME-Version: 1.0 In-Reply-To: <201601260757.u0Q7viGW029949@repo.freebsd.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit 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 09:10:39 -0000 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); > + } > } > } >