remove PIPE_WANTW, PIPE_WANTR and PIPE_WANTCLOSE. cv_waiters is enough.

this fixes a deadlock between pipe_direct_write and pipeclose.

XXX this code should be simplified.
it's mostly pointless to have two struct pipes linked together,
esp. when we don't support bi-directional pipes.
This commit is contained in:
yamt 2008-01-02 19:16:00 +00:00
parent 4f69d01d2f
commit c68e4b1c69
2 changed files with 11 additions and 59 deletions

View File

@ -1,4 +1,4 @@
/* $NetBSD: sys_pipe.c,v 1.92 2007/12/28 13:11:16 ad Exp $ */ /* $NetBSD: sys_pipe.c,v 1.93 2008/01/02 19:16:01 yamt Exp $ */
/*- /*-
* Copyright (c) 2003, 2007 The NetBSD Foundation, Inc. * Copyright (c) 2003, 2007 The NetBSD Foundation, Inc.
@ -83,7 +83,7 @@
*/ */
#include <sys/cdefs.h> #include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: sys_pipe.c,v 1.92 2007/12/28 13:11:16 ad Exp $"); __KERNEL_RCSID(0, "$NetBSD: sys_pipe.c,v 1.93 2008/01/02 19:16:01 yamt Exp $");
#include <sys/param.h> #include <sys/param.h>
#include <sys/systm.h> #include <sys/systm.h>
@ -583,13 +583,9 @@ again:
/* /*
* If the "write-side" is blocked, wake it up now. * If the "write-side" is blocked, wake it up now.
*/ */
if (rpipe->pipe_state & PIPE_WANTW) { cv_broadcast(&rpipe->pipe_cv);
rpipe->pipe_state &= ~PIPE_WANTW;
cv_broadcast(&rpipe->pipe_cv);
}
/* Now wait until the pipe is filled */ /* Now wait until the pipe is filled */
rpipe->pipe_state |= PIPE_WANTR;
error = cv_wait_sig(&rpipe->pipe_cv, rpipe->pipe_lock); error = cv_wait_sig(&rpipe->pipe_cv, rpipe->pipe_lock);
if (error != 0) if (error != 0)
goto unlocked_error; goto unlocked_error;
@ -602,21 +598,8 @@ again:
unlocked_error: unlocked_error:
--rpipe->pipe_busy; --rpipe->pipe_busy;
if (rpipe->pipe_busy == 0 || bp->cnt < MINPIPESIZE) {
/*
* PIPE_WANTCLOSE processing only makes sense if pipe_busy is 0.
*/
if ((rpipe->pipe_busy == 0) && (rpipe->pipe_state & PIPE_WANTCLOSE)) {
rpipe->pipe_state &= ~(PIPE_WANTCLOSE|PIPE_WANTW);
cv_broadcast(&rpipe->pipe_cv); cv_broadcast(&rpipe->pipe_cv);
} else if (bp->cnt < MINPIPESIZE) {
/*
* Handle write blocking hysteresis.
*/
if (rpipe->pipe_state & PIPE_WANTW) {
rpipe->pipe_state &= ~PIPE_WANTW;
cv_broadcast(&rpipe->pipe_cv);
}
} }
/* /*
@ -762,12 +745,7 @@ pipe_direct_write(struct file *fp, struct pipe *wpipe, struct uio *uio)
pipeunlock(wpipe); pipeunlock(wpipe);
while (error == 0 && wpipe->pipe_buffer.cnt > 0) { while (error == 0 && wpipe->pipe_buffer.cnt > 0) {
if (wpipe->pipe_state & PIPE_WANTR) { cv_broadcast(&wpipe->pipe_cv);
wpipe->pipe_state &= ~PIPE_WANTR;
cv_broadcast(&wpipe->pipe_cv);
}
wpipe->pipe_state |= PIPE_WANTW;
error = cv_wait_sig(&wpipe->pipe_cv, wpipe->pipe_lock); error = cv_wait_sig(&wpipe->pipe_cv, wpipe->pipe_lock);
if (error == 0 && wpipe->pipe_state & PIPE_EOF) if (error == 0 && wpipe->pipe_state & PIPE_EOF)
error = EPIPE; error = EPIPE;
@ -778,10 +756,7 @@ pipe_direct_write(struct file *fp, struct pipe *wpipe, struct uio *uio)
/* Wait until the reader is done */ /* Wait until the reader is done */
while (error == 0 && (wpipe->pipe_state & PIPE_DIRECTR)) { while (error == 0 && (wpipe->pipe_state & PIPE_DIRECTR)) {
if (wpipe->pipe_state & PIPE_WANTR) { cv_broadcast(&wpipe->pipe_cv);
wpipe->pipe_state &= ~PIPE_WANTR;
cv_broadcast(&wpipe->pipe_cv);
}
pipeselwakeup(wpipe, wpipe, POLL_IN); pipeselwakeup(wpipe, wpipe, POLL_IN);
error = cv_wait_sig(&wpipe->pipe_cv, wpipe->pipe_lock); error = cv_wait_sig(&wpipe->pipe_cv, wpipe->pipe_lock);
if (error == 0 && wpipe->pipe_state & PIPE_EOF) if (error == 0 && wpipe->pipe_state & PIPE_EOF)
@ -865,9 +840,7 @@ pipe_write(struct file *fp, off_t *offset, struct uio *uio, kauth_cred_t cred,
/* Aquire the long-term pipe lock */ /* Aquire the long-term pipe lock */
if ((error = pipelock(wpipe,1)) != 0) { if ((error = pipelock(wpipe,1)) != 0) {
--wpipe->pipe_busy; --wpipe->pipe_busy;
if (wpipe->pipe_busy == 0 if (wpipe->pipe_busy == 0) {
&& (wpipe->pipe_state & PIPE_WANTCLOSE)) {
wpipe->pipe_state &= ~(PIPE_WANTCLOSE | PIPE_WANTR);
cv_broadcast(&wpipe->pipe_cv); cv_broadcast(&wpipe->pipe_cv);
} }
mutex_exit(rpipe->pipe_lock); mutex_exit(rpipe->pipe_lock);
@ -903,10 +876,7 @@ pipe_write(struct file *fp, off_t *offset, struct uio *uio, kauth_cred_t cred,
* We break out if a signal occurs or the reader goes away. * We break out if a signal occurs or the reader goes away.
*/ */
while (error == 0 && wpipe->pipe_state & PIPE_DIRECTW) { while (error == 0 && wpipe->pipe_state & PIPE_DIRECTW) {
if (wpipe->pipe_state & PIPE_WANTR) { cv_broadcast(&wpipe->pipe_cv);
wpipe->pipe_state &= ~PIPE_WANTR;
cv_broadcast(&wpipe->pipe_cv);
}
pipeunlock(wpipe); pipeunlock(wpipe);
error = cv_wait_sig(&wpipe->pipe_cv, error = cv_wait_sig(&wpipe->pipe_cv,
wpipe->pipe_lock); wpipe->pipe_lock);
@ -1016,10 +986,7 @@ pipe_write(struct file *fp, off_t *offset, struct uio *uio, kauth_cred_t cred,
/* /*
* If the "read-side" has been blocked, wake it up now. * If the "read-side" has been blocked, wake it up now.
*/ */
if (wpipe->pipe_state & PIPE_WANTR) { cv_broadcast(&wpipe->pipe_cv);
wpipe->pipe_state &= ~PIPE_WANTR;
cv_broadcast(&wpipe->pipe_cv);
}
/* /*
* don't block on non-blocking I/O * don't block on non-blocking I/O
@ -1037,7 +1004,6 @@ pipe_write(struct file *fp, off_t *offset, struct uio *uio, kauth_cred_t cred,
pipeselwakeup(wpipe, wpipe, POLL_OUT); pipeselwakeup(wpipe, wpipe, POLL_OUT);
pipeunlock(wpipe); pipeunlock(wpipe);
wpipe->pipe_state |= PIPE_WANTW;
error = cv_wait_sig(&wpipe->pipe_cv, wpipe->pipe_lock); error = cv_wait_sig(&wpipe->pipe_cv, wpipe->pipe_lock);
(void)pipelock(wpipe, 0); (void)pipelock(wpipe, 0);
if (error != 0) if (error != 0)
@ -1054,18 +1020,8 @@ pipe_write(struct file *fp, off_t *offset, struct uio *uio, kauth_cred_t cred,
} }
--wpipe->pipe_busy; --wpipe->pipe_busy;
if ((wpipe->pipe_busy == 0) && (wpipe->pipe_state & PIPE_WANTCLOSE)) { if (wpipe->pipe_busy == 0 || bp->cnt > 0) {
wpipe->pipe_state &= ~(PIPE_WANTCLOSE | PIPE_WANTR);
cv_broadcast(&wpipe->pipe_cv); cv_broadcast(&wpipe->pipe_cv);
} else if (bp->cnt > 0) {
/*
* If we have put any characters in the buffer, we wake up
* the reader.
*/
if (wpipe->pipe_state & PIPE_WANTR) {
wpipe->pipe_state &= ~PIPE_WANTR;
cv_broadcast(&wpipe->pipe_cv);
}
} }
/* /*
@ -1308,7 +1264,6 @@ pipeclose(struct file *fp, struct pipe *pipe)
if (pipe->pipe_busy) { if (pipe->pipe_busy) {
while (pipe->pipe_busy) { while (pipe->pipe_busy) {
cv_broadcast(&pipe->pipe_cv); cv_broadcast(&pipe->pipe_cv);
pipe->pipe_state |= PIPE_WANTCLOSE;
cv_wait_sig(&pipe->pipe_cv, pipe->pipe_lock); cv_wait_sig(&pipe->pipe_cv, pipe->pipe_lock);
} }
} }

View File

@ -1,4 +1,4 @@
/* $NetBSD: pipe.h,v 1.22 2007/12/26 16:01:38 ad Exp $ */ /* $NetBSD: pipe.h,v 1.23 2008/01/02 19:16:00 yamt Exp $ */
/* /*
* Copyright (c) 1996 John S. Dyson * Copyright (c) 1996 John S. Dyson
@ -87,9 +87,6 @@ struct pipemapping {
* Bits in pipe_state. * Bits in pipe_state.
*/ */
#define PIPE_ASYNC 0x001 /* Async I/O */ #define PIPE_ASYNC 0x001 /* Async I/O */
#define PIPE_WANTR 0x002 /* Reader wants some characters */
#define PIPE_WANTW 0x004 /* Writer wants space to put characters */
#define PIPE_WANTCLOSE 0x008 /* Pipe is wanted to be run-down */
#define PIPE_EOF 0x010 /* Pipe is in EOF condition */ #define PIPE_EOF 0x010 /* Pipe is in EOF condition */
#define PIPE_SIGNALR 0x020 /* Do selwakeup() on read(2) */ #define PIPE_SIGNALR 0x020 /* Do selwakeup() on read(2) */
#define PIPE_DIRECTW 0x040 /* Pipe in direct write mode setup */ #define PIPE_DIRECTW 0x040 /* Pipe in direct write mode setup */