[master] 0068b31aa Collect and make explicit where we enter and leave "tasks" for the purpose of VRT_priv_task()

Poul-Henning Kamp phk at FreeBSD.org
Tue Aug 28 20:55:07 UTC 2018


commit 0068b31aa70140f729a729acdfa1dbb7b2678a50
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Tue Aug 28 20:52:13 2018 +0000

    Collect and make explicit where we enter and leave "tasks" for
    the purpose of VRT_priv_task()

diff --git a/bin/varnishd/cache/cache_busyobj.c b/bin/varnishd/cache/cache_busyobj.c
index 5527b588c..e27431042 100644
--- a/bin/varnishd/cache/cache_busyobj.c
+++ b/bin/varnishd/cache/cache_busyobj.c
@@ -133,14 +133,11 @@ VBO_GetBusyObj(struct worker *wrk, const struct req *req)
 	bo->director_req = req->director_hint;
 	bo->vcl = req->vcl;
 	VCL_Ref(bo->vcl);
-	VCL_Onboard(NULL, bo);
 
 	bo->t_first = bo->t_prev = NAN;
 
 	memcpy(bo->digest, req->digest, sizeof bo->digest);
 
-	VRTPRIV_init(bo->privs);
-
 	return (bo);
 }
 
@@ -156,8 +153,7 @@ VBO_ReleaseBusyObj(struct worker *wrk, struct busyobj **pbo)
 	AZ(bo->htc);
 	AZ(bo->stale_oc);
 
-	VRTPRIV_dynamic_kill(bo->privs, (uintptr_t)bo);
-	assert(VTAILQ_EMPTY(&bo->privs->privs));
+	AZ(bo->privs->magic);
 
 	VSLb(bo->vsl, SLT_BereqAcct, "%ju %ju %ju %ju %ju %ju",
 	    (uintmax_t)bo->acct.bereq_hdrbytes,
diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c
index 516268c5b..1e238d854 100644
--- a/bin/varnishd/cache/cache_esi_deliver.c
+++ b/bin/varnishd/cache/cache_esi_deliver.c
@@ -164,7 +164,6 @@ ved_include(struct req *preq, const char *src, const char *host,
 	AZ(req->vcl);
 	req->vcl = preq->vcl;
 	preq->vcl = NULL;
-	VCL_Onboard(req, NULL);
 
 	req->req_step = R_STP_RECV;
 	req->t_req = preq->t_req;
@@ -198,8 +197,6 @@ ved_include(struct req *preq, const char *src, const char *host,
 		AZ(req->wrk);
 	}
 
-	VRTPRIV_dynamic_kill(req->privs, (uintptr_t)req);
-
 	AZ(preq->vcl);
 	preq->vcl = req->vcl;
 	req->vcl = NULL;
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 0f81e544e..57583239a 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -924,6 +924,7 @@ vbf_fetch_thread(struct worker *wrk, void *priv)
 	}
 #endif
 
+	VCL_TaskEnter(bo->vcl, bo->privs);
 	while (stp != F_STP_DONE) {
 		CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
 		assert(bo->fetch_objcore->boc->refcount >= 1);
@@ -944,6 +945,7 @@ vbf_fetch_thread(struct worker *wrk, void *priv)
 
 	assert(bo->director_state == DIR_S_NULL);
 
+	VCL_TaskLeave(bo->vcl, bo->privs);
 	http_Teardown(bo->bereq);
 	http_Teardown(bo->beresp);
 
diff --git a/bin/varnishd/cache/cache_req.c b/bin/varnishd/cache/cache_req.c
index d741441ca..036056740 100644
--- a/bin/varnishd/cache/cache_req.c
+++ b/bin/varnishd/cache/cache_req.c
@@ -149,8 +149,6 @@ Req_New(const struct worker *wrk, struct sess *sp)
 	req->t_prev = NAN;
 	req->t_req = NAN;
 
-	VRTPRIV_init(req->privs);
-
 	return (req);
 }
 
@@ -181,14 +179,6 @@ Req_Release(struct req *req)
 	MPL_Free(pp->mpl_req, req);
 }
 
