Attachment 213659 Details for Bug 224615 – Updated FIFO/pipe fixes

[patch] Updated FIFO/pipe fixes

fifo-fixes.patch (text/plain), 6.14 KB, created by Jan Kokemüller on 2020-04-22 01:44:44 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jan Kokemüller

Created: 2020-04-22 01:44:44 UTC

Size: 6.14 KB

patch

obsolete

>commit e59ab0e9d88f705f41c780f6b7907beade0b5cf4
>Author: Jan KokemÃ¼ller <jan.kokemueller@gmail.com>
>Date:   Tue Dec 19 17:24:00 2017 +0100
>
>    fix some FIFO/pipe issues
>    
>    - fix PR 203366
>    - fix PR 224615 (EV_EOF on named pipe)
>    - wake up FIFO readers/writers when a new reader/writer connects
>    - only show POLLIN/POLLRDNORM for FIFO readers, not writers
>    - clear EV_EOF flag if FIFO EOF condition is cleared after a new reader/writer connected
>
>diff --git a/sys/fs/fifofs/fifo_vnops.c b/sys/fs/fifofs/fifo_vnops.c
>index b4fbc7a5e595..d511747b3e58 100644
>--- a/sys/fs/fifofs/fifo_vnops.c
>+++ b/sys/fs/fifofs/fifo_vnops.c
>@@ -174,8 +174,10 @@ fifo_open(ap)
> 		fip->fi_rgen++;
> 		if (fip->fi_readers == 1) {
> 			fpipe->pipe_state &= ~PIPE_EOF;
>-			if (fip->fi_writers > 0)
>+			if (fip->fi_writers > 0) {
> 				wakeup(&fip->fi_writers);
>+				pipeselwakeup(fpipe);
>+			}
> 		}
> 		fp->f_pipegen = fpipe->pipe_wgen - fip->fi_writers;
> 	}
>@@ -190,8 +192,10 @@ fifo_open(ap)
> 		fip->fi_wgen++;
> 		if (fip->fi_writers == 1) {
> 			fpipe->pipe_state &= ~PIPE_EOF;
>-			if (fip->fi_readers > 0)
>+			if (fip->fi_readers > 0) {
> 				wakeup(&fip->fi_readers);
>+				pipeselwakeup(fpipe);
>+			}
> 		}
> 	}
> 	if ((ap->a_mode & O_NONBLOCK) == 0) {
>@@ -210,6 +214,7 @@ fifo_open(ap)
> 					fpipe->pipe_state |= PIPE_EOF;
> 					if (fpipe->pipe_state & PIPE_WANTW)
> 						wakeup(fpipe);
>+					pipeselwakeup(fpipe);
> 					PIPE_UNLOCK(fpipe);
> 					fifo_cleanup(vp);
> 				}
>@@ -238,6 +243,7 @@ fifo_open(ap)
> 					if (fpipe->pipe_state & PIPE_WANTR)
> 						wakeup(fpipe);
> 					fpipe->pipe_wgen++;
>+					pipeselwakeup(fpipe);
> 					PIPE_UNLOCK(fpipe);
> 					fifo_cleanup(vp);
> 				}
>diff --git a/sys/kern/sys_pipe.c b/sys/kern/sys_pipe.c
>index 62dfa65c1dd8..19cab545d975 100644
>--- a/sys/kern/sys_pipe.c
>+++ b/sys/kern/sys_pipe.c
>@@ -824,7 +824,13 @@ pipe_read(struct file *fp, struct uio *uio, struct ucred *active_cred,
> 		}
> 	}
> 
>-	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
>+	 * that might lead to issues like PR 203366.
>+	 */
>+	if (nread > 0 &&
>+	    (rpipe->pipe_buffer.size - rpipe->pipe_buffer.cnt) >= PIPE_BUF)
> 		pipeselwakeup(rpipe);
> 
> 	PIPE_UNLOCK(rpipe);
>@@ -1430,7 +1436,8 @@ pipe_poll(struct file *fp, int events, struct ucred *active_cred,
> 
> 	if ((events & POLLINIGNEOF) == 0) {
> 		if (rpipe->pipe_state & PIPE_EOF) {
>-			revents |= (events & (POLLIN | POLLRDNORM));
>+			if (fp->f_flag & FREAD)
>+				revents |= (events & (POLLIN | POLLRDNORM));
> 			if (wpipe->pipe_present != PIPE_ACTIVE ||
> 			    (wpipe->pipe_state & PIPE_EOF))
> 				revents |= POLLHUP;
>@@ -1605,8 +1612,6 @@ pipeclose(struct pipe *cpipe)
> 	pipelock(cpipe, 0);
> 	pp = cpipe->pipe_pair;
> 
>-	pipeselwakeup(cpipe);
>-
> 	/*
> 	 * If the other side is blocked, wake it up saying that
> 	 * we want to close it down.
>@@ -1620,16 +1625,16 @@ pipeclose(struct pipe *cpipe)
> 		pipelock(cpipe, 0);
> 	}
> 
>+	pipeselwakeup(cpipe);
>+
> 	/*
> 	 * Disconnect from peer, if any.
> 	 */
> 	ppipe = cpipe->pipe_peer;
> 	if (ppipe->pipe_present == PIPE_ACTIVE) {
>-		pipeselwakeup(ppipe);
>-
> 		ppipe->pipe_state |= PIPE_EOF;
> 		wakeup(ppipe);
>-		KNOTE_LOCKED(&ppipe->pipe_sel.si_note, 0);
>+		pipeselwakeup(ppipe);
> 	}
> 
> 	/*
>@@ -1727,23 +1732,22 @@ 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) &&
>+	    !((rpipe->pipe_state & PIPE_NAMED) &&
>+	        (fp->f_seqcount == 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*/
>@@ -1757,18 +1761,25 @@ filt_pipewrite(struct knote *kn, long hint)
> 	 * knlist and the list lock (i.e., the pipe lock) is therefore not held.
> 	 */
> 	wpipe = kn->kn_hook;
>+
>+	if (wpipe->pipe_present == PIPE_ACTIVE) {
>+		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);
> }
> 
>diff --git a/tools/regression/poll/pipepoll.c b/tools/regression/poll/pipepoll.c
>index 5ca064bf6bf3..43bf03659330 100644
>--- a/tools/regression/poll/pipepoll.c
>+++ b/tools/regression/poll/pipepoll.c
>@@ -1,5 +1,6 @@
> /* $FreeBSD$ */
> 
>+#include <sys/event.h>
> #include <sys/poll.h>
> #include <sys/socket.h>
> #include <sys/stat.h>
>@@ -202,6 +203,31 @@ child(int fd, int num)
> 		if ((res = poll(&pfd, 1, 0)) < 0)
> 			err(1, "poll");
> 		report(num++, "6b", POLLHUP, pfd.revents, res, 1);
>+
>+		int kq = kqueue();
>+
>+		struct kevent kev;
>+		EV_SET(&kev, fd2, EVFILT_READ, EV_ADD, 0, 0, 0);
>+		if (kevent(kq, &kev, 1, NULL, 0, NULL) < 0) {
>+			err(1, "kevent");
>+		}
>+
>+		int n;
>+		struct timespec ts = { 0, 0 };
>+		if ((n = kevent(kq, NULL, 0, &kev, 1, &ts)) < 0) {
>+			err(1, "kevent");
>+		}
>+		for (int i = 0; i < n; ++i) {
>+			fprintf(stderr, "got event: %d\n", (int)kev.filter);
>+			fprintf(stderr, " fd: %d\n", (int)kev.ident);
>+			fprintf(stderr, " data: %d\n", (int)kev.data);
>+			fprintf(stderr, " flags: %x\n", (unsigned)kev.flags);
>+			fprintf(stderr, " fflags: %x\n", (unsigned)kev.fflags);
>+		}
>+		if (n == 0) {
>+			fprintf(stderr, "got no events\n");
>+		}
>+
> 		pfd.fd = fd;
> 		if ((res = poll(&pfd, 1, 0)) < 0)
> 			err(1, "poll");

Actions: View | Diff

Attachments on bug 224615: 189130 | 189131 | 213659 | 213662