[6.0] 150d4c4ee Clean up dead code in hsh_deref_objhead_unlock

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Fri Feb 8 13:13:11 UTC 2019


commit 150d4c4ee2841d46652d256c5a5a42efa944ce6e
Author: Dag Haavi Finstad <daghf at varnish-software.com>
Date:   Wed Dec 5 10:25:57 2018 +0100

    Clean up dead code in hsh_deref_objhead_unlock
    
    req's on waiting list do indeed hold an objhead ref (req->hash_objhead),
    so the condition for the while loop in hsh_deref_objhead_unlock will
    always evaluate to false.
    
    This gets rid of the needless rushing code and replaces it with an
    assert.

diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index 197ebc4c7..6d030c9ad 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -776,8 +776,10 @@ HSH_Unbusy(struct worker *wrk, struct objcore *oc)
 	VTAILQ_REMOVE(&oh->objcs, oc, hsh_list);
 	VTAILQ_INSERT_HEAD(&oh->objcs, oc, hsh_list);
 	oc->flags &= ~OC_F_BUSY;
-	if (!VTAILQ_EMPTY(&oh->waitinglist))
+	if (!VTAILQ_EMPTY(&oh->waitinglist)) {
+		assert(oh->refcnt > 1);
 		hsh_rush1(wrk, oh, &rush, HSH_RUSH_POLICY);
+	}
 	Lck_Unlock(&oh->mtx);
 	if (!(oc->flags & OC_F_PRIVATE))
 		EXP_Insert(wrk, oc);
@@ -932,8 +934,10 @@ HSH_DerefObjCore(struct worker *wrk, struct objcore **ocp, int rushmax)
 	r = --oc->refcnt;
 	if (!r)
 		VTAILQ_REMOVE(&oh->objcs, oc, hsh_list);
-	if (!VTAILQ_EMPTY(&oh->waitinglist))
+	if (!VTAILQ_EMPTY(&oh->waitinglist)) {
+		assert(oh->refcnt > 1);
 		hsh_rush1(wrk, oh, &rush, rushmax);
+	}
 	Lck_Unlock(&oh->mtx);
 	hsh_rush2(wrk, &rush);
 	if (r != 0)
@@ -958,11 +962,9 @@ static int
 hsh_deref_objhead_unlock(struct worker *wrk, struct objhead **poh)
 {
 	struct objhead *oh;
-	struct rush rush;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	TAKE_OBJ_NOTNULL(oh, poh, OBJHEAD_MAGIC);
-	INIT_OBJ(&rush, RUSH_MAGIC);
 
 	Lck_AssertHeld(&oh->mtx);
 
@@ -974,20 +976,8 @@ hsh_deref_objhead_unlock(struct worker *wrk, struct objhead **poh)
 		return(1);
 	}
 
-	/*
-	 * Make absolutely certain that we do not let the final ref
-	 * disappear until the waitinglist is empty.
-	 * This is necessary because the req's on the waiting list do
-	 * not hold any ref on the objhead of their own, and we cannot
-	 * just make the hold the same ref's as objcore, that would
-	 * confuse hashers.
-	 */
-	while (oh->refcnt == 1 && !VTAILQ_EMPTY(&oh->waitinglist)) {
-		hsh_rush1(wrk, oh, &rush, HSH_RUSH_ALL);
-		Lck_Unlock(&oh->mtx);
-		hsh_rush2(wrk, &rush);
-		Lck_Lock(&oh->mtx);
-	}
+	if (oh->refcnt == 1)
+		assert(VTAILQ_EMPTY(&oh->waitinglist));
 
 	assert(oh->refcnt > 0);
 	return (hash->deref(wrk, oh));


More information about the varnish-commit mailing list