[PATCH 03/25] BusyObj handling improvements

Martin Blix Grydeland martin at varnish-software.com
Sun Jan 22 18:53:09 CET 2012


Fix double allocations of busyobj in some cases

Make VBO_DerefBusyObj return refcount after decrementation

Make VBO_RefBusyObj return a pointer to the ref'ed busyobj

Make objcore's have a refcount on the busyobj

Drop const from VBO_RefBusyObj arg

BusyObj lock-less mode
---
 bin/varnishd/cache/cache.h         |    5 +++--
 bin/varnishd/cache/cache_busyobj.c |   22 +++++++++++++++-------
 bin/varnishd/cache/cache_center.c  |   36 +++++++++++++++++++-----------------
 bin/varnishd/cache/cache_hash.c    |   10 ++++++++--
 4 files changed, 45 insertions(+), 28 deletions(-)

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 7fcd41e..4f260e9 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -501,6 +501,7 @@ struct busyobj {
 	unsigned		magic;
 #define BUSYOBJ_MAGIC		0x23b95567
 	struct vbo		*vbo;
+	unsigned		use_locks;
 
 	uint8_t			*vary;
 	unsigned		is_gzip;
@@ -733,8 +734,8 @@ double BAN_Time(const struct ban *ban);
 /* cache_busyobj.c */
 void VBO_Init(void);
 struct busyobj *VBO_GetBusyObj(struct worker *wrk);
-void VBO_RefBusyObj(const struct busyobj *busyobj);
-void VBO_DerefBusyObj(struct worker *wrk, struct busyobj **busyobj);
+struct busyobj *VBO_RefBusyObj(struct busyobj *busyobj);
+unsigned VBO_DerefBusyObj(struct worker *wrk, struct busyobj **busyobj);
 void VBO_Free(struct vbo **vbo);
 
 /* cache_center.c [CNT] */
diff --git a/bin/varnishd/cache/cache_busyobj.c b/bin/varnishd/cache/cache_busyobj.c
index ab4db76..b8b6fd7 100644
--- a/bin/varnishd/cache/cache_busyobj.c
+++ b/bin/varnishd/cache/cache_busyobj.c
@@ -144,21 +144,25 @@ VBO_GetBusyObj(struct worker *wrk)
 	return (&vbo->bo);
 }
 
-void
-VBO_RefBusyObj(const struct busyobj *busyobj)
+struct busyobj *
+VBO_RefBusyObj(struct busyobj *busyobj)
 {
 	struct vbo *vbo;
 
 	CHECK_OBJ_NOTNULL(busyobj, BUSYOBJ_MAGIC);
 	vbo = busyobj->vbo;
 	CHECK_OBJ_NOTNULL(vbo, VBO_MAGIC);
-	Lck_Lock(&vbo->mtx);
+	if (busyobj->use_locks)
+		Lck_Lock(&vbo->mtx);
 	assert(vbo->refcount > 0);
 	vbo->refcount++;
-	Lck_Unlock(&vbo->mtx);
+	if (busyobj->use_locks)
+		Lck_Unlock(&vbo->mtx);
+
+	return (busyobj);
 }
 
-void
+unsigned
 VBO_DerefBusyObj(struct worker *wrk, struct busyobj **pbo)
 {
 	struct busyobj *bo;
@@ -172,10 +176,12 @@ VBO_DerefBusyObj(struct worker *wrk, struct busyobj **pbo)
 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
 	vbo = bo->vbo;
 	CHECK_OBJ_NOTNULL(vbo, VBO_MAGIC);
-	Lck_Lock(&vbo->mtx);
+	if (bo->use_locks)
+		Lck_Lock(&vbo->mtx);
 	assert(vbo->refcount > 0);
 	r = --vbo->refcount;
-	Lck_Unlock(&vbo->mtx);
+	if (bo->use_locks)
+		Lck_Unlock(&vbo->mtx);
 
 	if (r == 0) {
 		/* XXX: Sanity checks & cleanup */
@@ -196,4 +202,6 @@ VBO_DerefBusyObj(struct worker *wrk, struct busyobj **pbo)
 				VBO_Free(&vbo);
 		}
 	}
