On Tue, 31 Jul 2012, David Xu wrote:
On 2012/7/31 15:22, Giovanni Trematerra wrote:
On Tue, Jul 31, 2012 at 7:48 AM, David Xu <davi...@freebsd.org> wrote:
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.
Good commit message. Almost worth quoting in 3 followups :-). I wrote
most of the code and forgotten some details, and the above made them
clear.
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.
Not very historic. Only FreeBSD-8 (maybe 9?) did that.
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.
6b is intentionally different from Linux. I forget if it is to reduce
races with readers or just to simply the implementation and understanding
it. New readers simply joing old readers with a hangup set for all if
they manage to open the fifo (necessarily using O_NONBLOCK) after the
hangup but before the old readers go away. Since this seems to increase
races, I may remember it backwards
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.
This is historical back to FreeBSD-3 (earlier versions didn't have
poll()). I think it is just a bug. POLLHUP was unimplemented for
most file types before FreeBSD-8, and setting POLLIN works around this
for most callers. I tried to get it fixed for at least fifos when I
fixed POLLHUP for some file types. No one uses fifos, so they are
safer to fix than sockets :-).
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
Many poll regression tests fail.
http://lists.freebsd.org/pipermail/freebsd-arch/2012-January/012131.html
http://lists.freebsd.org/pipermail/freebsd-arch/2012-February/012314.html
There are also a lot of old PRs about this for poll() (not for your new
fifo implementation). I think the PRs are mentioned in these threads.
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.
Luckily I have forgotten the details for fifos and never understood them
all for nameless pipes, so you get to fix it :-).
Bruce
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
