From owner-svn-src-head@FreeBSD.ORG Tue Jul 31 08:21:25 2012 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id AE512106566B; Tue, 31 Jul 2012 08:21:25 +0000 (UTC) (envelope-from listlog2011@gmail.com) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 8F2D58FC0A; Tue, 31 Jul 2012 08:21:25 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id q6V8LMCv081509; Tue, 31 Jul 2012 08:21:23 GMT (envelope-from listlog2011@gmail.com) Message-ID: <50179581.9070805@gmail.com> Date: Tue, 31 Jul 2012 16:21:21 +0800 From: David Xu User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:14.0) Gecko/20120713 Thunderbird/14.0 MIME-Version: 1.0 To: Giovanni Trematerra References: <201207310548.q6V5mZHf091624@svn.freebsd.org> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, Konstantin Belousov , David Xu , svn-src-head@freebsd.org, bde@freebsd.org Subject: Re: svn commit: r238936 - in head/sys: fs/fifofs kern sys X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: davidxu@freebsd.org List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 31 Jul 2012 08:21:25 -0000 On 2012/7/31 15:22, Giovanni Trematerra wrote: > On Tue, Jul 31, 2012 at 7:48 AM, David Xu wrote: >> Author: davidxu >> Date: Tue Jul 31 05:48:35 2012 >> New Revision: 238936 >> URL: http://svn.freebsd.org/changeset/base/238936 >> >> Log: >> I am comparing current pipe code with the one in 8.3-STABLE r236165, >> I found 8.3 is a history BSD version using socket to implement FIFO >> pipe, it uses per-file seqcount to compare with writer generation >> stored in per-pipe object. The concept is after all writers are gone, >> the pipe enters next generation, all old readers have not closed the >> pipe should get the indication that the pipe is disconnected, result >> is they should get EPIPE, SIGPIPE or get POLLHUP in poll(). >> But newcomer should not know that previous writters were gone, it >> should treat it as a fresh session. >> I am trying to bring back FIFO pipe to history behavior. It is still >> unclear that if single EOF flag can represent SBS_CANTSENDMORE and >> SBS_CANTRCVMORE which socket-based version is using, but I have run >> the poll regression test in tool directory, output is same as the one >> on 8.3-STABLE now. >> I think the output "not ok 18 FIFO state 6b: poll result 0 expected 1. >> expected POLLHUP; got 0" might be bogus, because newcomer should not >> know that old writers were gone. I got the same behavior on Linux. >> Our implementation always return POLLIN for disconnected pipe even it >> should return POLLHUP, but I think it is not wise to remove POLLIN for >> compatible reason, this is our history behavior. >> > I'm sorry but I'm failing to understand the reason for this change. > Can you point me out a test that confirm that the change is needed. > The only thing I see is an increase in the memory footprint for the pipes. > There was a lot of discussions on this topic on -arch mailing list > > http://lists.freebsd.org/pipermail/freebsd-arch/2012-January/012131.html > http://lists.freebsd.org/pipermail/freebsd-arch/2012-February/012314.html > > Thank you > > -- > Gianni > The old code broke some history semantic of FIFO pipe, you can try the test tool /usr/src/tools/regression/poll/pipepoll, try it before and after my commit, also compare the result with 8.3-STABLE, without this commit, both sub-tests 6c and 6d failed. I think old code did not mimic original code correctly, in 8.3-STABLE code, seqcount is stored in each file, writer generation detection is based on each copy of seqcount, but your code stored single copy of seqcount in fifoinfo object which is store as vnode data, and made the writer generation flag global by setting PIPE_SAMEWGEN in pipe object and used this flag to determine if it should return POLLHUP/POLLIN or not, this is wrong, for example: when there is no writer but have old readers, new incoming reader will executes: line 174 and 175: fip->fi_seqcount = fip->fi_wgen - fip->fi_writers; FIFO_WPDWGEN(fip, fpipe); this causes fi_seqcount to be equal to fi_wgen because fi_writer is zero, and FIFO_WPDWGEN() turns on flag PIPE_SAMEWGEN. When PIPE_SAMEWGEN is on, pipe_poll() ignores EOF, this breaks old readers, it causes old reader to get nothing while it should get POLLHUP from poll(). The new incoming reader should get nothing, so I think sub-tests 6b is wrong.