-static void
-req_finalize(struct req *req)
-{
-	VRTPRIV_dynamic_kill(req->privs, (uintptr_t)req);
-	VRTPRIV_dynamic_kill(req->privs, (uintptr_t)&req->top);
-	assert(VTAILQ_EMPTY(&req->privs->privs));
-}
-
 /*----------------------------------------------------------------------
  * TODO:
  * - check for code duplication with cnt_recv_prep
@@ -198,7 +188,8 @@ req_finalize(struct req *req)
 void
 Req_Rollback(struct req *req)
 {
-	req_finalize(req);
+	VCL_TaskLeave(req->vcl, req->privs);
+	VCL_TaskEnter(req->vcl, req->privs);
 	HTTP_Copy(req->http, req->http0);
 	WS_Reset(req->ws, req->ws_req);
 }
@@ -220,6 +211,7 @@ Req_Cleanup(struct sess *sp, struct worker *wrk, struct req *req)
 	req->restarts = 0;
 
 	AZ(req->esi_level);
+	AZ(req->privs->magic);
 	assert(req->top == req);
 
 	if (req->vcl != NULL) {
@@ -229,8 +221,6 @@ Req_Cleanup(struct sess *sp, struct worker *wrk, struct req *req)
 		req->vcl = NULL;
 	}
 
-	req_finalize(req);
-
 	/* Charge and log byte counters */
 	if (req->vsl->wid) {
 		Req_AcctLogCharge(wrk->stats, req);
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index deecc3434..dcf6e9df0 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -1022,6 +1022,7 @@ CNT_Request(struct worker *wrk, struct req *req)
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
+	AN(req->vcl);
 
 	/*
 	 * Possible entrance states
@@ -1036,6 +1037,8 @@ CNT_Request(struct worker *wrk, struct req *req)
 	/* wrk can have changed for restarts */
 	req->vfc->wrk = req->wrk = wrk;
 	wrk->vsl = req->vsl;
+	if (req->req_step != R_STP_LOOKUP)
+		VCL_TaskEnter(req->vcl, req->privs);
 	for (nxt = REQ_FSM_MORE; nxt == REQ_FSM_MORE; ) {
 		/*
 		 * This is a good place to be paranoid about the various
@@ -1060,6 +1063,7 @@ CNT_Request(struct worker *wrk, struct req *req)
 	}
 	wrk->vsl = NULL;
 	if (nxt == REQ_FSM_DONE) {
+		VCL_TaskLeave(req->vcl, req->privs);
 		AN(req->vsl->wid);
 		VRB_Free(req);
 		req->wrk = NULL;
diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h
index 52a7e9622..154a6796b 100644
--- a/bin/varnishd/cache/cache_varnishd.h
+++ b/bin/varnishd/cache/cache_varnishd.h
@@ -387,7 +387,8 @@ void VCL_Poll(void);
 void VCL_Ref(struct vcl *);
 void VCL_Refresh(struct vcl **);
 void VCL_Rel(struct vcl **);
-void VCL_Onboard(const struct req *, const struct busyobj *);
+void VCL_TaskEnter(struct vcl *, struct vrt_privs *);
+void VCL_TaskLeave(struct vcl *, struct vrt_privs *);
 const char *VCL_Return_Name(unsigned);
 const char *VCL_Method_Name(unsigned);
 void VCL_Bo2Ctx(struct vrt_ctx *, struct busyobj *);
diff --git a/bin/varnishd/cache/cache_vcl_vrt.c b/bin/varnishd/cache/cache_vcl_vrt.c
index 71e53a164..06b006b52 100644
--- a/bin/varnishd/cache/cache_vcl_vrt.c
+++ b/bin/varnishd/cache/cache_vcl_vrt.c
@@ -76,16 +76,6 @@ VCL_Method_Name(unsigned m)
 
 /*--------------------------------------------------------------------*/
 
-void
-VCL_Onboard(const struct req *req, const struct busyobj *bo)
-{
-
-	CHECK_OBJ_ORNULL(req, REQ_MAGIC);
-	CHECK_OBJ_ORNULL(bo, BUSYOBJ_MAGIC);
-	assert(req != NULL || bo != NULL);
-	assert(req == NULL || bo == NULL);
-}
-
 void
 VCL_Refresh(struct vcl **vcc)
 {
@@ -335,11 +325,12 @@ VRT_vcl_select(VRT_CTX, VCL_VCL vcl)
 	struct req *req = ctx->req;
 
 	CHECK_OBJ_NOTNULL(vcl, VCL_MAGIC);
+	VCL_TaskLeave(req->vcl, req->privs);
 	VCL_Rel(&req->vcl);
 	vcl_get(&req->vcl, vcl);
 	/* XXX: better logging */
 	VSLb(ctx->req->vsl, SLT_Debug, "Now using %s VCL", vcl->loaded_name);
-	VCL_Onboard(req, NULL);
+	VCL_TaskEnter(req->vcl, req->privs);
 }
 
 struct vclref *
diff --git a/bin/varnishd/cache/cache_vrt.c b/bin/varnishd/cache/cache_vrt.c
index 2e8a8961b..af59b6a0b 100644
--- a/bin/varnishd/cache/cache_vrt.c
+++ b/bin/varnishd/cache/cache_vrt.c
@@ -633,7 +633,8 @@ VRT_Rollback(VRT_CTX, VCL_HTTP hp)
 	} else if (hp == ctx->http_bereq) {
 		CHECK_OBJ_NOTNULL(ctx->bo, BUSYOBJ_MAGIC);
 		// -> VBO_Rollback ?
-		VRTPRIV_dynamic_kill(ctx->bo->privs, (uintptr_t)ctx->bo);
+		VCL_TaskLeave(ctx->bo->vcl, ctx->bo->privs);
+		VCL_TaskEnter(ctx->bo->vcl, ctx->bo->privs);
 		HTTP_Copy(ctx->bo->bereq, ctx->bo->bereq0);
 		WS_Reset(ctx->bo->bereq->ws, ctx->bo->ws_bo);
 		WS_Reset(ctx->bo->ws, ctx->bo->ws_bo);
diff --git a/bin/varnishd/cache/cache_vrt_priv.c b/bin/varnishd/cache/cache_vrt_priv.c
index b29f12047..1d7aff5a2 100644
--- a/bin/varnishd/cache/cache_vrt_priv.c
+++ b/bin/varnishd/cache/cache_vrt_priv.c
@@ -190,3 +190,31 @@ VRT_priv_fini(const struct vmod_priv *p)
 	if (p->priv != NULL && p->free != NULL)
 		p->free(p->priv);
 }
+
+/*--------------------------------------------------------------------*/
+
+void
+VCL_TaskEnter(struct vcl *vcl, struct vrt_privs *privs)
+{
+
+	VSL(SLT_Debug, 0, "%s %p %p %u", __func__, vcl, privs, privs->magic);
+	AN(vcl);
+	AZ(privs->magic);
+	VRTPRIV_init(privs);
+}
+
+void
+VCL_TaskLeave(struct vcl *vcl, struct vrt_privs *privs)
+{
+	struct vrt_priv *vp, *vp1;
+
+	AN(vcl);
+	VSL(SLT_Debug, 0, "%s %p %p %u", __func__, vcl, privs, privs->magic);
+	CHECK_OBJ_NOTNULL(privs, VRT_PRIVS_MAGIC);
+	VTAILQ_FOREACH_SAFE(vp, &privs->privs, list, vp1) {
+		VTAILQ_REMOVE(&privs->privs, vp, list);
+		VRT_priv_fini(vp->priv);
+	}
+	INIT_OBJ(privs, 0);
+}
+
diff --git a/bin/varnishd/http1/cache_http1_fsm.c b/bin/varnishd/http1/cache_http1_fsm.c
index 30bd3b894..b339339f5 100644
--- a/bin/varnishd/http1/cache_http1_fsm.c
+++ b/bin/varnishd/http1/cache_http1_fsm.c
@@ -286,7 +286,6 @@ http1_dissect(struct worker *wrk, struct req *req)
 	VCL_Refresh(&wrk->vcl);
 	req->vcl = wrk->vcl;
 	wrk->vcl = NULL;
-	VCL_Onboard(req, NULL);
 
 	HTTP_Setup(req->http, req->ws, req->vsl, SLT_ReqMethod);
 	req->err_code = HTTP1_DissectRequest(req->htc, req->http);
diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index 644474309..f6ea07eae 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -643,7 +643,6 @@ h2_rx_headers(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2)
 	VCL_Refresh(&wrk->vcl);
 	req->vcl = wrk->vcl;
 	wrk->vcl = NULL;
-	VCL_Onboard(req, NULL);
 	req->acct.req_hdrbytes += h2->rxf_len;
 
 	HTTP_Setup(req->http, req->ws, req->vsl, SLT_ReqMethod);


More information about the varnish-commit mailing list