PERFORCE change 85342 for review

[John Baldwin]

On Saturday 15 October 2005 11:46 am, soc-chenk wrote:
> http://perforce.freebsd.org/chv.cgi?CH=85342
>
> Change 85342 by soc-chenk at soc-chenk_leavemealone on 2005/10/15 15:45:35
>
> 	device read converted to use msleep instead of condvar
> 	[this seems to prevent daemon-segfaults-in-readdir panics]
> 	Submitted by:	soc-chenk

Considering that cv_foo and msleep are both just slim wrappers around 
sleepq_*(), I can't see how this diff would make any difference, unless 
adding the 'goto again;' is what fixed the bug, in which case you could have 
kept the cv(9) usage.

>
> Affected files ...
>
> .. //depot/projects/soc2005/fuse4bsd2/fuse_module/fuse.c#18 edit
> .. //depot/projects/soc2005/fuse4bsd2/fuse_module/fuse.h#9 edit
>
> Differences ...
>
> ==== //depot/projects/soc2005/fuse4bsd2/fuse_module/fuse.c#18 (text+ko)
> ====
>
> @@ -478,7 +478,6 @@
>
>  	/* Setting up fields of mine */
>  	mtx_init(&data->msg_mtx, "mutex for fuse message list", NULL, MTX_DEF);
> -	cv_init(&data->msg_cv, "cv to wake up fusedev_read");
>  	STAILQ_INIT(&data->fmsg_head);
>  	mtx_init(&data->ticket_mtx, "mutex for the fuse ticketer", NULL,
>  	         MTX_DEF);
> @@ -503,7 +502,6 @@
>
>  	/* Driving off stage all that stuff thrown at device... */
>  	mtx_destroy(&data->msg_mtx);
> -	cv_destroy(&data->msg_cv);
>  	mtx_destroy(&data->callback_mtx);
>  	mtx_destroy(&data->ticket_mtx);
>
> @@ -526,7 +524,7 @@
>  	DEBUG2G("banning daemon\n");
>  	mtx_lock(&data->msg_mtx);
>  	data->dataflag |= FDAT_KICK;
> -	cv_signal(&data->msg_cv);
> +	wakeup_one(data);
>  	mtx_unlock(&data->msg_mtx);
>  }
>
> @@ -693,7 +691,7 @@
>  	mtx_lock(&tick->data->msg_mtx);
>  	fuse_msgn_push(&tick->msgn);
>  	DEBUG("ring the bell\n");
> -	cv_signal(&tick->data->msg_cv);
> +	wakeup_one(tick->data);
>  	mtx_unlock(&tick->data->msg_mtx);
>  }
>
> @@ -1077,6 +1075,7 @@
>  	fuprintf("fuse device being read on thread %d\n", uio->uio_td->td_tid);
>
>  	mtx_lock(&data->msg_mtx);
> +again:
>  	if (fdata_kick_get(data)) {
>  		DEBUG("we know early on that reader should be kicked so we don't wait
> for news\n"); mtx_unlock(&data->msg_mtx);
> @@ -1084,13 +1083,23 @@
>  	}
>
>  	if ( ! (fmsgn = fdata_pop_msg(data))) {
> -		err = cv_wait_sig(&data->msg_cv, &data->msg_mtx);
> +		err = msleep(data, &data->msg_mtx, PCATCH, "fu msg", 0);
>  		if (err != 0) {
>  			mtx_unlock(&data->msg_mtx);
>  			return (fdata_kick_get(data) ? ENODEV : err);
>  		}
>  		fmsgn = fdata_pop_msg(data);
>  	}
> +	if (! fmsgn) {
> +		/*
> +		 * We can get here if fuse daemon suddenly terminates,
> +		 * eg, by being hit by a SIGKILL
> +		 * -- and some other cases, too, tho not totally clear, when
> +		 * (cv_signal/wakeup_one signals the whole process ?)
> +		 */
> +		DEBUG("no message on thread #%d\n", uio->uio_td->td_tid);
> +		goto again;
> +	}
>  	mtx_unlock(&data->msg_mtx);
>
>  	if (fdata_kick_get(data)) {
> @@ -1105,16 +1114,6 @@
>  		}
>  		return (ENODEV); /* This should make the daemon get off of us */
>  	}
> -	if (! fmsgn) {
> -		/*
> -		 * We can get here if fuse daemon suddenly terminates,
> -		 * eg, by being hit by a SIGKILL
> -		 * -- and some other cases, too, tho not totally clear, when
> -		 * (cv_signal signals the whole process ?)
> -		 */
> -		DEBUG("no message on thread #%d\n", uio->uio_td->td_tid);
> -		return (EINTR);
> -	}
>  	DEBUG("message got on thread #%d\n", uio->uio_td->td_tid);
>
>  	KASSERT(fmsgn->msg_bufdata || fmsgn->msg_bufsize == 0,
>
> ==== //depot/projects/soc2005/fuse4bsd2/fuse_module/fuse.h#9 (text+ko) ====
>
> @@ -90,7 +90,6 @@
>
>  struct fuse_data {
>  	struct mtx msg_mtx;
> -	struct cv msg_cv;
>  	STAILQ_HEAD(, fuse_msg_node) fmsg_head;
>
>  	struct mtx callback_mtx;

-- 
John Baldwin <jhb at FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org