diff options
author | alc <alc@FreeBSD.org> | 1999-06-05 03:53:57 +0000 |
---|---|---|
committer | alc <alc@FreeBSD.org> | 1999-06-05 03:53:57 +0000 |
commit | eba3d08af07abb7fff289c7622ac947a1b5f260b (patch) | |
tree | e0315786c00cf909230c5fb9095c0c0c78a9814d /sys/kern/sys_pipe.c | |
parent | 08a158ed9e3b32854b9fbefa35e4a8b559878533 (diff) | |
download | FreeBSD-src-eba3d08af07abb7fff289c7622ac947a1b5f260b.zip FreeBSD-src-eba3d08af07abb7fff289c7622ac947a1b5f260b.tar.gz |
Restructure pipe_read in order to eliminate several race conditions.
Submitted by: Matthew Dillon <dillon@apollo.backplane.com> and myself
Diffstat (limited to 'sys/kern/sys_pipe.c')
-rw-r--r-- | sys/kern/sys_pipe.c | 114 |
1 files changed, 46 insertions, 68 deletions
diff --git a/sys/kern/sys_pipe.c b/sys/kern/sys_pipe.c index cb1cd30..a7266eb 100644 --- a/sys/kern/sys_pipe.c +++ b/sys/kern/sys_pipe.c @@ -16,7 +16,7 @@ * 4. Modifications may be freely made to this file if the above conditions * are met. * - * $Id: sys_pipe.c,v 1.50 1999/02/04 23:50:49 dillon Exp $ + * $Id: sys_pipe.c,v 1.51 1999/04/04 21:41:15 dt Exp $ */ /* @@ -327,11 +327,15 @@ pipe_read(fp, uio, cred, flags) { struct pipe *rpipe = (struct pipe *) fp->f_data; - int error = 0; + int error; int nread = 0; u_int size; ++rpipe->pipe_busy; + error = pipelock(rpipe, 1); + if (error) + goto unlocked_error; + while (uio->uio_resid) { /* * normal pipe buffer receive @@ -342,11 +346,9 @@ pipe_read(fp, uio, cred, flags) size = rpipe->pipe_buffer.cnt; if (size > (u_int) uio->uio_resid) size = (u_int) uio->uio_resid; - if ((error = pipelock(rpipe,1)) == 0) { - error = uiomove( &rpipe->pipe_buffer.buffer[rpipe->pipe_buffer.out], + + error = uiomove(&rpipe->pipe_buffer.buffer[rpipe->pipe_buffer.out], size, uio); - pipeunlock(rpipe); - } if (error) { break; } @@ -355,21 +357,29 @@ pipe_read(fp, uio, cred, flags) rpipe->pipe_buffer.out = 0; rpipe->pipe_buffer.cnt -= size; + + /* + * If there is no more to read in the pipe, reset + * its pointers to the beginning. This improves + * cache hit stats. + */ + if (rpipe->pipe_buffer.cnt == 0) { + rpipe->pipe_buffer.in = 0; + rpipe->pipe_buffer.out = 0; + } nread += size; #ifndef PIPE_NODIRECT /* * Direct copy, bypassing a kernel buffer. */ } else if ((size = rpipe->pipe_map.cnt) && - (rpipe->pipe_state & PIPE_DIRECTW)) { - caddr_t va; + (rpipe->pipe_state & PIPE_DIRECTW)) { + caddr_t va; if (size > (u_int) uio->uio_resid) size = (u_int) uio->uio_resid; - if ((error = pipelock(rpipe,1)) == 0) { - va = (caddr_t) rpipe->pipe_map.kva + rpipe->pipe_map.pos; - error = uiomove(va, size, uio); - pipeunlock(rpipe); - } + + va = (caddr_t) rpipe->pipe_map.kva + rpipe->pipe_map.pos; + error = uiomove(va, size, uio); if (error) break; nread += size; @@ -382,32 +392,6 @@ pipe_read(fp, uio, cred, flags) #endif } else { /* - * If there is no more to read in the pipe, reset - * its pointers to the beginning. This improves - * cache hit stats. - * - * We get this over with now because it may block - * and cause the state to change out from under us, - * rather then have to re-test the state both before - * and after this fragment. - */ - - if ((error = pipelock(rpipe,1)) == 0) { - if (rpipe->pipe_buffer.cnt == 0) { - rpipe->pipe_buffer.in = 0; - rpipe->pipe_buffer.out = 0; - } - pipeunlock(rpipe); - - /* - * If pipe filled up due to pipelock - * blocking, loop back up. - */ - if (rpipe->pipe_buffer.cnt > 0) - continue; - } - - /* * detect EOF condition */ if (rpipe->pipe_state & PIPE_EOF) { @@ -424,55 +408,49 @@ pipe_read(fp, uio, cred, flags) } /* - * break if error (signal via pipelock), or if some - * data was read + * Break if some data was read. */ - if (error || nread > 0) + if (nread > 0) break; /* - * Handle non-blocking mode operation + * Unlock the pipe buffer for our remaining processing. We + * will either break out with an error or we will sleep and + * relock to loop. */ - - if (fp->f_flag & FNONBLOCK) { - error = EAGAIN; - break; - } + pipeunlock(rpipe); /* - * Wait for more data + * Handle non-blocking mode operation or + * wait for more data. */ - - rpipe->pipe_state |= PIPE_WANTR; - if ((error = tsleep(rpipe, PRIBIO|PCATCH, "piperd", 0)) != 0) { - break; + if (fp->f_flag & FNONBLOCK) + error = EAGAIN; + else { + rpipe->pipe_state |= PIPE_WANTR; + if ((error = tsleep(rpipe, PRIBIO|PCATCH, "piperd", 0)) == 0) + error = pipelock(rpipe, 1); } + if (error) + goto unlocked_error; } } + pipeunlock(rpipe); if (error == 0) getnanotime(&rpipe->pipe_atime); - +unlocked_error: --rpipe->pipe_busy; + + /* + * PIPE_WANT processing only makes sense if pipe_busy is 0. + */ if ((rpipe->pipe_busy == 0) && (rpipe->pipe_state & PIPE_WANT)) { rpipe->pipe_state &= ~(PIPE_WANT|PIPE_WANTW); wakeup(rpipe); } else if (rpipe->pipe_buffer.cnt < MINPIPESIZE) { /* - * If there is no more to read in the pipe, reset - * its pointers to the beginning. This improves - * cache hit stats. - */ - if (rpipe->pipe_buffer.cnt == 0) { - if ((error == 0) && (error = pipelock(rpipe,1)) == 0) { - rpipe->pipe_buffer.in = 0; - rpipe->pipe_buffer.out = 0; - pipeunlock(rpipe); - } - } - - /* - * If the "write-side" has been blocked, wake it up now. + * Handle write blocking hysteresis. */ if (rpipe->pipe_state & PIPE_WANTW) { rpipe->pipe_state &= ~PIPE_WANTW; |