+
+	return (r);
 }
diff --git a/bin/varnishd/cache/cache_center.c b/bin/varnishd/cache/cache_center.c
index f9908b3..2909212 100644
--- a/bin/varnishd/cache/cache_center.c
+++ b/bin/varnishd/cache/cache_center.c
@@ -119,6 +119,7 @@ cnt_wait(struct sess *sp, struct worker *wrk, struct req *req)
 	AZ(req->esi_level);
 	assert(req->xid == 0);
 	req->t_resp = NAN;
+	AZ(wrk->busyobj);
 
 	assert(!isnan(sp->t_req));
 	tmo = (int)(1e3 * cache_param->timeout_linger);
@@ -381,12 +382,11 @@ cnt_done(struct sess *sp, struct worker *wrk, struct req *req)
 	CHECK_OBJ_ORNULL(req->vcl, VCL_CONF_MAGIC);
 
 	AZ(req->obj);
+	AZ(req->objcore);
 	AZ(wrk->busyobj);
 	req->director = NULL;
 	req->restarts = 0;
 
-	wrk->busyobj = NULL;
-
 	SES_Charge(sp);
 
 	/* If we did an ESI include, don't mess up our state */
@@ -535,7 +535,7 @@ cnt_error(struct sess *sp, struct worker *wrk, struct req *req)
 	if (req->handling == VCL_RET_RESTART &&
 	    req->restarts <  cache_param->max_restarts) {
 		HSH_Drop(wrk);
-		VBO_DerefBusyObj(wrk, &wrk->busyobj);
+		(void)VBO_DerefBusyObj(wrk, &wrk->busyobj);
 		req->director = NULL;
 		req->restarts++;
 		sp->step = STP_RECV;
@@ -552,7 +552,7 @@ cnt_error(struct sess *sp, struct worker *wrk, struct req *req)
 	req->err_code = 0;
 	req->err_reason = NULL;
 	http_Setup(wrk->busyobj->bereq, NULL);
-	VBO_DerefBusyObj(wrk, &wrk->busyobj);
+	(void)VBO_DerefBusyObj(wrk, &wrk->busyobj);
 	sp->step = STP_PREPRESP;
 	return (0);
 }
@@ -680,7 +680,7 @@ cnt_fetch(struct sess *sp, struct worker *wrk, struct req *req)
 		AZ(HSH_Deref(wrk, req->objcore, NULL));
 		req->objcore = NULL;
 	}
-	VBO_DerefBusyObj(wrk, &wrk->busyobj);
+	(void)VBO_DerefBusyObj(wrk, &wrk->busyobj);
 	req->director = NULL;
 	req->storage_hint = NULL;
 
@@ -852,7 +852,7 @@ cnt_fetchbody(struct sess *sp, struct worker *wrk, struct req *req)
 		req->err_code = 503;
 		sp->step = STP_ERROR;
 		VDI_CloseFd(wrk, &wrk->busyobj->vbc);
-		VBO_DerefBusyObj(wrk, &wrk->busyobj);
+		(void)VBO_DerefBusyObj(wrk, &wrk->busyobj);
 		return (0);
 	}
 	CHECK_OBJ_NOTNULL(req->obj, OBJECT_MAGIC);
@@ -922,7 +922,7 @@ cnt_fetchbody(struct sess *sp, struct worker *wrk, struct req *req)
 
 	if (i) {
 		HSH_Drop(wrk);
-		VBO_DerefBusyObj(wrk, &wrk->busyobj);
+		(void)VBO_DerefBusyObj(wrk, &wrk->busyobj);
 		AZ(req->obj);
 		req->err_code = 503;
 		sp->step = STP_ERROR;
@@ -935,7 +935,7 @@ cnt_fetchbody(struct sess *sp, struct worker *wrk, struct req *req)
 		AN(req->obj->objcore->ban);
 		HSH_Unbusy(wrk);
 	}
