[master] 196e396 Struct exp being 20 bytes long causes alignment issues in persistent.

Poul-Henning Kamp phk at FreeBSD.org
Tue Feb 9 01:19:12 CET 2016


commit 196e3965a9b8461ea1136b7d8d10240bf081d8ce
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Mon Feb 8 22:15:43 2016 +0000

    Struct exp being 20 bytes long causes alignment issues in persistent.
    
    Add macros to work on "disolved" struct exp

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 95dde95..7ca3eb7 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -694,10 +694,34 @@ extern pthread_t cli_thread;
 #define ASSERT_CLI() do {assert(pthread_self() == cli_thread);} while (0)
 
 /* cache_expire.c */
-void EXP_Clr(struct exp *e);
+
+/*
+ * The set of variables which control object expiry are inconveniently
+ * 24 bytes long (double+3*float) and this causes alignment waste if
+ * we put then in a struct.
+ * These three macros operate on the struct we don't use.
+ */
+
+#define EXP_ZERO(xx)							\
+	do {								\
+		(xx)->t_origin = 0.0;					\
+		(xx)->ttl = 0.0;					\
+		(xx)->grace = 0.0;					\
+		(xx)->keep = 0.0;					\
+	} while (0)
+
+#define EXP_COPY(to,fm)							\
+	do {								\
+		(to)->t_origin = (fm)->t_origin;			\
+		(to)->ttl = (fm)->ttl;					\
+		(to)->grace = (fm)->grace;				\
+		(to)->keep = (fm)->keep;				\
+	} while (0)
+
+#define EXP_WHEN(to)							\
+	((to)->t_origin + (to)->ttl + (to)->grace + (to)->keep)
 
 double EXP_Ttl(const struct req *, const struct exp*);
-double EXP_When(const struct exp *exp);
 void EXP_Insert(struct worker *wrk, struct objcore *oc);
 void EXP_Rearm(struct objcore *, double now, double ttl, double grace,
     double keep);
diff --git a/bin/varnishd/cache/cache_expire.c b/bin/varnishd/cache/cache_expire.c
index 06db2d7..10f5f82 100644
--- a/bin/varnishd/cache/cache_expire.c
+++ b/bin/varnishd/cache/cache_expire.c
@@ -87,20 +87,6 @@ exp_event(struct worker *wrk, struct objcore *oc, enum exp_event_e e)
 }
 
 /*--------------------------------------------------------------------
- * struct exp manipulations
- */
-
-void
-EXP_Clr(struct exp *e)
-{
-
-	e->ttl = -1;
-	e->grace = 0;
-	e->keep = 0;
-	e->t_origin = 0;
-}
-
-/*--------------------------------------------------------------------
  * Calculate an objects effective ttl time, taking req.ttl into account
  * if it is available.
  */
@@ -120,7 +106,7 @@ EXP_Ttl(const struct req *req, const struct exp *e)
  * Calculate when this object is no longer useful
  */
 
