[master] 68a4240 Add a "private" oh which we can hang pass and error objectcores from, so that we do not need to special-case locking all over the place.
Poul-Henning Kamp
phk at varnish-cache.org
Wed Aug 28 14:29:27 CEST 2013
commit 68a424092d589932c782c9514d340f1752f094a5
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Wed Aug 28 12:28:43 2013 +0000
Add a "private" oh which we can hang pass and error objectcores
from, so that we do not need to special-case locking all over the
place.
Give the busyobj a real reference to the objcore it's fetching.
diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 7f1b82f..4598ad3 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -423,6 +423,7 @@ struct objcore {
#define OC_F_LRUDONTMOVE (1<<4)
#define OC_F_PRIV (1<<5) /* Stevedore private flag */
#define OC_F_LURK (3<<6) /* Ban-lurker-color */
+#define OC_F_PRIVATE (1<<8)
unsigned timer_idx;
VTAILQ_ENTRY(objcore) list;
VTAILQ_ENTRY(objcore) lru_list;
diff --git a/bin/varnishd/cache/cache_busyobj.c b/bin/varnishd/cache/cache_busyobj.c
index 63ad963..cb22c54 100644
--- a/bin/varnishd/cache/cache_busyobj.c
+++ b/bin/varnishd/cache/cache_busyobj.c
@@ -243,7 +243,7 @@ void
VBO_setstate(struct busyobj *bo, enum busyobj_state_e next)
{
Lck_Lock(&bo->mtx);
- VSLb(bo->vsl, SLT_Debug, "XXX: %d -> %d", bo->state, next);
+ VSLb(bo->vsl, SLT_Debug, "XXX BOS: %d -> %d", bo->state, next);
assert(next > bo->state);
bo->state = next;
AZ(pthread_cond_signal(&bo->cond));
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index bde06c8..90d07fd 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -207,6 +207,8 @@ vbf_stp_fetchhdr(struct worker *wrk, struct busyobj *bo)
if (bo->do_esi)
bo->do_stream = 0;
+ if (bo->do_pass)
+ bo->fetch_objcore->flags |= OC_F_PASS;
if (wrk->handling == VCL_RET_DELIVER)
return (F_STP_FETCH);
@@ -357,7 +359,7 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
}
bo->stats = NULL;
if (obj == NULL) {
- AZ(HSH_Deref(&wrk->stats, bo->fetch_objcore, NULL));
+ (void)HSH_Deref(&wrk->stats, bo->fetch_objcore, NULL);
bo->fetch_objcore = NULL;
VDI_CloseFd(&bo->vbc);
VBO_setstate(bo, BOS_FAILED);
@@ -406,13 +408,14 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
assert(bo->refcount >= 1);
- if (obj->objcore->objhead != NULL) {
+ if (!(bo->fetch_obj->objcore->flags & OC_F_PRIVATE)) {
EXP_Insert(obj);
AN(obj->objcore->ban);
- AZ(obj->ws_o->overflow);
- HSH_Unbusy(&wrk->stats, obj->objcore);
}
+ AZ(obj->ws_o->overflow);
+ HSH_Unbusy(&wrk->stats, obj->objcore);
+
if (bo->vfp == NULL)
bo->vfp = &VFP_nop;
@@ -423,9 +426,6 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
assert(bo->refcount >= 1);
- if (obj->objcore->objhead != NULL)
- HSH_Ref(obj->objcore);
-
if (bo->state == BOS_FAILED) {
/* handle early failures */
(void)HSH_Deref(&wrk->stats, NULL, &obj);
@@ -433,6 +433,7 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
}
VBO_setstate(bo, BOS_FINISHED);
+VSLb(bo->vsl, SLT_Debug, "YYY REF %d %d", bo->refcount, bo->fetch_obj->objcore->refcnt);
VBO_DerefBusyObj(wrk, &bo); // XXX ?
return (F_STP_DONE);
}
@@ -550,6 +551,7 @@ VBF_Fetch(struct worker *wrk, struct req *req, struct objcore *oc, int pass)
req->vary_b = NULL;
AZ(bo->fetch_objcore);
+ HSH_Ref(oc);
bo->fetch_objcore = oc;
AZ(bo->req);
diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index ffd1087..9abe891 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -63,6 +63,7 @@
#include "vsha256.h"
static const struct hash_slinger *hash;
+static struct objhead *private_oh;
/*---------------------------------------------------------------------*/
@@ -79,11 +80,25 @@ HSH_NewObjCore(struct worker *wrk)
}
/*---------------------------------------------------------------------*/
+
+static struct objhead *
+hsh_newobjhead(void)
+{
+ struct objhead *oh;
+
+ ALLOC_OBJ(oh, OBJHEAD_MAGIC);
+ XXXAN(oh);
+ oh->refcnt = 1;
+ VTAILQ_INIT(&oh->objcs);
+ Lck_New(&oh->mtx, lck_objhdr);
+ return (oh);
+}
+
+/*---------------------------------------------------------------------*/
/* Precreate an objhead and object for later use */
static void
hsh_prealloc(struct worker *wrk)
{
- struct objhead *oh;
struct waitinglist *wl;
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
@@ -93,12 +108,7 @@ hsh_prealloc(struct worker *wrk)
CHECK_OBJ_NOTNULL(wrk->nobjcore, OBJCORE_MAGIC);
if (wrk->nobjhead == NULL) {
- ALLOC_OBJ(oh, OBJHEAD_MAGIC);
- XXXAN(oh);
- oh->refcnt = 1;
- VTAILQ_INIT(&oh->objcs);
- Lck_New(&oh->mtx, lck_objhdr);
- wrk->nobjhead = oh;
+ wrk->nobjhead = hsh_newobjhead();
wrk->stats.n_objecthead++;
}
CHECK_OBJ_NOTNULL(wrk->nobjhead, OBJHEAD_MAGIC);
@@ -116,6 +126,28 @@ hsh_prealloc(struct worker *wrk)
hash->prep(wrk);
}
+/*---------------------------------------------------------------------*/
+
+struct objcore *
+HSH_Private(struct worker *wrk)
+{
+ struct objcore *oc;
+
+ CHECK_OBJ_NOTNULL(private_oh, OBJHEAD_MAGIC);
+
+ oc = HSH_NewObjCore(wrk);
+ AN(oc);
+ oc->refcnt = 1;
+ oc->objhead = private_oh;
+ oc->flags |= OC_F_PRIVATE;
+ Lck_Lock(&private_oh->mtx);
+ VTAILQ_INSERT_TAIL(&private_oh->objcs, oc, list);
+ private_oh->refcnt++;
+ Lck_Unlock(&private_oh->mtx);
+ return (oc);
+}
+
+/*---------------------------------------------------------------------*/
void
HSH_Cleanup(struct worker *wrk)
{
@@ -286,6 +318,7 @@ hsh_insert_busyobj(struct worker *wrk, struct objhead *oh)
struct objcore *oc;
CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
+ Lck_AssertHeld(&oh->mtx);
oc = wrk->nobjcore;
wrk->nobjcore = NULL;
@@ -687,44 +720,34 @@ HSH_Deref(struct dstat *ds, struct objcore *oc, struct object **oo)
}
CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
+ assert(oc->refcnt > 0);
oh = oc->objhead;
- if (oh != NULL) {
- CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
+ CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
- Lck_Lock(&oh->mtx);
- assert(oh->refcnt > 0);
- assert(oc->refcnt > 0);
- r = --oc->refcnt;
- if (!r)
- VTAILQ_REMOVE(&oh->objcs, oc, list);
- else {
- /* Must have an object */
- AN(oc->methods);
- }
- if (oh->waitinglist != NULL)
- hsh_rush(ds, oh);
- Lck_Unlock(&oh->mtx);
- if (r != 0)
- return (r);
+ Lck_Lock(&oh->mtx);
+ assert(oh->refcnt > 0);
+ r = --oc->refcnt;
+ if (!r)
+ VTAILQ_REMOVE(&oh->objcs, oc, list);
+ if (oh->waitinglist != NULL)
+ hsh_rush(ds, oh);
+ Lck_Unlock(&oh->mtx);
+ if (r != 0)
+ return (r);
- BAN_DestroyObj(oc);
- AZ(oc->ban);
- } else
- AZ(oc->refcnt);
+ BAN_DestroyObj(oc);
+ AZ(oc->ban);
- if (oc->methods != NULL) {
+ if (oc->methods != NULL)
oc_freeobj(oc);
- ds->n_object--;
- }
+ ds->n_object--;
FREE_OBJ(oc);
ds->n_objectcore--;
- if (oh != NULL) {
- /* Drop our ref on the objhead */
- assert(oh->refcnt > 0);
- (void)HSH_DerefObjHead(ds, &oh);
- }
+ /* Drop our ref on the objhead */
+ assert(oh->refcnt > 0);
+ (void)HSH_DerefObjHead(ds, &oh);
return (0);
}
@@ -740,6 +763,14 @@ HSH_DerefObjHead(struct dstat *ds, struct objhead **poh)
*poh = NULL;
CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
+ if (oh == private_oh) {
+ Lck_Lock(&oh->mtx);
+ assert(oh->refcnt > 1);
+ oh->refcnt--;
+ Lck_Unlock(&oh->mtx);
+ return(1);
+ }
+
assert(oh->refcnt > 0);
r = hash->deref(oh);
if (!r)
@@ -755,4 +786,6 @@ HSH_Init(const struct hash_slinger *slinger)
hash = slinger;
if (hash->start != NULL)
hash->start();
+ private_oh = hsh_newobjhead();
+ private_oh->refcnt = 1;
}
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index 0d59693..3e4dabf 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -161,8 +161,11 @@ cnt_deliver(struct worker *wrk, struct req *req)
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
CHECK_OBJ_NOTNULL(req->obj, OBJECT_MAGIC);
+ CHECK_OBJ_NOTNULL(req->obj->objcore, OBJCORE_MAGIC);
CHECK_OBJ_NOTNULL(req->vcl, VCL_CONF_MAGIC);
+ assert(req->obj->objcore->refcnt > 0);
+
req->res_mode = 0;
if (!req->disable_esi && req->obj->esidata != NULL) {
@@ -247,6 +250,7 @@ cnt_deliver(struct worker *wrk, struct req *req)
STV_Freestore(req->obj);
assert(WRW_IsReleased(wrk));
+VSLb(req->vsl, SLT_Debug, "XXX REF %d", req->obj->objcore->refcnt);
(void)HSH_Deref(&wrk->stats, NULL, &req->obj);
http_Teardown(req->resp);
return (REQ_FSM_DONE);
@@ -283,7 +287,7 @@ cnt_error(struct worker *wrk, struct req *req)
req->busyobj = bo;
AZ(bo->stats);
bo->stats = &wrk->stats;
- bo->fetch_objcore = HSH_NewObjCore(wrk);
+ bo->fetch_objcore = HSH_Private(wrk);
req->obj = STV_NewObject(bo,
TRANSIENT_STORAGE, cache_param->http_resp_size,
(uint16_t)cache_param->http_max_hdr);
@@ -467,7 +471,7 @@ cnt_lookup(struct worker *wrk, struct req *req)
AZ(req->objcore);
if (lr == HSH_MISS) {
/* Found nothing */
- VSLb(req->vsl, SLT_Debug, "XXXX MISS\n");
+ VSLb(req->vsl, SLT_Debug, "XXXX MISS");
AZ(oc);
AN(boc);
AN(boc->flags & OC_F_BUSY);
@@ -483,7 +487,7 @@ cnt_lookup(struct worker *wrk, struct req *req)
if (oc->flags & OC_F_PASS) {
/* Found a hit-for-pass */
- VSLb(req->vsl, SLT_Debug, "XXXX HIT-FOR-PASS\n");
+ VSLb(req->vsl, SLT_Debug, "XXXX HIT-FOR-PASS");
AZ(boc);
(void)HSH_Deref(&wrk->stats, oc, NULL);
req->objcore = NULL;
@@ -649,7 +653,7 @@ cnt_pass(struct worker *wrk, struct req *req)
assert (wrk->handling == VCL_RET_FETCH);
req->acct_req.pass++;
- oc = HSH_NewObjCore(wrk);
+ oc = HSH_Private(wrk);
AN(oc);
req->busyobj = VBF_Fetch(wrk, req, oc, 1);
req->req_step = R_STP_FETCH;
diff --git a/bin/varnishd/hash/hash_slinger.h b/bin/varnishd/hash/hash_slinger.h
index e21a96e..79443c7 100644
--- a/bin/varnishd/hash/hash_slinger.h
+++ b/bin/varnishd/hash/hash_slinger.h
@@ -73,6 +73,7 @@ void HSH_AddString(const struct req *, const char *str);
void HSH_Insert(struct worker *, const void *hash, struct objcore *);
void HSH_Purge(struct worker *, struct objhead *, double ttl, double grace);
void HSH_config(const char *h_arg);
+struct objcore *HSH_Private(struct worker *wrk);
struct objcore *HSH_NewObjCore(struct worker *wrk);
#ifdef VARNISH_CACHE_CHILD
More information about the varnish-commit
mailing list