diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h index 7d6b684571c..6f80c14a698 100644 --- a/bin/varnishd/cache/cache.h +++ b/bin/varnishd/cache/cache.h @@ -491,9 +491,6 @@ struct req { struct objcore *body_oc; - /* The busy objhead we sleep on */ - struct objhead *hash_objhead; - /* Built Vary string == workspace reservation */ uint8_t *vary_b; uint8_t *vary_e; diff --git a/bin/varnishd/cache/cache_ban_lurker.c b/bin/varnishd/cache/cache_ban_lurker.c index 76ac772b9f7..311ec31d7fe 100644 --- a/bin/varnishd/cache/cache_ban_lurker.c +++ b/bin/varnishd/cache/cache_ban_lurker.c @@ -332,7 +332,7 @@ ban_lurker_test_ban(struct worker *wrk, struct vsl_log *vsl, struct ban *bt, if (i) ObjSendEvent(wrk, oc, OEV_BANCHG); } - (void)HSH_DerefObjCore(wrk, &oc, 0); + (void)HSH_DerefObjCore(wrk, &oc); } } diff --git a/bin/varnishd/cache/cache_busyobj.c b/bin/varnishd/cache/cache_busyobj.c index 9a5fe36d11a..37d42088c5d 100644 --- a/bin/varnishd/cache/cache_busyobj.c +++ b/bin/varnishd/cache/cache_busyobj.c @@ -176,8 +176,7 @@ VBO_ReleaseBusyObj(struct worker *wrk, struct busyobj **pbo) wrk->stats->ws_backend_overflow++; if (bo->fetch_objcore != NULL) { - (void)HSH_DerefObjCore(wrk, &bo->fetch_objcore, - HSH_RUSH_POLICY); + (void)HSH_DerefObjCore(wrk, &bo->fetch_objcore); } VRT_Assign_Backend(&bo->director_req, NULL); diff --git a/bin/varnishd/cache/cache_expire.c b/bin/varnishd/cache/cache_expire.c index fb823e3cc9d..68bd56041ca 100644 --- a/bin/varnishd/cache/cache_expire.c +++ b/bin/varnishd/cache/cache_expire.c @@ -213,7 +213,7 @@ EXP_Insert(struct worker *wrk, struct objcore *oc) ObjSendEvent(wrk, oc, OEV_EXPIRE); tmpoc = oc; assert(oc->refcnt >= 2); /* Silence coverity */ - (void)HSH_DerefObjCore(wrk, &oc, 0); + (void)HSH_DerefObjCore(wrk, &oc); AZ(oc); assert(tmpoc->refcnt >= 1); /* Silence coverity */ } @@ -287,7 +287,7 @@ exp_inbox(struct exp_priv *ep, struct objcore *oc, unsigned flags, double now) VXID(ObjGetXID(ep->wrk, oc)), EXP_Ttl(NULL, oc) - now, (intmax_t)oc->hits); ObjSendEvent(ep->wrk, oc, OEV_EXPIRE); - (void)HSH_DerefObjCore(ep->wrk, &oc, 0); + (void)HSH_DerefObjCore(ep->wrk, &oc); return; } @@ -365,7 +365,7 @@ exp_expire(struct exp_priv *ep, vtim_real now) VXID(ObjGetXID(ep->wrk, oc)), EXP_Ttl(NULL, oc) - now, (intmax_t)oc->hits); ObjSendEvent(ep->wrk, oc, OEV_EXPIRE); - (void)HSH_DerefObjCore(ep->wrk, &oc, 0); + (void)HSH_DerefObjCore(ep->wrk, &oc); } return (0); } diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c index f49de59cf71..102f8b58235 100644 --- a/bin/varnishd/cache/cache_fetch.c +++ b/bin/varnishd/cache/cache_fetch.c @@ -705,11 +705,8 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo) assert(oc->boc->state == BOS_REQ_DONE); - if (bo->do_stream) { - ObjSetState(wrk, oc, BOS_PREP_STREAM); - HSH_Unbusy(wrk, oc); + if (bo->do_stream) ObjSetState(wrk, oc, BOS_STREAM); - } VSLb(bo->vsl, SLT_Fetch_Body, "%u %s %s", bo->htc->body_status->nbr, bo->htc->body_status->name, @@ -744,11 +741,8 @@ vbf_stp_fetchend(struct worker *wrk, struct busyobj *bo) if (bo->do_stream) assert(oc->boc->state == BOS_STREAM); - else { + else assert(oc->boc->state == BOS_REQ_DONE); - ObjSetState(wrk, oc, BOS_PREP_STREAM); - HSH_Unbusy(wrk, oc); - } ObjSetState(wrk, oc, BOS_FINISHED); VSLb_ts_busyobj(bo, "BerespBody", W_TIM_real(wrk)); @@ -877,11 +871,8 @@ vbf_stp_condfetch(struct worker *wrk, struct busyobj *bo) ObjSetFlag(bo->wrk, oc, OF_IMSCAND, 0); AZ(ObjCopyAttr(bo->wrk, oc, stale_oc, OA_GZIPBITS)); - if (bo->do_stream) { - ObjSetState(wrk, oc, BOS_PREP_STREAM); - HSH_Unbusy(wrk, oc); + if (bo->do_stream) ObjSetState(wrk, oc, BOS_STREAM); - } INIT_OBJ(vop, VBF_OBITER_PRIV_MAGIC); vop->bo = bo; @@ -1025,8 +1016,7 @@ vbf_stp_error(struct worker *wrk, struct busyobj *bo) } AZ(ObjSetU64(wrk, oc, OA_LEN, o)); VSB_destroy(&synth_body); - ObjSetState(wrk, oc, BOS_PREP_STREAM); - HSH_Unbusy(wrk, oc); + ObjSetState(wrk, oc, BOS_STREAM); if (stale != NULL && oc->ttl > 0) HSH_Kill(stale); ObjSetState(wrk, oc, BOS_FINISHED); @@ -1047,10 +1037,8 @@ vbf_stp_fail(struct worker *wrk, struct busyobj *bo) CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC); assert(oc->boc->state < BOS_FINISHED); - HSH_Fail(oc); - if (!(oc->flags & OC_F_BUSY)) - HSH_Kill(oc); ObjSetState(wrk, oc, BOS_FAILED); + HSH_Kill(oc); return (F_STP_DONE); } @@ -1122,7 +1110,7 @@ vbf_fetch_thread(struct worker *wrk, void *priv) http_Teardown(bo->beresp); // can not make assumptions about the number of references here #3434 if (bo->bereq_body != NULL) - (void) HSH_DerefObjCore(bo->wrk, &bo->bereq_body, 0); + (void)HSH_DerefObjCore(bo->wrk, &bo->bereq_body); if (oc->boc->state == BOS_FINISHED) { AZ(oc->flags & OC_F_FAILED); @@ -1132,7 +1120,7 @@ vbf_fetch_thread(struct worker *wrk, void *priv) // AZ(oc->boc); // XXX if (bo->stale_oc != NULL) - (void)HSH_DerefObjCore(wrk, &bo->stale_oc, 0); + (void)HSH_DerefObjCore(wrk, &bo->stale_oc); wrk->vsl = NULL; HSH_DerefBoc(wrk, oc); @@ -1221,7 +1209,7 @@ VBF_Fetch(struct worker *wrk, struct req *req, struct objcore *oc, wrk->stats->bgfetch_no_thread++; (void)vbf_stp_fail(req->wrk, bo); if (bo->stale_oc != NULL) - (void)HSH_DerefObjCore(wrk, &bo->stale_oc, 0); + (void)HSH_DerefObjCore(wrk, &bo->stale_oc); HSH_DerefBoc(wrk, oc); SES_Rel(bo->sp); THR_SetBusyobj(NULL); @@ -1234,11 +1222,9 @@ VBF_Fetch(struct worker *wrk, struct req *req, struct objcore *oc, (void)VRB_Ignore(req); } else { ObjWaitState(oc, BOS_STREAM); - if (oc->boc->state == BOS_FAILED) { - AN((oc->flags & OC_F_FAILED)); - } else { - AZ(oc->flags & OC_F_BUSY); - } + AZ(oc->flags & OC_F_BUSY); + if (oc->boc->state == BOS_FAILED) + AN(oc->flags & OC_F_FAILED); } } AZ(bo); @@ -1246,5 +1232,5 @@ VBF_Fetch(struct worker *wrk, struct req *req, struct objcore *oc, assert(oc->boc == boc); HSH_DerefBoc(wrk, oc); if (mode == VBF_BACKGROUND) - (void)HSH_DerefObjCore(wrk, &oc, HSH_RUSH_POLICY); + (void)HSH_DerefObjCore(wrk, &oc); } diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c index ce31aecf09f..4f26f1de8d6 100644 --- a/bin/varnishd/cache/cache_hash.c +++ b/bin/varnishd/cache/cache_hash.c @@ -75,12 +75,12 @@ struct rush { static const struct hash_slinger *hash; static struct objhead *private_oh; -static void hsh_rush1(const struct worker *, struct objhead *, - struct rush *, int); +static void hsh_rush1(const struct worker *, struct objcore *, + struct rush *); static void hsh_rush2(struct worker *, struct rush *); static int hsh_deref_objhead(struct worker *wrk, struct objhead **poh); static int hsh_deref_objhead_unlock(struct worker *wrk, struct objhead **poh, - int); + struct objcore *oc); /*---------------------------------------------------------------------*/ @@ -324,7 +324,7 @@ HSH_Insert(struct worker *wrk, const void *digest, struct objcore *oc, VTAILQ_INSERT_HEAD(&oh->objcs, oc, hsh_list); oc->flags &= ~OC_F_BUSY; if (!VTAILQ_EMPTY(&oh->waitinglist)) - hsh_rush1(wrk, oh, &rush, HSH_RUSH_POLICY); + hsh_rush1(wrk, oc, &rush); Lck_Unlock(&oh->mtx); hsh_rush2(wrk, &rush); @@ -355,6 +355,41 @@ hsh_insert_busyobj(const struct worker *wrk, struct objhead *oh) return (oc); } +/*--------------------------------------------------------------------- +*/ + +static unsigned +hsh_rush_match(struct req *req) +{ + struct objhead *oh; + struct objcore *oc; + const uint8_t *vary; + + oc = req->objcore; + CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC); + + AZ(oc->flags & OC_F_BUSY); + if (oc->flags & (OC_F_WITHDRAWN|OC_F_HFM|OC_F_HFP|OC_F_CANCEL| + OC_F_PRIVATE|OC_F_FAILED|OC_F_DYING)) + + return (0); + + if (req->vcf != NULL) /* NB: must operate under oh lock. */ + return (0); + + oh = oc->objhead; + CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC); + + if (req->hash_ignore_vary) + return (1); + if (!ObjHasAttr(req->wrk, oc, OA_VARY)) + return (1); + + vary = ObjGetAttr(req->wrk, oc, OA_VARY, NULL); + AN(vary); + return (VRY_Match(req, vary)); +} + /*--------------------------------------------------------------------- */ @@ -383,6 +418,7 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp) CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); CHECK_OBJ_NOTNULL(wrk->wpriv, WORKER_PRIV_MAGIC); CHECK_OBJ_NOTNULL(req->http, HTTP_MAGIC); + CHECK_OBJ_ORNULL(req->objcore, OBJCORE_MAGIC); CHECK_OBJ_ORNULL(req->vcf, VCF_MAGIC); AN(hash); @@ -390,15 +426,32 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp) if (DO_DEBUG(DBG_HASHEDGE)) hsh_testmagic(req->digest); - if (req->hash_objhead != NULL) { + if (req->objcore != NULL && hsh_rush_match(req)) { + TAKE_OBJ_NOTNULL(oc, &req->objcore, OBJCORE_MAGIC); + *ocp = oc; + oh = oc->objhead; + Lck_Lock(&oh->mtx); + oc->hits++; + boc_progress = oc->boc == NULL ? -1 : oc->boc->fetched_so_far; + AN(hsh_deref_objhead_unlock(wrk, &oh, NULL)); + Req_LogHit(wrk, req, oc, boc_progress); + /* NB: since this hit comes from the waiting list instead of + * a regular lookup, grace is not considered. The object is + * fresh in the context of the waiting list, even expired: it + * was successfully just [re]validated by a fetch task. + */ + return (HSH_HIT); + } + + if (req->objcore != NULL) { /* * This req came off the waiting list, and brings an - * oh refcnt with it. + * oh refcnt and an incompatible oc refcnt with it, + * the latter acquired during rush hour. */ - CHECK_OBJ_NOTNULL(req->hash_objhead, OBJHEAD_MAGIC); - oh = req->hash_objhead; + oh = req->objcore->objhead; + (void)HSH_DerefObjCore(wrk, &req->objcore); Lck_Lock(&oh->mtx); - req->hash_objhead = NULL; } else { AN(wrk->wpriv->nobjhead); oh = hash->lookup(wrk, req->digest, &wrk->wpriv->nobjhead); @@ -501,7 +554,7 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp) if (oc != NULL && oc->flags & OC_F_HFP) { xid = VXID(ObjGetXID(wrk, oc)); dttl = EXP_Dttl(req, oc); - AN(hsh_deref_objhead_unlock(wrk, &oh, HSH_RUSH_POLICY)); + AN(hsh_deref_objhead_unlock(wrk, &oh, oc)); wrk->stats->cache_hitpass++; VSLb(req->vsl, SLT_HitPass, "%u %.6f", xid, dttl); return (HSH_HITPASS); @@ -521,7 +574,7 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp) } oc->hits++; boc_progress = oc->boc == NULL ? -1 : oc->boc->fetched_so_far; - AN(hsh_deref_objhead_unlock(wrk, &oh, HSH_RUSH_POLICY)); + AN(hsh_deref_objhead_unlock(wrk, &oh, oc)); Req_LogHit(wrk, req, oc, boc_progress); return (HSH_HIT); } @@ -570,7 +623,7 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp) exp_oc->refcnt++; *ocp = exp_oc; exp_oc->hits++; - AN(hsh_deref_objhead_unlock(wrk, &oh, 0)); + AN(hsh_deref_objhead_unlock(wrk, &oh, NULL)); Req_LogHit(wrk, req, exp_oc, boc_progress); return (HSH_GRACE); } @@ -581,11 +634,12 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp) AZ(req->hash_ignore_busy); /* - * The objhead reference transfers to the sess, we get it - * back when the sess comes off the waiting list and - * calls us again + * The objhead reference is held by req while it is parked on the + * waiting list. The oh pointer is taken back from the objcore that + * triggers a rush of req off the waiting list. */ - req->hash_objhead = oh; + assert(oh->refcnt > 1); + req->wrk = NULL; req->waitinglist = 1; @@ -603,22 +657,34 @@ HSH_Lookup(struct req *req, struct objcore **ocp, struct objcore **bocp) */ static void -hsh_rush1(const struct worker *wrk, struct objhead *oh, struct rush *r, int max) +hsh_rush1(const struct worker *wrk, struct objcore *oc, struct rush *r) { - int i; + struct objhead *oh; struct req *req; - - if (max == 0) - return; - if (max == HSH_RUSH_POLICY) - max = cache_param->rush_exponent; - assert(max > 0); + int i, max; CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); - CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC); + CHECK_OBJ_ORNULL(oc, OBJCORE_MAGIC); CHECK_OBJ_NOTNULL(r, RUSH_MAGIC); VTAILQ_INIT(&r->reqs); + + if (oc == NULL) + return; + + oh = oc->objhead; + CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC); Lck_AssertHeld(&oh->mtx); + + AZ(oc->flags & OC_F_BUSY); + if (oc->flags & (OC_F_WITHDRAWN|OC_F_FAILED)) + max = 1; + else if (oc->flags & (OC_F_HFM|OC_F_HFP|OC_F_CANCEL|OC_F_PRIVATE| + OC_F_DYING)) + max = cache_param->rush_exponent; + else + max = INT_MAX; + assert(max > 0); + for (i = 0; i < max; i++) { req = VTAILQ_FIRST(&oh->waitinglist); if (req == NULL) @@ -628,8 +694,10 @@ hsh_rush1(const struct worker *wrk, struct objhead *oh, struct rush *r, int max) AZ(req->wrk); VTAILQ_REMOVE(&oh->waitinglist, req, w_list); VTAILQ_INSERT_TAIL(&r->reqs, req, w_list); - req->waitinglist = 0; + req->objcore = oc; } + + oc->refcnt += i; } /*--------------------------------------------------------------------- @@ -746,7 +814,7 @@ HSH_Purge(struct worker *wrk, struct objhead *oh, vtim_real ttl_now, EXP_Remove(ocp[i], NULL); else EXP_Rearm(ocp[i], ttl_now, ttl, grace, keep); - (void)HSH_DerefObjCore(wrk, &ocp[i], 0); + (void)HSH_DerefObjCore(wrk, &ocp[i]); AZ(ocp[i]); total++; } @@ -780,13 +848,16 @@ HSH_Purge(struct worker *wrk, struct objhead *oh, vtim_real ttl_now, */ void -HSH_Fail(struct objcore *oc) +HSH_Fail(struct worker *wrk, struct objcore *oc) { struct objhead *oh; + struct rush rush; + CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC); oh = oc->objhead; CHECK_OBJ(oh, OBJHEAD_MAGIC); + INIT_OBJ(&rush, RUSH_MAGIC); /* * We have to have either a busy bit, so that HSH_Lookup @@ -797,7 +868,12 @@ HSH_Fail(struct objcore *oc) Lck_Lock(&oh->mtx); oc->flags |= OC_F_FAILED; + if (oc->flags & OC_F_BUSY) { + oc->flags &= ~OC_F_BUSY; + hsh_rush1(wrk, oc, &rush); + } Lck_Unlock(&oh->mtx); + hsh_rush2(wrk, &rush); } /*--------------------------------------------------------------------- @@ -855,6 +931,36 @@ HSH_Cancel(struct worker *wrk, struct objcore *oc, struct boc *boc) ObjSlim(wrk, oc); } +/*--------------------------------------------------------------------- + * Withdraw an objcore that will not proceed with a fetch. + */ + +void +HSH_Withdraw(struct worker *wrk, struct objcore **ocp) +{ + struct objhead *oh; + struct objcore *oc; + struct rush rush; + + CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); + TAKE_OBJ_NOTNULL(oc, ocp, OBJCORE_MAGIC); + INIT_OBJ(&rush, RUSH_MAGIC); + + oh = oc->objhead; + CHECK_OBJ(oh, OBJHEAD_MAGIC); + + Lck_Lock(&oh->mtx); + AZ(oc->stobj->stevedore); + assert(oc->flags == OC_F_BUSY); + assert(oc->refcnt == 1); + assert(oh->refcnt > 0); + oc->flags = OC_F_WITHDRAWN; + hsh_rush1(wrk, oc, &rush); /* grabs up to 1 oc ref */ + assert(HSH_DerefObjCoreUnlock(wrk, &oc) <= 1); + + hsh_rush2(wrk, &rush); +} + /*--------------------------------------------------------------------- * Unbusy an objcore when the object is completely fetched. */ @@ -868,38 +974,46 @@ HSH_Unbusy(struct worker *wrk, struct objcore *oc) CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC); CHECK_OBJ_NOTNULL(oc->boc, BOC_MAGIC); + AN(oc->flags & OC_F_BUSY); oh = oc->objhead; CHECK_OBJ(oh, OBJHEAD_MAGIC); + + /* NB: It is guaranteed that exactly one request is waiting for + * the objcore for pass objects. The other reference is held by + * the current fetch task. + */ + if (oc->flags & OC_F_PRIVATE) { + assert(oc->refcnt == 2); + oc->flags &= ~OC_F_BUSY; + return; + } + INIT_OBJ(&rush, RUSH_MAGIC); AN(oc->stobj->stevedore); - AN(oc->flags & OC_F_BUSY); + AZ(oc->flags & OC_F_PRIVATE); assert(oh->refcnt > 0); assert(oc->refcnt > 0); - if (!(oc->flags & OC_F_PRIVATE)) { - BAN_NewObjCore(oc); - AN(oc->ban); - } + BAN_NewObjCore(oc); + AN(oc->ban); /* XXX: pretouch neighbors on oh->objcs to prevent page-on under mtx */ Lck_Lock(&oh->mtx); assert(oh->refcnt > 0); assert(oc->refcnt > 0); - if (!(oc->flags & OC_F_PRIVATE)) - EXP_RefNewObjcore(oc); /* Takes a ref for expiry */ + EXP_RefNewObjcore(oc); /* Takes a ref for expiry */ /* XXX: strictly speaking, we should sort in Date: order. */ 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)) { assert(oh->refcnt > 1); - hsh_rush1(wrk, oh, &rush, HSH_RUSH_POLICY); + hsh_rush1(wrk, oc, &rush); } Lck_Unlock(&oh->mtx); - EXP_Insert(wrk, oc); /* Does nothing unless EXP_RefNewObjcore was - * called */ + EXP_Insert(wrk, oc); hsh_rush2(wrk, &rush); } @@ -1038,35 +1152,46 @@ HSH_DerefBoc(struct worker *wrk, struct objcore *oc) */ int -HSH_DerefObjCore(struct worker *wrk, struct objcore **ocp, int rushmax) +HSH_DerefObjCore(struct worker *wrk, struct objcore **ocp) { struct objcore *oc; struct objhead *oh; - struct rush rush; - int r; CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); TAKE_OBJ_NOTNULL(oc, ocp, OBJCORE_MAGIC); assert(oc->refcnt > 0); - INIT_OBJ(&rush, RUSH_MAGIC); oh = oc->objhead; CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC); Lck_Lock(&oh->mtx); + return (HSH_DerefObjCoreUnlock(wrk, &oc)); +} + +int +HSH_DerefObjCoreUnlock(struct worker *wrk, struct objcore **ocp) +{ + struct objcore *oc; + struct objhead *oh; + int r; + + CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); + TAKE_OBJ_NOTNULL(oc, ocp, OBJCORE_MAGIC); + assert(oc->refcnt > 0); + + oh = oc->objhead; + CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC); + + Lck_AssertHeld(&oh->mtx); assert(oh->refcnt > 0); r = --oc->refcnt; if (!r) VTAILQ_REMOVE(&oh->objcs, oc, hsh_list); - 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) return (r); + AZ(oc->flags & OC_F_BUSY); AZ(oc->exp_flags); BAN_DestroyObj(oc); @@ -1083,7 +1208,8 @@ HSH_DerefObjCore(struct worker *wrk, struct objcore **ocp, int rushmax) } static int -hsh_deref_objhead_unlock(struct worker *wrk, struct objhead **poh, int max) +hsh_deref_objhead_unlock(struct worker *wrk, struct objhead **poh, + struct objcore *oc) { struct objhead *oh; struct rush rush; @@ -1105,7 +1231,7 @@ hsh_deref_objhead_unlock(struct worker *wrk, struct objhead **poh, int max) INIT_OBJ(&rush, RUSH_MAGIC); if (!VTAILQ_EMPTY(&oh->waitinglist)) { assert(oh->refcnt > 1); - hsh_rush1(wrk, oh, &rush, max); + hsh_rush1(wrk, oc, &rush); } if (oh->refcnt == 1) @@ -1126,7 +1252,7 @@ hsh_deref_objhead(struct worker *wrk, struct objhead **poh) TAKE_OBJ_NOTNULL(oh, poh, OBJHEAD_MAGIC); Lck_Lock(&oh->mtx); - return (hsh_deref_objhead_unlock(wrk, &oh, 0)); + return (hsh_deref_objhead_unlock(wrk, &oh, NULL)); } void diff --git a/bin/varnishd/cache/cache_obj.c b/bin/varnishd/cache/cache_obj.c index ec819bf8cb7..19f30b1d139 100644 --- a/bin/varnishd/cache/cache_obj.c +++ b/bin/varnishd/cache/cache_obj.c @@ -86,6 +86,7 @@ #include "cache_varnishd.h" #include "cache_obj.h" +#include "cache_objhead.h" #include "vend.h" #include "storage/storage.h" @@ -295,8 +296,7 @@ ObjWaitExtend(const struct worker *wrk, const struct objcore *oc, uint64_t l) */ void -ObjSetState(struct worker *wrk, const struct objcore *oc, - enum boc_state_e next) +ObjSetState(struct worker *wrk, struct objcore *oc, enum boc_state_e next) { const struct obj_methods *om; @@ -305,7 +305,6 @@ ObjSetState(struct worker *wrk, const struct objcore *oc, assert(next > oc->boc->state); CHECK_OBJ_ORNULL(oc->stobj->stevedore, STEVEDORE_MAGIC); - assert(next != BOS_STREAM || oc->boc->state == BOS_PREP_STREAM); assert(next != BOS_FINISHED || (oc->oa_present & (1 << OA_LEN))); if (oc->stobj->stevedore != NULL) { @@ -314,6 +313,11 @@ ObjSetState(struct worker *wrk, const struct objcore *oc, om->objsetstate(wrk, oc, next); } + if (next == BOS_FAILED) + HSH_Fail(wrk, oc); + else if (oc->boc->state < BOS_STREAM && next >= BOS_STREAM) + HSH_Unbusy(wrk, oc); + Lck_Lock(&oc->boc->mtx); oc->boc->state = next; PTOK(pthread_cond_broadcast(&oc->boc->cond)); diff --git a/bin/varnishd/cache/cache_objhead.h b/bin/varnishd/cache/cache_objhead.h index 2edfb5a883b..94ef81db527 100644 --- a/bin/varnishd/cache/cache_objhead.h +++ b/bin/varnishd/cache/cache_objhead.h @@ -65,20 +65,19 @@ enum lookup_e { HSH_BUSY, }; -void HSH_Fail(struct objcore *); void HSH_Kill(struct objcore *); void HSH_Replace(struct objcore *, const struct objcore *); void HSH_Insert(struct worker *, const void *hash, struct objcore *, struct ban *); +void HSH_Withdraw(struct worker *, struct objcore **); +void HSH_Fail(struct worker *, struct objcore *); void HSH_Unbusy(struct worker *, struct objcore *); int HSH_Snipe(const struct worker *, struct objcore *); struct boc *HSH_RefBoc(const struct objcore *); void HSH_DerefBoc(struct worker *wrk, struct objcore *); void HSH_DeleteObjHead(const struct worker *, struct objhead *); - -int HSH_DerefObjCore(struct worker *, struct objcore **, int rushmax); -#define HSH_RUSH_POLICY -1 - +int HSH_DerefObjCore(struct worker *, struct objcore **); +int HSH_DerefObjCoreUnlock(struct worker *, struct objcore **); enum lookup_e HSH_Lookup(struct req *, struct objcore **, struct objcore **); void HSH_Ref(struct objcore *o); void HSH_AddString(struct req *, void *ctx, const char *str); diff --git a/bin/varnishd/cache/cache_req_body.c b/bin/varnishd/cache/cache_req_body.c index 51d45471ec5..109999a0294 100644 --- a/bin/varnishd/cache/cache_req_body.c +++ b/bin/varnishd/cache/cache_req_body.c @@ -69,6 +69,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv) req->body_oc = HSH_Private(req->wrk); AN(req->body_oc); + req->body_oc->flags &= ~OC_F_BUSY; if (req->storage != NULL) stv = req->storage; @@ -80,7 +81,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv) if (STV_NewObject(req->wrk, req->body_oc, stv, 0) == 0) { req->req_body_status = BS_ERROR; HSH_DerefBoc(req->wrk, req->body_oc); - AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0)); + AZ(HSH_DerefObjCore(req->wrk, &req->body_oc)); (void)VFP_Error(vfc, "Object allocation failed:" " Ran out of space in %s", stv->vclname); return (-1); @@ -94,7 +95,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv) if (VFP_Open(ctx, vfc) < 0) { req->req_body_status = BS_ERROR; HSH_DerefBoc(req->wrk, req->body_oc); - AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0)); + AZ(HSH_DerefObjCore(req->wrk, &req->body_oc)); return (-1); } @@ -142,7 +143,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv) VSLb_ts_req(req, "ReqBody", VTIM_real()); if (func != NULL) { HSH_DerefBoc(req->wrk, req->body_oc); - AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0)); + AZ(HSH_DerefObjCore(req->wrk, &req->body_oc)); if (vfps == VFP_END && r == 0 && (flush & OBJ_ITER_END) == 0) r = func(priv, flush | OBJ_ITER_END, NULL, 0); if (vfps != VFP_END) { @@ -158,7 +159,7 @@ vrb_pull(struct req *req, ssize_t maxsize, objiterate_f *func, void *priv) if (vfps != VFP_END) { req->req_body_status = BS_ERROR; - AZ(HSH_DerefObjCore(req->wrk, &req->body_oc, 0)); + AZ(HSH_DerefObjCore(req->wrk, &req->body_oc)); return (-1); } @@ -279,7 +280,7 @@ VRB_Free(struct req *req) if (req->body_oc == NULL) return; - r = HSH_DerefObjCore(req->wrk, &req->body_oc, 0); + r = HSH_DerefObjCore(req->wrk, &req->body_oc); // each busyobj may have gained a reference assert (r >= 0); diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c index ed97a01107a..4f42fb65775 100644 --- a/bin/varnishd/cache/cache_req_fsm.c +++ b/bin/varnishd/cache/cache_req_fsm.c @@ -219,7 +219,7 @@ cnt_deliver(struct worker *wrk, struct req *req) ObjTouch(req->wrk, req->objcore, req->t_prev); if (Resp_Setup_Deliver(req)) { - (void)HSH_DerefObjCore(wrk, &req->objcore, HSH_RUSH_POLICY); + (void)HSH_DerefObjCore(wrk, &req->objcore); req->err_code = 500; req->req_step = R_STP_SYNTH; return (REQ_FSM_MORE); @@ -237,7 +237,7 @@ cnt_deliver(struct worker *wrk, struct req *req) if (wrk->vpi->handling != VCL_RET_DELIVER) { HSH_Cancel(wrk, req->objcore, NULL); - (void)HSH_DerefObjCore(wrk, &req->objcore, HSH_RUSH_POLICY); + (void)HSH_DerefObjCore(wrk, &req->objcore); http_Teardown(req->resp); switch (wrk->vpi->handling) { @@ -382,6 +382,7 @@ cnt_synth(struct worker *wrk, struct req *req) req->objcore = HSH_Private(wrk); CHECK_OBJ_NOTNULL(req->objcore, OBJCORE_MAGIC); + req->objcore->flags &= ~OC_F_BUSY; szl = -1; if (STV_NewObject(wrk, req->objcore, stv_transient, 0)) { body = VSB_data(synth_body); @@ -411,7 +412,7 @@ cnt_synth(struct worker *wrk, struct req *req) VSLb(req->vsl, SLT_Error, "Could not get storage"); req->doclose = SC_OVERLOAD; VSLb_ts_req(req, "Resp", W_TIM_real(wrk)); - (void)HSH_DerefObjCore(wrk, &req->objcore, 1); + (void)HSH_DerefObjCore(wrk, &req->objcore); http_Teardown(req->resp); return (REQ_FSM_DONE); } @@ -509,7 +510,7 @@ cnt_transmit(struct worker *wrk, struct req *req) if (boc != NULL) HSH_DerefBoc(wrk, req->objcore); - (void)HSH_DerefObjCore(wrk, &req->objcore, HSH_RUSH_POLICY); + (void)HSH_DerefObjCore(wrk, &req->objcore); http_Teardown(req->resp); req->vdp_filter_list = NULL; @@ -536,7 +537,7 @@ cnt_fetch(struct worker *wrk, struct req *req) if (req->objcore->flags & OC_F_FAILED) { req->err_code = 503; req->req_step = R_STP_SYNTH; - (void)HSH_DerefObjCore(wrk, &req->objcore, 1); + (void)HSH_DerefObjCore(wrk, &req->objcore); AZ(req->objcore); return (REQ_FSM_MORE); } @@ -555,20 +556,23 @@ cnt_lookup(struct worker *wrk, struct req *req) { struct objcore *oc, *busy; enum lookup_e lr; - int had_objhead = 0; + int had_objcore = 0; CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC); CHECK_OBJ_NOTNULL(req, REQ_MAGIC); - AZ(req->objcore); AZ(req->stale_oc); AN(req->vcl); VRY_Prep(req); - AZ(req->objcore); - if (req->hash_objhead) - had_objhead = 1; + if (req->waitinglist) { + CHECK_OBJ_NOTNULL(req->objcore, OBJCORE_MAGIC); + req->waitinglist = 0; + had_objcore = 1; + } else + AZ(req->objcore); + wrk->strangelove = 0; lr = HSH_Lookup(req, &oc, &busy); if (lr == HSH_BUSY) { @@ -584,7 +588,7 @@ cnt_lookup(struct worker *wrk, struct req *req) if ((unsigned)wrk->strangelove >= cache_param->vary_notice) VSLb(req->vsl, SLT_Notice, "vsl: High number of variants (%d)", wrk->strangelove); - if (had_objhead) + if (had_objcore) VSLb_ts_req(req, "Waitinglist", W_TIM_real(wrk)); if (req->vcf != NULL) { @@ -664,10 +668,10 @@ cnt_lookup(struct worker *wrk, struct req *req) } /* Drop our object, we won't need it */ - (void)HSH_DerefObjCore(wrk, &req->objcore, HSH_RUSH_POLICY); + (void)HSH_DerefObjCore(wrk, &req->objcore); if (busy != NULL) { - (void)HSH_DerefObjCore(wrk, &busy, 0); + HSH_Withdraw(wrk, &busy); VRY_Clear(req); } @@ -694,7 +698,7 @@ cnt_miss(struct worker *wrk, struct req *req) wrk->stats->cache_miss++; VBF_Fetch(wrk, req, req->objcore, req->stale_oc, VBF_NORMAL); if (req->stale_oc != NULL) - (void)HSH_DerefObjCore(wrk, &req->stale_oc, 0); + (void)HSH_DerefObjCore(wrk, &req->stale_oc); req->req_step = R_STP_FETCH; return (REQ_FSM_MORE); case VCL_RET_FAIL: @@ -714,8 +718,8 @@ cnt_miss(struct worker *wrk, struct req *req) } VRY_Clear(req); if (req->stale_oc != NULL) - (void)HSH_DerefObjCore(wrk, &req->stale_oc, 0); - AZ(HSH_DerefObjCore(wrk, &req->objcore, 1)); + (void)HSH_DerefObjCore(wrk, &req->stale_oc); + HSH_Withdraw(wrk, &req->objcore); return (REQ_FSM_MORE); } @@ -1085,7 +1089,7 @@ cnt_purge(struct worker *wrk, struct req *req) (void)HSH_Purge(wrk, boc->objhead, req->t_req, 0, 0, 0); - AZ(HSH_DerefObjCore(wrk, &boc, 1)); + HSH_Withdraw(wrk, &boc); VCL_purge_method(req->vcl, wrk, req, NULL, NULL); switch (wrk->vpi->handling) { diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h index a73ff71708d..aec38824fc8 100644 --- a/bin/varnishd/cache/cache_varnishd.h +++ b/bin/varnishd/cache/cache_varnishd.h @@ -336,8 +336,7 @@ int ObjGetSpace(struct worker *, struct objcore *, ssize_t *sz, uint8_t **ptr); void ObjExtend(struct worker *, struct objcore *, ssize_t l, int final); uint64_t ObjWaitExtend(const struct worker *, const struct objcore *, uint64_t l); -void ObjSetState(struct worker *, const struct objcore *, - enum boc_state_e next); +void ObjSetState(struct worker *, struct objcore *, enum boc_state_e next); void ObjWaitState(const struct objcore *, enum boc_state_e want); void ObjTouch(struct worker *, struct objcore *, vtim_real now); void ObjFreeObj(struct worker *, struct objcore *); diff --git a/bin/varnishd/cache/cache_vary.c b/bin/varnishd/cache/cache_vary.c index 101eca231b0..9865769d768 100644 --- a/bin/varnishd/cache/cache_vary.c +++ b/bin/varnishd/cache/cache_vary.c @@ -224,8 +224,7 @@ vry_cmp(const uint8_t *v1, const uint8_t *v2) void VRY_Prep(struct req *req) { - if (req->hash_objhead == NULL) { - /* Not a waiting list return */ + if (!req->waitinglist) { AZ(req->vary_b); AZ(req->vary_e); (void)WS_ReserveAll(req->ws); diff --git a/bin/varnishd/cache/cache_vrt_var.c b/bin/varnishd/cache/cache_vrt_var.c index 9a14519b8ae..0d3bb16efc0 100644 --- a/bin/varnishd/cache/cache_vrt_var.c +++ b/bin/varnishd/cache/cache_vrt_var.c @@ -630,7 +630,7 @@ VRT_u_bereq_body(VRT_CTX) CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC); CHECK_OBJ_NOTNULL(ctx->bo, BUSYOBJ_MAGIC); if (ctx->bo->bereq_body != NULL) { - (void)HSH_DerefObjCore(ctx->bo->wrk, &ctx->bo->bereq_body, 0); + (void)HSH_DerefObjCore(ctx->bo->wrk, &ctx->bo->bereq_body); http_Unset(ctx->bo->bereq, H_Content_Length); } diff --git a/bin/varnishd/storage/storage_lru.c b/bin/varnishd/storage/storage_lru.c index 5e046e79468..ec0369c8173 100644 --- a/bin/varnishd/storage/storage_lru.c +++ b/bin/varnishd/storage/storage_lru.c @@ -205,6 +205,6 @@ LRU_NukeOne(struct worker *wrk, struct lru *lru) ObjSlim(wrk, oc); VSLb(wrk->vsl, SLT_ExpKill, "LRU xid=%ju", VXID(ObjGetXID(wrk, oc))); - (void)HSH_DerefObjCore(wrk, &oc, 0); // Ref from HSH_Snipe + (void)HSH_DerefObjCore(wrk, &oc); // Ref from HSH_Snipe return (1); } diff --git a/bin/varnishd/storage/storage_persistent_silo.c b/bin/varnishd/storage/storage_persistent_silo.c index 81c24b4367f..eb96acd3adb 100644 --- a/bin/varnishd/storage/storage_persistent_silo.c +++ b/bin/varnishd/storage/storage_persistent_silo.c @@ -178,7 +178,7 @@ smp_load_seg(struct worker *wrk, const struct smp_sc *sc, HSH_Insert(wrk, so->hash, oc, ban); AN(oc->ban); HSH_DerefBoc(wrk, oc); // XXX Keep it an stream resurrection? - (void)HSH_DerefObjCore(wrk, &oc, HSH_RUSH_POLICY); + (void)HSH_DerefObjCore(wrk, &oc); wrk->stats->n_vampireobject++; } Pool_Sumstat(wrk); diff --git a/bin/varnishtest/tests/c00125.vtc b/bin/varnishtest/tests/c00125.vtc new file mode 100644 index 00000000000..8fbca4f46b2 --- /dev/null +++ b/bin/varnishtest/tests/c00125.vtc @@ -0,0 +1,156 @@ +varnishtest "successful expired waiting list hit" + +barrier b1 cond 2 +barrier b2 cond 2 +barrier b3 cond 2 +barrier b4 cond 2 + + +server s1 { + rxreq + expect req.http.user-agent == c1 + expect req.http.bgfetch == false + barrier b1 sync + barrier b2 sync + txresp -hdr "Cache-Control: max-age=60" -hdr "Age: 120" + + rxreq + expect req.http.user-agent == c3 + expect req.http.bgfetch == true + txresp + + # The no-cache case only works with a complicit VCL, for now. + rxreq + expect req.http.user-agent == c4 + expect req.http.bgfetch == false + barrier b3 sync + barrier b4 sync + txresp -hdr "Cache-Control: no-cache" + + rxreq + expect req.http.user-agent == c6 + expect req.http.bgfetch == false + txresp -hdr "Cache-Control: no-cache" +} -start + +varnish v1 -cliok "param.set default_grace 1h" +varnish v1 -cliok "param.set thread_pools 1" +varnish v1 -cliok "param.set debug +syncvsl,+waitinglist" +varnish v1 -vcl+backend { + sub vcl_backend_fetch { + set bereq.http.bgfetch = bereq.is_bgfetch; + } + sub vcl_beresp_stale { + # We just validated a stale object, do not mark it as + # uncacheable. The object remains available for grace + # hits and background fetches. + return; + } + sub vcl_beresp_control { + if (beresp.http.cache-control == "no-cache") { + # Keep beresp.uncacheable clear. + return; + } + } + sub vcl_deliver { + set resp.http.obj-hits = obj.hits; + set resp.http.obj-ttl = obj.ttl; + } +} -start + +client c1 { + txreq -url "/stale-hit" + rxresp + expect resp.status == 200 + expect resp.http.x-varnish == 1001 + expect resp.http.obj-hits == 0 + expect resp.http.obj-ttl < 0 +} -start + +barrier b1 sync + +client c2 { + txreq -url "/stale-hit" + rxresp + expect resp.status == 200 + expect resp.http.x-varnish == "1004 1002" + expect resp.http.obj-hits == 1 + expect resp.http.obj-ttl < 0 +} -start + +varnish v1 -expect busy_sleep == 1 +barrier b2 sync + +client c1 -wait +client c2 -wait + +varnish v1 -vsl_catchup + +varnish v1 -expect cache_miss == 1 +varnish v1 -expect cache_hit == 1 +varnish v1 -expect cache_hit_grace == 0 +varnish v1 -expect s_bgfetch == 0 + +client c3 { + txreq -url "/stale-hit" + rxresp + expect resp.status == 200 + expect resp.http.x-varnish == "1006 1002" + expect resp.http.obj-hits == 2 + expect resp.http.obj-ttl < 0 +} -run + +varnish v1 -vsl_catchup + +varnish v1 -expect cache_miss == 1 +varnish v1 -expect cache_hit == 2 +varnish v1 -expect cache_hit_grace == 1 +varnish v1 -expect s_bgfetch == 1 + +# The only way for a plain no-cache to be hit is to have a non-zero keep. +varnish v1 -cliok "param.set default_ttl 0" +varnish v1 -cliok "param.set default_grace 0" +varnish v1 -cliok "param.set default_keep 1h" + +client c4 { + txreq -url "/no-cache-hit" + rxresp + expect resp.status == 200 + expect resp.http.x-varnish == 1009 + expect resp.http.obj-hits == 0 + expect resp.http.obj-ttl <= 0 +} -start + +barrier b3 sync + +client c5 { + txreq -url "/no-cache-hit" + rxresp + expect resp.status == 200 + expect resp.http.x-varnish == "1012 1010" + expect resp.http.obj-hits == 1 + expect resp.http.obj-ttl <= 0 +} -start + +varnish v1 -expect busy_sleep == 2 +barrier b4 sync + +client c4 -wait +client c5 -wait + +varnish v1 -vsl_catchup + +varnish v1 -expect cache_miss == 2 +varnish v1 -expect cache_hit == 3 +varnish v1 -expect cache_hit_grace == 1 +varnish v1 -expect s_bgfetch == 1 + +# No hit when not on the waiting list +client c6 { + txreq -url "/no-cache-hit" + rxresp + expect resp.status == 200 + expect resp.http.x-varnish == 1014 + expect resp.http.obj-hits == 0 + expect resp.http.obj-ttl <= 0 +} -run diff --git a/include/tbl/boc_state.h b/include/tbl/boc_state.h index 44984de81b9..2e041366068 100644 --- a/include/tbl/boc_state.h +++ b/include/tbl/boc_state.h @@ -32,7 +32,6 @@ BOC_STATE(INVALID, invalid) /* don't touch (yet) */ BOC_STATE(REQ_DONE, req_done) /* bereq.* can be examined */ -BOC_STATE(PREP_STREAM, prep_stream) /* Prepare for streaming */ BOC_STATE(STREAM, stream) /* beresp.* can be examined */ BOC_STATE(FINISHED, finished) /* object is complete */ BOC_STATE(FAILED, failed) /* something went wrong */ diff --git a/include/tbl/oc_flags.h b/include/tbl/oc_flags.h index 768962fdaee..0866cc38f02 100644 --- a/include/tbl/oc_flags.h +++ b/include/tbl/oc_flags.h @@ -30,7 +30,8 @@ /*lint -save -e525 -e539 */ -OC_FLAG(BUSY, busy, (1<<1)) //lint !e835 +OC_FLAG(WITHDRAWN, withdrawn, (1<<0)) //lint !e835 +OC_FLAG(BUSY, busy, (1<<1)) OC_FLAG(HFM, hfm, (1<<2)) OC_FLAG(HFP, hfp, (1<<3)) OC_FLAG(CANCEL, cancel, (1<<4))