[master] 244705bd6 give all requests a struct reqtop

Nils Goroll nils.goroll at uplex.de
Wed Nov 13 08:48:06 UTC 2019


commit 244705bd619c914526663d6943d573e0a835d724
Author: Nils Goroll <nils.goroll at uplex.de>
Date:   Tue Nov 5 17:44:21 2019 +0100

    give all requests a struct reqtop
    
    Dynamically creating it through Req_MakeTop() would further complicate
    rollbacks.
    
    The memory overhead is basically identical to embedding struct reqtop
    into struct req, except that, for ESI, we have the (struct req).top
    point to the top request's struct reqtop.
    
    With this commit, tests/r02849.vtc and tests/r03003.vtc are failing as
    excpected. While this may impose issues with git bisect, I still think
    that this extra commit helps clarity.

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 689fe90df..898b6b83e 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -537,7 +537,7 @@ struct req {
 	struct vcf		*vcf;
 };
 
-#define IS_TOPREQ(req) ((req)->top == NULL || (req)->top->topreq == (req))
+#define IS_TOPREQ(req) ((req)->top->topreq == (req))
 
 /*--------------------------------------------------------------------
  * Struct sess is a high memory-load structure because sessions typically
diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c
index 51413eaf4..cb7a6e1c8 100644
--- a/bin/varnishd/cache/cache_esi_deliver.c
+++ b/bin/varnishd/cache/cache_esi_deliver.c
@@ -139,6 +139,7 @@ ved_include(struct req *preq, const char *src, const char *host,
 
 	req->esi_level = preq->esi_level + 1;
 
+	memset(req->top, 0, sizeof *req->top);
 	req->top = preq->top;
 
 	HTTP_Setup(req->http, req->ws, req->vsl, SLT_ReqMethod);
@@ -264,16 +265,6 @@ ved_vdp_esi_init(struct req *req, void **priv)
 	*priv = ecx;
 	RFC2616_Weaken_Etag(req->resp);
 
-	if (IS_TOPREQ(req)) {
-		Req_MakeTop(req);
-		if (req->top == NULL) {
-			VSLb(req->vsl, SLT_Error,
-			    "(top)request workspace overflow");
-			Req_Fail(req, SC_OVERLOAD);
-			return (-1);
-		}
-	}
-
 	req->res_mode |= RES_ESI;
 	if (req->resp_len != 0)
 		req->resp_len = -1;
diff --git a/bin/varnishd/cache/cache_req.c b/bin/varnishd/cache/cache_req.c
index 7812172de..10b986472 100644
--- a/bin/varnishd/cache/cache_req.c
+++ b/bin/varnishd/cache/cache_req.c
@@ -137,6 +137,11 @@ Req_New(const struct worker *wrk, struct sess *sp)
 	INIT_OBJ(req->htc, HTTP_CONN_MAGIC);
 	p = (void*)PRNDUP(p + sizeof(*req->htc));
 
+	req->top = (void*)p;
+	INIT_OBJ(req->top, REQTOP_MAGIC);
+	req->top->topreq = req;
+	p = (void*)PRNDUP(p + sizeof(*req->top));
+
 	assert(p < e);
 
 	WS_Init(req->ws, "req", p, e - p);
@@ -170,7 +175,6 @@ Req_Release(struct req *req)
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
 	MPL_AssertSane(req);
 	VSL_Flush(req->vsl, 0);
-	req->top = NULL;
 	MPL_Free(pp->mpl_req, req);
 }
 
@@ -189,7 +193,6 @@ Req_Rollback(struct req *req)
 	if (WS_Overflowed(req->ws))
 		req->wrk->stats->ws_client_overflow++;
 	WS_Reset(req->ws, req->ws_req);
-	req->top = NULL;
 }
 
 /*----------------------------------------------------------------------
@@ -235,7 +238,6 @@ Req_Cleanup(struct sess *sp, struct worker *wrk, struct req *req)
 	req->hash_ignore_busy = 0;
 	req->esi_level = 0;
 	req->is_hit = 0;
-	req->top = 0;
 
 	if (WS_Overflowed(req->ws))
 		wrk->stats->ws_client_overflow++;
@@ -254,20 +256,3 @@ Req_Fail(struct req *req, enum sess_close reason)
 	AN(req->transport->req_fail);
 	req->transport->req_fail(req, reason);
 }
-
-/*----------------------------------------------------------------------
- */
-
-void
-Req_MakeTop(struct req *req)
-{
-
-	CHECK_OBJ_ORNULL(req->top, REQTOP_MAGIC);
-	if (req->top != NULL)
-		return;
-	req->top = WS_Alloc(req->ws, sizeof *req->top);
-	if (req->top != NULL) {
-		INIT_OBJ(req->top, REQTOP_MAGIC);
-		req->top->topreq = req;
-	}
-}
diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h
index b455600ca..b8ccb4704 100644
--- a/bin/varnishd/cache/cache_varnishd.h
+++ b/bin/varnishd/cache/cache_varnishd.h
@@ -351,7 +351,6 @@ void Req_Rollback(struct req *req);
 void Req_Cleanup(struct sess *sp, struct worker *wrk, struct req *req);
 void Req_Fail(struct req *req, enum sess_close reason);
 void Req_AcctLogCharge(struct VSC_main_wrk *, struct req *);
-void Req_MakeTop(struct req *req);
 
 /* cache_req_body.c */
 int VRB_Ignore(struct req *);
diff --git a/bin/varnishd/cache/cache_vcl.c b/bin/varnishd/cache/cache_vcl.c
index 51cc21430..fe905fea8 100644
--- a/bin/varnishd/cache/cache_vcl.c
+++ b/bin/varnishd/cache/cache_vcl.c
@@ -101,11 +101,8 @@ VCL_Req2Ctx(struct vrt_ctx *ctx, struct req *req)
 	ctx->vcl = req->vcl;
 	ctx->vsl = req->vsl;
 	ctx->http_req = req->http;
-	CHECK_OBJ_ORNULL(req->top, REQTOP_MAGIC);
-	if (req->top != NULL)
-		ctx->http_req_top = req->top->topreq->http;
-	else
-		ctx->http_req_top = req->http;
+	CHECK_OBJ_NOTNULL(req->top, REQTOP_MAGIC);
+	ctx->http_req_top = req->top->topreq->http;
 	ctx->http_resp = req->resp;
 	ctx->req = req;
 	ctx->sp = req->sp;
diff --git a/bin/varnishd/cache/cache_vrt_vcl.c b/bin/varnishd/cache/cache_vrt_vcl.c
index 453499b99..66f0ce9e5 100644
--- a/bin/varnishd/cache/cache_vrt_vcl.c
+++ b/bin/varnishd/cache/cache_vrt_vcl.c
@@ -428,7 +428,7 @@ vcl_call_method(struct worker *wrk, struct req *req, struct busyobj *bo,
 		CHECK_OBJ(req, REQ_MAGIC);
 		CHECK_OBJ_NOTNULL(req->sp, SESS_MAGIC);
 		CHECK_OBJ_NOTNULL(req->vcl, VCL_MAGIC);
-		CHECK_OBJ_ORNULL(req->top, REQTOP_MAGIC);
+		CHECK_OBJ_NOTNULL(req->top, REQTOP_MAGIC);
 		VCL_Req2Ctx(&ctx, req);
 	}
 	if (bo != NULL) {


More information about the varnish-commit mailing list