-double
+static double
 EXP_When(const struct exp *e)
 {
 	double when;
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index de2a2ca..57295da 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -798,6 +798,7 @@ vbf_stp_error(struct worker *wrk, struct busyobj *bo)
 	http_TimeHeader(bo->beresp, "Date: ", now);
 	http_SetHeader(bo->beresp, "Server: Varnish");
 
+	bo->fetch_objcore->exp.t_origin = now;
 	if (!VTAILQ_EMPTY(&bo->fetch_objcore->objhead->waitinglist)) {
 		/*
 		 * If there is a waitinglist, it means that there is no
@@ -806,13 +807,13 @@ vbf_stp_error(struct worker *wrk, struct busyobj *bo)
 		 * each objcore on the waiting list sequentially attempt
 		 * to fetch from the backend.
 		 */
-		bo->fetch_objcore->exp.t_origin = now;
 		bo->fetch_objcore->exp.ttl = 1;
 		bo->fetch_objcore->exp.grace = 5;
 		bo->fetch_objcore->exp.keep = 5;
 	} else {
-		EXP_Clr(&bo->fetch_objcore->exp);
-		bo->fetch_objcore->exp.t_origin = now;
+		bo->fetch_objcore->exp.ttl = 0;
+		bo->fetch_objcore->exp.grace = 0;
+		bo->fetch_objcore->exp.keep = 0;
 	}
 
 	synth_body = VSB_new_auto();
diff --git a/bin/varnishd/storage/storage_persistent.c b/bin/varnishd/storage/storage_persistent.c
index 14ee3a7..bbc8a83 100644
--- a/bin/varnishd/storage/storage_persistent.c
+++ b/bin/varnishd/storage/storage_persistent.c
@@ -475,7 +475,7 @@ smp_allocx(const struct stevedore *st, size_t min_size, size_t max_size,
 			sc->next_top -= sizeof(**so);
 			*so = (void*)(sc->base + sc->next_top);
 			/* Render this smp_object mostly harmless */
-			EXP_Clr(&(*so)->exp);
+			EXP_ZERO((*so));
 			(*so)->ban = 0.;
 			(*so)->ptr = 0;
 			sg->objs = *so;
@@ -562,7 +562,7 @@ smp_allocobj(struct worker *wrk, const struct stevedore *stv,
 	/* We have to do this somewhere, might as well be here... */
 	assert(sizeof so->hash == DIGEST_LEN);
 	memcpy(so->hash, oc->objhead->digest, DIGEST_LEN);
-	so->exp = oc->exp;
+	EXP_COPY(so, &oc->exp);
 	so->ptr = (uint8_t*)o - sc->base;
 	so->ban = BAN_Time(oc->ban);
 
diff --git a/bin/varnishd/storage/storage_persistent.h b/bin/varnishd/storage/storage_persistent.h
index 4ccb014..9edb47b 100644
--- a/bin/varnishd/storage/storage_persistent.h
+++ b/bin/varnishd/storage/storage_persistent.h
@@ -144,7 +144,10 @@ struct smp_segptr {
 
 struct smp_object {
 	uint8_t			hash[32];	/* really: DIGEST_LEN */
-	struct exp		exp;
+	double			t_origin;
+	float			ttl;
+	float			grace;
+	float			keep;
 	uint32_t		__filler__;	/* -> align/8 on 32bit */
 	double			ban;
 	uint64_t		ptr;		/* rel to silo */
diff --git a/bin/varnishd/storage/storage_persistent_silo.c b/bin/varnishd/storage/storage_persistent_silo.c
index f46787f..9710dab 100644
--- a/bin/varnishd/storage/storage_persistent_silo.c
+++ b/bin/varnishd/storage/storage_persistent_silo.c
@@ -157,7 +157,7 @@ smp_load_seg(struct worker *wrk, const struct smp_sc *sc,
 	/* Clear the bogus "hold" count */
 	sg->nobj = 0;
 	for (;no > 0; so++,no--) {
-		if (EXP_When(&so->exp) < t_now)
+		if (EXP_WHEN(so) < t_now)
 			continue;
 		oc = ObjNew(wrk);
 		oc->flags &= ~OC_F_BUSY;
@@ -166,7 +166,7 @@ smp_load_seg(struct worker *wrk, const struct smp_sc *sc,
 		VTAILQ_INSERT_TAIL(&sg->objcores, oc, lru_list);
 		oc->stobj->priv2 |= NEED_FIXUP;
 		oc->ban = BAN_RefBan(oc, so->ban);
-		oc->exp = so->exp;
+		EXP_COPY(&oc->exp, so);
 		sg->nobj++;
 		oc->refcnt++;
 		HSH_Insert(wrk, so->hash, oc);
@@ -439,8 +439,8 @@ smp_sml_getobj(struct worker *wrk, struct objcore *oc)
 			bad |= 0x100;
 
 		if(bad) {
-			EXP_Clr(&oc->exp);
-			EXP_Clr(&so->exp);
+			EXP_ZERO(&oc->exp);
+			EXP_ZERO(so);
 		}
 
 		sg->nfixed++;
@@ -470,11 +470,11 @@ smp_oc_objupdatemeta(struct worker *wrk, struct objcore *oc)
 		/* Lock necessary, we might race close_seg */
 		Lck_Lock(&sg->sc->mtx);
 		so->ban = BAN_Time(oc->ban);
-		so->exp = oc->exp;
+		EXP_COPY(so, &oc->exp);
 		Lck_Unlock(&sg->sc->mtx);
 	} else {
 		so->ban = BAN_Time(oc->ban);
-		so->exp = oc->exp;
+		EXP_COPY(so, &oc->exp);
 	}
 }
 
@@ -491,7 +491,7 @@ smp_oc_objfree(struct worker *wrk, struct objcore *oc)
 	so = smp_find_so(sg, oc->stobj->priv2);
 
 	Lck_Lock(&sg->sc->mtx);
-	EXP_Clr(&so->exp);
+	EXP_ZERO(so);
 	so->ptr = 0;
 
 	assert(sg->nobj > 0);



More information about the varnish-commit mailing list