On 2012/7/31 15:22, Giovanni Trematerra wrote:
On Tue, Jul 31, 2012 at 7:48 AM, David Xu <davi...@freebsd.org> 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.
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
