On Tue, 25 Aug 2009, Jilles Tjoelker wrote:
On Tue, Aug 25, 2009 at 04:07:11AM +1000, Bruce Evans wrote:
On Sun, 23 Aug 2009, Jilles Tjoelker wrote:
I think poll on fifos should instead be fixed by closing the
half-connection corresponding to writing from fi_readsock to
fi_writesock. I have tried this out, see attached patch. With the patch,
pipepoll only gives "expected POLLHUP; got POLLIN | POLLHUP" errors and
an error in fifo case 6b caused by our distinction between new and old
readers.
This sort of worked for me, but has several problems:
% Index: sys/fs/fifofs/fifo_vnops.c
% ===================================================================
% --- sys/fs/fifofs/fifo_vnops.c (revision 196459)
% +++ sys/fs/fifofs/fifo_vnops.c (working copy)
% @@ -193,6 +193,10 @@
% goto fail2;
% fip->fi_writesock = wso;
% error = soconnect2(wso, rso);
% + if (error == 0)
% + error = soshutdown(rso, SHUT_WR);
% + if (error == 0)
% + error = soshutdown(wso, SHUT_RD);
% if (error) {
% (void)soclose(wso);
% fail2:
The second soshutdown() is only harmful. I see the following state changes
- the first soshutdown() sets SBS_CANTRCVMORE on rso like you would expect,
and also sets SBS_CANTSENDMORE on wso. This gives the desired state.
- the second soshutdown() then clears SBS_CANTRCVMORE on rso (without
the first soshutdown() it leaves both flags clear in both directions).
This clobbers the desired state. The failure shows in just one of my
uncommitted regression tests (when there is a writer and there was
a reader, poll() returns POLLOUT for the writer, but should return
POLLHUP; the missing SBS_CANTRCVMORE on rso prevents it ever returning
POLLHUP for writers).
After removing the second soshutdown() and fixing a spurious POLLIN (see
below), all my tests pass.
I have removed the second shutdown, it is not necessary.
The second shutdown became harmless for me when I fixed the clobbering of
sb_state. Does it have any effect?
% Index: sys/kern/uipc_socket.c
% ===================================================================
% --- sys/kern/uipc_socket.c (revision 196469)
% +++ sys/kern/uipc_socket.c (working copy)
% @@ -2898,11 +2898,13 @@
% if (so->so_oobmark || (so->so_rcv.sb_state & SBS_RCVATMARK))
% revents |= events & (POLLPRI | POLLRDBAND);
%
% - if ((events & POLLINIGNEOF) == 0)
% - if (so->so_rcv.sb_state & SBS_CANTRCVMORE)
% - revents |= POLLHUP;
% - if (so->so_snd.sb_state & SBS_CANTSENDMORE)
% - revents |= POLLHUP;
% + if ((events & POLLINIGNEOF) == 0) {
Old problems become larger:
I don't like POLLINIGNEOF (for input) affecting POLLHUP for output. This
seems to cause no problems for fifos, at least when the kernel sets
POLLINIGNEOF, but it is hard to understand even why it doesn't cause
problems, and kib@ wants POLLINIGNEOF to remain user-settable, so the
complications might remain exported to userland for for fifos and
sockets, where they are harder to document and understand.
I do not like userland POLLINIGNEOF either. I think programs can do fine
with the standard functionality (closing and reopening a fifo to reset
the POLLHUP state).
% + revents |= events & (POLLIN | POLLRDNORM);
This gives spurious POLLINs when POLLHUP is also returned, and thus defeats
the point of soreadable_data() being different from soreadable(). Tests
6a-6d show the spurious POLLIN. I don't understand how tests 6a and 6c-6d
passed for you.
Same problem here. I think kib@ wants to keep this in 8.x for the sake
of buggy programs that do not check for POLLHUP. I suppose we can do it
properly in 9.x.
But it may break non-buggy programs, and doesn't help for buggy programs
like gdb. (gdb exits immediately when it sees POLLHUP, despite POLLIN
also being set and input being queued. gdb used to work before it was
converted to use poll(). Then it saw a ready input fd and had to read()
it to detect EOF.)
I now have better tests for POLLOUT, and have tested the buggy case
for select some more (test 9, and 6b (?) in current): if POLLINIGNEOF
is not applied for select(), then test 9 passes as expected. Test 9
only covers a fifo in initial state with a reader and no writers. This
is EOF for read() and input-ready for select(), but neither POLLIN nor
POLLHUP for poll(). Test 6b is for essentially the initial state for
read() and select(), but for poll() the state is too-sticky hangup
(POSIX) or back to the initial state (Linux-2.6.10/FreeBSD-current).
Bruce
_______________________________________________
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"
