Date: Tue, 31 Jul 2012 05:48:35 +0000 (UTC) From: David Xu <davidxu@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r238936 - in head/sys: fs/fifofs kern sys Message-ID: <201207310548.q6V5mZHf091624@svn.freebsd.org>
next in thread | raw e-mail | index | archive | help
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. Regression test: /usr/src/tools/regression/poll Modified: head/sys/fs/fifofs/fifo_vnops.c head/sys/kern/sys_pipe.c head/sys/sys/pipe.h Modified: head/sys/fs/fifofs/fifo_vnops.c ============================================================================== --- head/sys/fs/fifofs/fifo_vnops.c Tue Jul 31 05:44:03 2012 (r238935) +++ head/sys/fs/fifofs/fifo_vnops.c Tue Jul 31 05:48:35 2012 (r238936) @@ -59,23 +59,13 @@ * Notes about locking: * - fi_pipe is invariant since init time. * - fi_readers and fi_writers are protected by the vnode lock. - * - fi_wgen and fi_seqcount are protected by the pipe mutex. */ struct fifoinfo { struct pipe *fi_pipe; long fi_readers; long fi_writers; - int fi_wgen; - int fi_seqcount; }; -#define FIFO_UPDWGEN(fip, pip) do { \ - if ((fip)->fi_wgen == (fip)->fi_seqcount) \ - (pip)->pipe_state |= PIPE_SAMEWGEN; \ - else \ - (pip)->pipe_state &= ~PIPE_SAMEWGEN; \ -} while (0) - static vop_print_t fifo_print; static vop_open_t fifo_open; static vop_close_t fifo_close; @@ -161,7 +151,7 @@ fifo_open(ap) return (error); fip = malloc(sizeof(*fip), M_VNODE, M_WAITOK); fip->fi_pipe = fpipe; - fip->fi_wgen = fip->fi_readers = fip->fi_writers = 0; + fpipe->pipe_wgen = fip->fi_readers = fip->fi_writers = 0; KASSERT(vp->v_fifoinfo == NULL, ("fifo_open: v_fifoinfo race")); vp->v_fifoinfo = fip; } @@ -181,8 +171,7 @@ fifo_open(ap) if (fip->fi_writers > 0) wakeup(&fip->fi_writers); } - fip->fi_seqcount = fip->fi_wgen - fip->fi_writers; - FIFO_UPDWGEN(fip, fpipe); + fp->f_seqcount = fpipe->pipe_wgen - fip->fi_writers; } if (ap->a_mode & FWRITE) { if ((ap->a_mode & O_NONBLOCK) && fip->fi_readers == 0) { @@ -235,8 +224,7 @@ fifo_open(ap) fpipe->pipe_state |= PIPE_EOF; if (fpipe->pipe_state & PIPE_WANTR) wakeup(fpipe); - fip->fi_wgen++; - FIFO_UPDWGEN(fip, fpipe); + fpipe->pipe_wgen++; PIPE_UNLOCK(fpipe); fifo_cleanup(vp); } @@ -300,8 +288,7 @@ fifo_close(ap) cpipe->pipe_state &= ~PIPE_WANTR; wakeup(cpipe); } - fip->fi_wgen++; - FIFO_UPDWGEN(fip, cpipe); + cpipe->pipe_wgen++; pipeselwakeup(cpipe); PIPE_UNLOCK(cpipe); } Modified: head/sys/kern/sys_pipe.c ============================================================================== --- head/sys/kern/sys_pipe.c Tue Jul 31 05:44:03 2012 (r238935) +++ head/sys/kern/sys_pipe.c Tue Jul 31 05:48:35 2012 (r238936) @@ -1442,7 +1442,7 @@ pipe_poll(fp, events, active_cred, td) levents = events & (POLLIN | POLLINIGNEOF | POLLPRI | POLLRDNORM | POLLRDBAND); if (rpipe->pipe_state & PIPE_NAMED && fp->f_flag & FREAD && levents && - rpipe->pipe_state & PIPE_SAMEWGEN) + fp->f_seqcount == rpipe->pipe_wgen) events |= POLLINIGNEOF; if ((events & POLLINIGNEOF) == 0) { Modified: head/sys/sys/pipe.h ============================================================================== --- head/sys/sys/pipe.h Tue Jul 31 05:44:03 2012 (r238935) +++ head/sys/sys/pipe.h Tue Jul 31 05:48:35 2012 (r238936) @@ -96,7 +96,6 @@ struct pipemapping { #define PIPE_DIRECTW 0x400 /* Pipe direct write active. */ #define PIPE_DIRECTOK 0x800 /* Direct mode ok. */ #define PIPE_NAMED 0x1000 /* Is a named pipe. */ -#define PIPE_SAMEWGEN 0x2000 /* same write generation for named pipes. */ /* * Per-pipe data structure. @@ -115,6 +114,7 @@ struct pipe { u_int pipe_state; /* pipe status info */ int pipe_busy; /* busy flag, mostly to handle rundown sanely */ int pipe_present; /* still present? */ + int pipe_wgen; /* writer generation for named pipe */ ino_t pipe_ino; /* fake inode for stat(2) */ };
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201207310548.q6V5mZHf091624>