Date: Thu, 04 Feb 1999 16:30:11 -0800 From: Julian Elischer <julian@whistle.com> To: Matthew Dillon <dillon@apollo.backplane.com> Cc: current@FreeBSD.ORG Subject: Re: Bug in piperd Message-ID: <36BA3B93.2781E494@whistle.com> References: <199902042219.OAA90785@apollo.backplane.com>
next in thread | previous in thread | raw e-mail | index | archive | help
I've been seeng lockups in 3.0 where as is in piperd,
but the stack trace has always looked as if the problem was in soft
updates or the syncer daemon..
Matthew Dillon wrote:
>
> Ha. I've been slowly reducing MAXMEM on my test box to force it to
> swap more heavily running buildworld and found a race in the
> kern/sys_pipe.c module.
>
> My buildworld froze up... i.e. just stopped running. Everything else
> on the box was fine. ps showed an 'as' command stuck in 'piperd'.
>
> The problem is simple (pseudo code fragment):
>
> pipe read:
> ...
> * check for EOF
> * check for waiting writers
> * lock the pipe
> * check for waiting writers
> * check for non-blocking I/O
> * sleep in 'piperd'
>
> The problem, of course, is that if 'lock the pipe' blocks, it is possible
> for the writer side to close the pipe. Since EOF is not checked for
> after the pipe has been locked, the reader enters a 'piperd' state
> and never gets woken up again.
>
> I am testing a fix now and will then commit it.
>
> If anyone sees anything obviously wrong with this patch, please email me.
>
> -Matt
> Matthew Dillon
> <dillon@backplane.com>
>
> Index: sys_pipe.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/kern/sys_pipe.c,v
> retrieving revision 1.49
> diff -u -r1.49 sys_pipe.c
> --- sys_pipe.c 1999/01/28 00:57:47 1.49
> +++ sys_pipe.c 1999/02/04 22:14:58
> @@ -381,12 +381,32 @@
> #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);
> + }
> +
> + /*
> * detect EOF condition
> */
> if (rpipe->pipe_state & PIPE_EOF) {
> /* XXX error = ? */
> break;
> }
> +
> /*
> * If the "write-side" has been blocked, wake it up now.
> */
> @@ -394,34 +414,26 @@
> rpipe->pipe_state &= ~PIPE_WANTW;
> wakeup(rpipe);
> }
> - if (nread > 0)
> +
> + /*
> + * break if error (signal via pipelock), or if some
> + * data was read
> + */
> + if (error || nread > 0)
> break;
>
> + /*
> + * Handle non-blocking mode operation
> + */
> +
> if (fp->f_flag & FNONBLOCK) {
> error = EAGAIN;
> break;
> }
>
> /*
> - * If there is no more to read in the pipe, reset
> - * its pointers to the beginning. This improves
> - * cache hit stats.
> + * Wait for more data
> */
> -
> - if ((error = pipelock(rpipe,1)) == 0) {
> - if (rpipe->pipe_buffer.cnt == 0) {
> - rpipe->pipe_buffer.in = 0;
> - rpipe->pipe_buffer.out = 0;
> - }
> - pipeunlock(rpipe);
> - } else {
> - break;
> - }
> -
> - if (rpipe->pipe_state & PIPE_WANTW) {
> - rpipe->pipe_state &= ~PIPE_WANTW;
> - wakeup(rpipe);
> - }
>
> rpipe->pipe_state |= PIPE_WANTR;
> if ((error = tsleep(rpipe, PRIBIO|PCATCH, "piperd", 0)) != 0) {
>
> To Unsubscribe: send mail to majordomo@FreeBSD.org
> with "unsubscribe freebsd-current" in the body of the message
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?36BA3B93.2781E494>