svn commit: r360380 - in head: lib/libc/sys sys/kern

[Mark Johnston]

Author: markj
Date: Mon Apr 27 15:59:19 2020
New Revision: 360380
URL: https://svnweb.freebsd.org/changeset/base/360380

Log:
  Fix handling of EV_EOF for named pipes.
  
  Contrary to the kevent man page, EV_EOF on a fifo is not cleared by
  EV_CLEAR.  Modify the read and write filters to clear EV_EOF when the
  fifo's PIPE_EOF flag is clear, and update the man page to document the
  new behaviour.
  
  Modify the write filter to return the amount of buffer space available
  even if no readers are present.  This matches the behaviour for sockets.
  
  When reading from a pipe, only call pipeselwakeup() if some data was
  actually read.  This prevents the continuous re-triggering of a
  EVFILT_READ event on EOF when in edge-triggered mode.
  
  PR:		203366, 224615
  Submitted by:	Jan Kokemüller <jan.kokemueller at gmail.com>
  MFC after:	2 weeks
  Differential Revision:	https://reviews.freebsd.org/D24528

Modified:
  head/lib/libc/sys/kqueue.2
  head/sys/kern/sys_pipe.c

Modified: head/lib/libc/sys/kqueue.2
==============================================================================
--- head/lib/libc/sys/kqueue.2	Mon Apr 27 15:59:07 2020	(r360379)
+++ head/lib/libc/sys/kqueue.2	Mon Apr 27 15:59:19 2020	(r360380)
@@ -24,7 +24,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd April 21, 2020
+.Dd April 27, 2020
 .Dt KQUEUE 2
 .Os
 .Sh NAME
@@ -323,8 +323,7 @@ When the last writer disconnects, the filter will set
 .Dv EV_EOF
 in
 .Va flags .
-This may be cleared by passing in
-.Dv EV_CLEAR ,
+This will be cleared by the filter when a new writer connects,
 at which point the
 filter will resume waiting for data to become available before
 returning.
@@ -343,9 +342,10 @@ For sockets, pipes
 and fifos,
 .Va data
 will contain the amount of space remaining in the write buffer.
-The filter will set EV_EOF when the reader disconnects, and for
-the fifo case, this may be cleared by use of
-.Dv EV_CLEAR .
+The filter will set
+.Dv EV_EOF
+when the reader disconnects, and for the fifo case, this will be cleared
+when a new reader connects.
 Note that this filter is not supported for vnodes or BPF devices.
 .Pp
 For sockets, the low water mark and socket error handling is

Modified: head/sys/kern/sys_pipe.c
==============================================================================
--- head/sys/kern/sys_pipe.c	Mon Apr 27 15:59:07 2020	(r360379)
+++ head/sys/kern/sys_pipe.c	Mon Apr 27 15:59:19 2020	(r360380)
@@ -824,7 +824,12 @@ unlocked_error:
 		}
 	}
 
-	if ((rpipe->pipe_buffer.size - rpipe->pipe_buffer.cnt) >= PIPE_BUF)
+	/*
+	 * Only wake up writers if there was actually something read.
+	 * Otherwise, when calling read(2) at EOF, a spurious wakeup occurs.
+	 */
+	if (nread > 0 &&
+	    rpipe->pipe_buffer.size - rpipe->pipe_buffer.cnt >= PIPE_BUF)
 		pipeselwakeup(rpipe);
 
 	PIPE_UNLOCK(rpipe);
@@ -1726,48 +1731,54 @@ filt_pipedetach(struct knote *kn)
 static int
 filt_piperead(struct knote *kn, long hint)
 {
+	struct file *fp = kn->kn_fp;
 	struct pipe *rpipe = kn->kn_hook;
-	struct pipe *wpipe = rpipe->pipe_peer;
-	int ret;
 
 	PIPE_LOCK_ASSERT(rpipe, MA_OWNED);
 	kn->kn_data = rpipe->pipe_buffer.cnt;
 	if (kn->kn_data == 0)
 		kn->kn_data = rpipe->pipe_map.cnt;
 
-	if ((rpipe->pipe_state & PIPE_EOF) ||
-	    wpipe->pipe_present != PIPE_ACTIVE ||
-	    (wpipe->pipe_state & PIPE_EOF)) {
+	if ((rpipe->pipe_state & PIPE_EOF) != 0 &&
+	    ((rpipe->pipe_state & PIPE_NAMED) == 0 ||
+	    fp->f_pipegen != rpipe->pipe_wgen)) {
 		kn->kn_flags |= EV_EOF;
 		return (1);
 	}
-	ret = kn->kn_data > 0;
-	return ret;
+	kn->kn_flags &= ~EV_EOF;
+	return (kn->kn_data > 0);
 }
 
 /*ARGSUSED*/
 static int
 filt_pipewrite(struct knote *kn, long hint)
 {
-	struct pipe *wpipe;
+	struct pipe *wpipe = kn->kn_hook;
 
 	/*
 	 * If this end of the pipe is closed, the knote was removed from the
 	 * knlist and the list lock (i.e., the pipe lock) is therefore not held.
 	 */
-	wpipe = kn->kn_hook;
+	if (wpipe->pipe_present == PIPE_ACTIVE ||
+	    (wpipe->pipe_state & PIPE_NAMED) != 0) {
+		PIPE_LOCK_ASSERT(wpipe, MA_OWNED);
+
+		if (wpipe->pipe_state & PIPE_DIRECTW) {
+			kn->kn_data = 0;
+		} else if (wpipe->pipe_buffer.size > 0) {
+			kn->kn_data = wpipe->pipe_buffer.size -
+			    wpipe->pipe_buffer.cnt;
+		} else {
+			kn->kn_data = PIPE_BUF;
+		}
+	}
+
 	if (wpipe->pipe_present != PIPE_ACTIVE ||
 	    (wpipe->pipe_state & PIPE_EOF)) {
-		kn->kn_data = 0;
 		kn->kn_flags |= EV_EOF;
 		return (1);
 	}
-	PIPE_LOCK_ASSERT(wpipe, MA_OWNED);
-	kn->kn_data = (wpipe->pipe_buffer.size > 0) ?
-	    (wpipe->pipe_buffer.size - wpipe->pipe_buffer.cnt) : PIPE_BUF;
-	if (wpipe->pipe_state & PIPE_DIRECTW)
-		kn->kn_data = 0;
-
+	kn->kn_flags &= ~EV_EOF;
 	return (kn->kn_data >= PIPE_BUF);
 }