-	VBO_DerefBusyObj(wrk, &wrk->busyobj);
+	(void)VBO_DerefBusyObj(wrk, &wrk->busyobj);
 	wrk->acct_tmp.fetch++;
 	sp->step = STP_PREPRESP;
 	return (0);
@@ -1011,7 +1011,7 @@ cnt_streambody(struct sess *sp, struct worker *wrk, struct req *req)
 	assert(WRW_IsReleased(wrk));
 	assert(wrk->wrw.ciov == wrk->wrw.siov);
 	(void)HSH_Deref(wrk, NULL, &req->obj);
-	VBO_DerefBusyObj(wrk, &wrk->busyobj);
+	(void)VBO_DerefBusyObj(wrk, &wrk->busyobj);
 	http_Setup(req->resp, NULL);
 	sp->step = STP_DONE;
 	return (0);
@@ -1103,6 +1103,10 @@ cnt_hit(struct sess *sp, struct worker *wrk, struct req *req)
 	/* Drop our object, we won't need it */
 	(void)HSH_Deref(wrk, NULL, &req->obj);
 	req->objcore = NULL;
+	if (wrk->busyobj != NULL) {
+		CHECK_OBJ_NOTNULL(wrk->busyobj, BUSYOBJ_MAGIC);
+		(void)VBO_DerefBusyObj(wrk, &wrk->busyobj);
+	}
 
 	switch(req->handling) {
 	case VCL_RET_PASS:
@@ -1274,7 +1278,6 @@ cnt_miss(struct sess *sp, struct worker *wrk, struct req *req)
 	AN(req->objcore);
 	CHECK_OBJ_NOTNULL(wrk->busyobj, BUSYOBJ_MAGIC);
 	WS_Reset(wrk->ws, NULL);
-	wrk->busyobj = VBO_GetBusyObj(wrk);
 	http_Setup(wrk->busyobj->bereq, wrk->ws);
 	http_FilterReq(sp, HTTPH_R_FETCH);
 	http_ForceGet(wrk->busyobj->bereq);
@@ -1299,13 +1302,13 @@ cnt_miss(struct sess *sp, struct worker *wrk, struct req *req)
 		AZ(HSH_Deref(wrk, req->objcore, NULL));
 		req->objcore = NULL;
 		http_Setup(wrk->busyobj->bereq, NULL);
-		VBO_DerefBusyObj(wrk, &wrk->busyobj);
+		(void)VBO_DerefBusyObj(wrk, &wrk->busyobj);
 		sp->step = STP_ERROR;
 		return (0);
 	case VCL_RET_PASS:
 		AZ(HSH_Deref(wrk, req->objcore, NULL));
 		req->objcore = NULL;
-		VBO_DerefBusyObj(wrk, &wrk->busyobj);
+		(void)VBO_DerefBusyObj(wrk, &wrk->busyobj);
 		sp->step = STP_PASS;
 		return (0);
 	case VCL_RET_FETCH:
@@ -1315,7 +1318,7 @@ cnt_miss(struct sess *sp, struct worker *wrk, struct req *req)
 	case VCL_RET_RESTART:
 		AZ(HSH_Deref(wrk, req->objcore, NULL));
 		req->objcore = NULL;
-		VBO_DerefBusyObj(wrk, &wrk->busyobj);
+		(void)VBO_DerefBusyObj(wrk, &wrk->busyobj);
 		INCOMPL();
 	default:
 		WRONG("Illegal action in vcl_miss{}");
@@ -1364,7 +1367,6 @@ cnt_pass(struct sess *sp, struct worker *wrk, struct req *req)
 	AZ(req->obj);
 	AZ(wrk->busyobj);
 
-	wrk->busyobj = VBO_GetBusyObj(wrk);
 	WS_Reset(wrk->ws, NULL);
 	wrk->busyobj = VBO_GetBusyObj(wrk);
 	http_Setup(wrk->busyobj->bereq, wrk->ws);
@@ -1376,7 +1378,7 @@ cnt_pass(struct sess *sp, struct worker *wrk, struct req *req)
 	VCL_pass_method(sp);
 	if (req->handling == VCL_RET_ERROR) {
 		http_Setup(wrk->busyobj->bereq, NULL);
-		VBO_DerefBusyObj(wrk, &wrk->busyobj);
+		(void)VBO_DerefBusyObj(wrk, &wrk->busyobj);
 		sp->step = STP_ERROR;
 		return (0);
 	}
@@ -1423,7 +1425,6 @@ cnt_pipe(struct sess *sp, struct worker *wrk, const struct req *req)
 	AZ(wrk->busyobj);
 
 	wrk->acct_tmp.pipe++;
-	wrk->busyobj = VBO_GetBusyObj(wrk);
 	WS_Reset(wrk->ws, NULL);
 	wrk->busyobj = VBO_GetBusyObj(wrk);
 	http_Setup(wrk->busyobj->bereq, wrk->ws);
@@ -1438,7 +1439,7 @@ cnt_pipe(struct sess *sp, struct worker *wrk, const struct req *req)
 	PipeSession(sp);
 	assert(WRW_IsReleased(wrk));
 	http_Setup(wrk->busyobj->bereq, NULL);
-	VBO_DerefBusyObj(wrk, &wrk->busyobj);
+	(void)VBO_DerefBusyObj(wrk, &wrk->busyobj);
 	sp->step = STP_DONE;
 	return (0);
 }
