svn commit: r367852 - in head/sys: kern sys

[Mateusz Guzik]

Author: mjg
Date: Thu Nov 19 19:25:47 2020
New Revision: 367852
URL: https://svnweb.freebsd.org/changeset/base/367852

Log:
  pipe: thundering herd problem in pipelock
  
  All reads and writes are serialized with a hand-rolled lock, but unlocking it
  always wakes up all waiters. Existing flag fields get resized to make room for
  introduction of waiter counter without growing the struct.
  
  Reviewed by:	kib
  Differential Revision:	https://reviews.freebsd.org/D27273

Modified:
  head/sys/kern/sys_pipe.c
  head/sys/sys/pipe.h

Modified: head/sys/kern/sys_pipe.c
==============================================================================
--- head/sys/kern/sys_pipe.c	Thu Nov 19 19:05:16 2020	(r367851)
+++ head/sys/kern/sys_pipe.c	Thu Nov 19 19:25:47 2020	(r367852)
@@ -622,9 +622,13 @@ pipelock(struct pipe *cpipe, int catch)
 	if (catch)
 		prio |= PCATCH;
 	while (cpipe->pipe_state & PIPE_LOCKFL) {
-		cpipe->pipe_state |= PIPE_LWANT;
+		KASSERT(cpipe->pipe_waiters >= 0,
+		    ("%s: bad waiter count %d", __func__,
+		    cpipe->pipe_waiters));
+		cpipe->pipe_waiters++;
 		error = msleep(cpipe, PIPE_MTX(cpipe),
 		    prio, "pipelk", 0);
+		cpipe->pipe_waiters--;
 		if (error != 0)
 			return (error);
 	}
@@ -642,10 +646,12 @@ pipeunlock(struct pipe *cpipe)
 	PIPE_LOCK_ASSERT(cpipe, MA_OWNED);
 	KASSERT(cpipe->pipe_state & PIPE_LOCKFL,
 		("Unlocked pipe passed to pipeunlock"));
+	KASSERT(cpipe->pipe_waiters >= 0,
+	    ("%s: bad waiter count %d", __func__,
+	    cpipe->pipe_waiters));
 	cpipe->pipe_state &= ~PIPE_LOCKFL;
-	if (cpipe->pipe_state & PIPE_LWANT) {
-		cpipe->pipe_state &= ~PIPE_LWANT;
-		wakeup(cpipe);
+	if (cpipe->pipe_waiters > 0) {
+		wakeup_one(cpipe);
 	}
 }
 

Modified: head/sys/sys/pipe.h
==============================================================================
--- head/sys/sys/pipe.h	Thu Nov 19 19:05:16 2020	(r367851)
+++ head/sys/sys/pipe.h	Thu Nov 19 19:25:47 2020	(r367852)
@@ -116,9 +116,10 @@ struct pipe {
 	struct	pipe *pipe_peer;	/* link with other direction */
 	struct	pipepair *pipe_pair;	/* container structure pointer */
 	u_short	pipe_state;		/* pipe status info */
-	u_short	pipe_type;		/* pipe type info */
+	u_char	pipe_type;		/* pipe type info */
+	u_char	pipe_present;		/* still present? */
+	int	pipe_waiters;		/* pipelock waiters */
 	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) */
 };