thundering pipe herds

[David Holland <dholland-tech%netbsd.org@localhost>]

mjg at freebsd says that their pipes have a thundering herd problem
that manifests in make's token pipe stuff at high -j values. We have
the same logic, and it's easily fixed.

Completely untested patch follows; does anyone object to this assuming
that it works (possibly after minor changes)?

(pipe_waiters is inserted where it is so it occupies what's currently
struct padding on 64-bit machines)



Index: sys/pipe.h
===================================================================
RCS file: /cvsroot/src/sys/sys/pipe.h,v
retrieving revision 1.36
diff -u -p -r1.36 pipe.h
--- sys/pipe.h	22 Aug 2018 01:05:24 -0000	1.36
+++ sys/pipe.h	23 Nov 2020 21:23:07 -0000
@@ -93,8 +93,7 @@ struct pipemapping {
 #define PIPE_DIRECTR	0x080	/* Pipe direct read request (setup complete) */
 #define	PIPE_LOCKFL	0x100	/* Process has exclusive access to
 				   pointers/data. */
-#define	PIPE_LWANT	0x200	/* Process wants exclusive access to
-				   pointers/data. */
+/*	unused  	0x200	*/
 #define	PIPE_RESTART	0x400	/* Return ERESTART to blocked syscalls */
 
 /*
@@ -114,6 +113,7 @@ struct pipe {
 	struct	timespec pipe_mtime;	/* time of last modify */
 	struct	timespec pipe_btime;	/* time of creation */
 	pid_t	pipe_pgid;		/* process group for sigio */
+	u_int	pipe_waiters;		/* number of waiters pending */
 	struct	pipe *pipe_peer;	/* link with other direction */
 	u_int	pipe_state;		/* pipe status info */
 	int	pipe_busy;		/* busy flag, to handle rundown */
Index: kern/sys_pipe.c
===================================================================
RCS file: /cvsroot/src/sys/kern/sys_pipe.c,v
retrieving revision 1.148
diff -u -p -r1.148 sys_pipe.c
--- kern/sys_pipe.c	26 Apr 2019 17:24:23 -0000	1.148
+++ kern/sys_pipe.c	23 Nov 2020 21:23:07 -0000
@@ -371,13 +371,19 @@ pipelock(struct pipe *pipe, bool catch_p
 	KASSERT(mutex_owned(pipe->pipe_lock));
 
 	while (pipe->pipe_state & PIPE_LOCKFL) {
-		pipe->pipe_state |= PIPE_LWANT;
+		pipe->pipe_waiters++;
+		KASSERT(pipe->pipe_waiters != 0); /* just in case */
 		if (catch_p) {
 			error = cv_wait_sig(&pipe->pipe_lkcv, pipe->pipe_lock);
-			if (error != 0)
+			if (error != 0) {
+				KASSERT(pipe->pipe_waiters > 0);
+				pipe->pipe_waiters--;
 				return error;
+			}
 		} else
 			cv_wait(&pipe->pipe_lkcv, pipe->pipe_lock);
+		KASSERT(pipe->pipe_waiters > 0);
+		pipe->pipe_waiters--;
 	}
 
 	pipe->pipe_state |= PIPE_LOCKFL;
@@ -395,9 +401,8 @@ pipeunlock(struct pipe *pipe)
 	KASSERT(pipe->pipe_state & PIPE_LOCKFL);
 
 	pipe->pipe_state &= ~PIPE_LOCKFL;
-	if (pipe->pipe_state & PIPE_LWANT) {
-		pipe->pipe_state &= ~PIPE_LWANT;
-		cv_broadcast(&pipe->pipe_lkcv);
+	if (pipe->pipe_waiters > 0) {
+		cv_signal(&pipe->pipe_lkcv);
 	}
 }
 


-- 
David A. Holland
dholland%netbsd.org@localhost