@@ -1577,6 +1578,7 @@ cnt_start(struct sess *sp, struct worker *wrk, struct req *req)
 	AZ(req->vcl);
 	EXP_Clr(&req->exp);
 	AZ(req->esi_level);
+	AZ(wrk->busyobj);
 
 	/* Update stats of various sorts */
 	wrk->stats.client_req++;
diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index d967291..e78eb60 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -306,6 +306,7 @@ HSH_Lookup(struct sess *sp, struct objhead **poh)
 	AN(sp->req->director);
 	AN(hash);
 	wrk = sp->wrk;
+	AZ(wrk->busyobj);
 
 	HSH_Prealloc(sp);
 	memcpy(sp->wrk->nobjhead->digest, sp->req->digest,
@@ -459,7 +460,7 @@ HSH_Lookup(struct sess *sp, struct objhead **poh)
 		wrk->busyobj->vary = sp->req->vary_b;
 	else
 		wrk->busyobj->vary = NULL;
-	oc->busyobj = wrk->busyobj;
+	oc->busyobj = VBO_RefBusyObj(wrk->busyobj);
 
 	/*
 	 * Busy objects go on the tail, so they will not trip up searches.
@@ -622,7 +623,8 @@ HSH_Unbusy(struct worker *wrk)
 	VTAILQ_REMOVE(&oh->objcs, oc, list);
 	VTAILQ_INSERT_HEAD(&oh->objcs, oc, list);
 	oc->flags &= ~OC_F_BUSY;
-	oc->busyobj = NULL;
+	if (oc->busyobj != NULL)
+		(void)VBO_DerefBusyObj(wrk, &oc->busyobj);
 	if (oh->waitinglist != NULL)
 		hsh_rush(oh);
 	AN(oc->ban);
@@ -710,6 +712,10 @@ HSH_Deref(struct worker *wrk, struct objcore *oc, struct object **oo)
 	BAN_DestroyObj(oc);
 	AZ(oc->ban);
 
+	if (oc->busyobj != NULL)
+		(void)VBO_DerefBusyObj(wrk, &oc->busyobj);
+	AZ(oc->busyobj);
+
 	if (oc->methods != NULL) {
 		oc_freeobj(oc);
 		wrk->stats.n_object--;
-- 
1.7.4.1




More information about the varnish-dev mailing list