[master] 95aeadd Don't hold waiters mutex'en over Waiter_Call() since they are also held over Wait_Enter() processing, and that makes it impossible for the code using the waiter to avoid one deadlock or another.

Poul-Henning Kamp phk at FreeBSD.org
Mon Jul 6 22:39:46 CEST 2015


commit 95aeadd2d421aad7fccd3fcf666a8fe52920b457
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Mon Jul 6 20:37:29 2015 +0000

    Don't hold waiters mutex'en over Waiter_Call() since they are also
    held over Wait_Enter() processing, and that makes it impossible
    for the code using the waiter to avoid one deadlock or another.
    
    Depending on what the call-back code does, this may increase or
    decrease contests for the waiters mutex.

diff --git a/bin/varnishd/waiter/cache_waiter_epoll.c b/bin/varnishd/waiter/cache_waiter_epoll.c
index 9eeec23..e60d55a 100644
--- a/bin/varnishd/waiter/cache_waiter_epoll.c
+++ b/bin/varnishd/waiter/cache_waiter_epoll.c
@@ -84,9 +84,9 @@ vwe_thread(void *priv)
 	THR_SetName("cache-epoll");
 
 	now = VTIM_real();
-	Lck_Lock(&vwe->mtx);
 	while (1) {
 		while (1) {
+			Lck_Lock(&vwe->mtx);
 			/*
 			 * XXX: We could avoid many syscalls here if we were
 			 * XXX: allowed to just close the fd's on timeout.
@@ -103,6 +103,7 @@ vwe_thread(void *priv)
 			AZ(epoll_ctl(vwe->epfd, EPOLL_CTL_DEL, wp->fd, NULL));
 			vwe->nwaited--;
 			Wait_HeapDelete(w, wp);
+			Lck_Unlock(&vwe->mtx);
 			Wait_Call(w, wp, WAITER_TIMEOUT, now);
 		}
 		then = vwe->next - now;
@@ -113,14 +114,15 @@ vwe_thread(void *priv)
 		assert(n >= 0);
 		assert(n <= NEEV);
 		now = VTIM_real();
-		Lck_Lock(&vwe->mtx);
 		for (ep = ev, i = 0; i < n; i++, ep++) {
 			if (ep->data.ptr == vwe) {
 				assert(read(vwe->pipe[0], &c, 1) == 1);
 				continue;
 			}
 			CAST_OBJ_NOTNULL(wp, ep->data.ptr, WAITED_MAGIC);
+			Lck_Lock(&vwe->mtx);
 			Wait_HeapDelete(w, wp);
+			Lck_Unlock(&vwe->mtx);
 			AZ(epoll_ctl(vwe->epfd, EPOLL_CTL_DEL, wp->fd, NULL));
 			vwe->nwaited--;
 			if (ep->events & EPOLLIN)
@@ -135,7 +137,6 @@ vwe_thread(void *priv)
 		if (vwe->nwaited == 0 && vwe->die)
 			break;
 	}
-	Lck_Unlock(&vwe->mtx);
 	AZ(close(vwe->pipe[0]));
 	AZ(close(vwe->pipe[1]));
 	AZ(close(vwe->epfd));
diff --git a/bin/varnishd/waiter/cache_waiter_kqueue.c b/bin/varnishd/waiter/cache_waiter_kqueue.c
index 63bf794..1b7e6a0 100644
--- a/bin/varnishd/waiter/cache_waiter_kqueue.c
+++ b/bin/varnishd/waiter/cache_waiter_kqueue.c
@@ -79,9 +79,9 @@ vwk_thread(void *priv)
 	THR_SetName("cache-kqueue");
 
 	now = VTIM_real();
-	Lck_Lock(&vwk->mtx);
 	while (1) {
 		while (1) {
+			Lck_Lock(&vwk->mtx);
 			/*
 			 * XXX: We could avoid many syscalls here if we were
 			 * XXX: allowed to just close the fd's on timeout.
@@ -98,6 +98,7 @@ vwk_thread(void *priv)
 			EV_SET(ke, wp->fd, EVFILT_READ, EV_DELETE, 0, 0, NULL);
 			AZ(kevent(vwk->kq, ke, 1, NULL, 0, NULL));
 			Wait_HeapDelete(w, wp);
+			Lck_Unlock(&vwk->mtx);
 			Wait_Call(w, wp, WAITER_TIMEOUT, now);
 		}
 		then = vwk->next - now;
@@ -108,7 +109,6 @@ vwk_thread(void *priv)
 		assert(n >= 0);
 		assert(n <= NKEV);
 		now = VTIM_real();
-		Lck_Lock(&vwk->mtx);
 		for (kp = ke, j = 0; j < n; j++, kp++) {
 			assert(kp->filter == EVFILT_READ);
 			if (ke[j].udata == vwk) {
@@ -116,7 +116,9 @@ vwk_thread(void *priv)
 				continue;
 			}
 			CAST_OBJ_NOTNULL(wp, ke[j].udata, WAITED_MAGIC);
+			Lck_Lock(&vwk->mtx);
 			Wait_HeapDelete(w, wp);
+			Lck_Unlock(&vwk->mtx);
 			vwk->nwaited--;
 			if (kp->flags & EV_EOF)
 				Wait_Call(w, wp, WAITER_REMCLOSE, now);
@@ -126,7 +128,6 @@ vwk_thread(void *priv)
 		if (vwk->nwaited == 0 && vwk->die)
 			break;
 	}
-	Lck_Unlock(&vwk->mtx);
 	AZ(close(vwk->pipe[0]));
 	AZ(close(vwk->pipe[1]));
 	AZ(close(vwk->kq));



More information about the varnish-commit mailing list