Tighten on the relative accesses to wrap-sequence and special marker

values in the shmlog.

PS: Sometimes I really long for the days when computers executed the
instructions in the order written.



git-svn-id: http://www.varnish-cache.org/svn/trunk/varnish-cache@4924 d4fa192b-c00b-0410-8231-f00ffab90ce4

bin/varnishd/cache_shmlog.c

@@ -69,12 +69,12 @@ static void
 vsl_wrap(void)
 {
 
-	vsl_log_start[1] = vsl_w0(SLT_ENDMARKER, 0);
+	vsl_log_start[1] = VSL_ENDMARKER;
 	do
 		vsl_log_start[0]++;
 	while (vsl_log_start[0] == 0);
 	VWMB();
-	*vsl_log_nxt = vsl_w0(SLT_WRAPMARKER, 0);
+	*vsl_log_nxt = VSL_WRAPMARKER;
 	vsl_log_nxt = vsl_log_start + 1;
 	VSL_stats->shm_cycles++;
 }
@@ -106,7 +106,7 @@ vsl_get(unsigned len, unsigned records, unsigned flushes)
 	p = vsl_log_nxt;
 	vsl_log_nxt = VSL_END(vsl_log_nxt, len);
 
-	*vsl_log_nxt = vsl_w0(SLT_ENDMARKER, 0);
+	*vsl_log_nxt = VSL_ENDMARKER;
 
 	assert(vsl_log_nxt < vsl_log_end);
 	assert(((uintptr_t)vsl_log_nxt & 0x3) == 0);

bin/varnishd/mgt_shmem.c

@@ -295,7 +295,8 @@ mgt_SHM_Init(const char *fn, const char *l_arg)
 	AN(vsl_log_start);
 	vsl_log_end = (void*)((uint8_t *)vsl_log_start + s1);
 	vsl_log_nxt = vsl_log_start + 1;
-	*vsl_log_nxt = (SLT_ENDMARKER << 24);
+	*vsl_log_nxt = VSL_ENDMARKER;
+	VWMB();
 
 	do
 		*vsl_log_start = random() & 0xffff;

bin/varnishlog/flint.lnt

@@ -15,4 +15,5 @@
 -e574	// Signed-unsigned mix with relational
 
 -e788	// enum constant '___' not used within defaulted switch
+-e641	// Converting enum '___' to '___'
 

include/shmlog.h

@@ -96,24 +96,26 @@ struct shmloghead {
  *	[n + 2] ... [m]	= content
  */
 
-#define VSL_WORDS(len) (((len) + 3) / 4)
-#define VSL_END(ptr, len) ((ptr) + 2 + VSL_WORDS(len))
-#define VSL_NEXT(ptr) VSL_END(ptr, VSL_LEN(ptr))
-#define VSL_LEN(ptr) ((ptr)[0] & 0xffff)
-#define VSL_TAG(ptr) ((ptr)[0] >> 24)
-#define VSL_ID(ptr) ((ptr)[1])
-#define VSL_DATA(ptr) ((char*)((ptr)+2))
+#define VSL_WORDS(len)		(((len) + 3) / 4)
+#define VSL_END(ptr, len)	((ptr) + 2 + VSL_WORDS(len))
+#define VSL_NEXT(ptr)		VSL_END(ptr, VSL_LEN(ptr))
+#define VSL_LEN(ptr)		((ptr)[0] & 0xffff)
+#define VSL_TAG(ptr)		((ptr)[0] >> 24)
+#define VSL_ID(ptr)		((ptr)[1])
+#define VSL_DATA(ptr)		((char*)((ptr)+2))
+
+#define VSL_ENDMARKER	(((uint32_t)SLT_Reserved << 24) | 0x454545) /* "EEE" */
+#define VSL_WRAPMARKER	(((uint32_t)SLT_Reserved << 24) | 0x575757) /* "WWW" */
 
 /*
  * The identifiers in shmlogtag are "SLT_" + XML tag.  A script may be run
  * on this file to extract the table rather than handcode it
  */
 enum shmlogtag {
-	SLT_ENDMARKER = 0,
 #define SLTM(foo)	SLT_##foo,
 #include "shmlog_tags.h"
 #undef SLTM
-	SLT_WRAPMARKER = 255U
+	SLT_Reserved = 255
 };
 
 /* This function lives in both libvarnish and libvarnishapi */

include/varnishapi.h

@@ -50,7 +50,7 @@ typedef int vsl_handler(void *priv, enum shmlogtag tag, unsigned fd,
 vsl_handler VSL_H_Print;
 struct VSL_data;
 struct VSL_data *VSL_New(void);
-void VSL_Select(struct VSL_data *vd, unsigned tag);
+void VSL_Select(const struct VSL_data *vd, unsigned tag);
 int VSL_OpenLog(struct VSL_data *vd);
 void VSL_NonBlocking(struct VSL_data *vd, int nb);
 int VSL_Dispatch(struct VSL_data *vd, vsl_handler *func, void *priv);

lib/libvarnishapi/Makefile.am

@@ -14,6 +14,7 @@ libvarnishapi_la_SOURCES = \
 	../libvarnish/svn_version.c \
 	../libvarnish/version.c \
 	../libvarnish/vin.c \
+	../libvarnish/vmb.c \
 	../libvarnish/vre.c \
 	base64.c \
 	vsl.c \

lib/libvarnishapi/vsl.h

@@ -66,9 +66,11 @@ struct VSL_data {
 
 	/* Stuff relating the log records below here */
 
-	uint32_t		*log_start;
-	uint32_t		*log_end;
-	uint32_t		*log_ptr;
+	volatile uint32_t	*log_start;
+	volatile uint32_t	*log_end;
+	volatile uint32_t	*log_ptr;
+
+	volatile uint32_t	last_seq;
 
 	/* for -r option */
 	int			r_fd;

lib/libvarnishapi/vsl_log.c

@@ -49,6 +49,7 @@ SVNID("$Id$")
 #include "varnishapi.h"
 
 #include "vsl.h"
+#include "vmb.h"
 
 static int vsl_nextlog(struct VSL_data *vd, uint32_t **pp);
 
@@ -63,7 +64,7 @@ const char *VSL_tags[256] = {
 /*--------------------------------------------------------------------*/
 
 void
-VSL_Select(struct VSL_data *vd, unsigned tag)
+VSL_Select(const struct VSL_data *vd, unsigned tag)
 {
 
 	CHECK_OBJ_NOTNULL(vd, VSL_MAGIC);
@@ -88,9 +89,8 @@ static int
 vsl_nextlog(struct VSL_data *vd, uint32_t **pp)
 {
 	unsigned w, l;
-	uint8_t t;
+	uint32_t t;
 	int i;
-	uint32_t seq;
 
 	CHECK_OBJ_NOTNULL(vd, VSL_MAGIC);
 	if (vd->r_fd != -1) {
@@ -111,27 +111,26 @@ vsl_nextlog(struct VSL_data *vd, uint32_t **pp)
 		*pp = vd->rbuf;
 		return (1);
 	}
-	seq = vd->log_start[0];
 	for (w = 0; w < TIMEOUT_USEC;) {
-		t = VSL_TAG(vd->log_ptr);
+		t = *vd->log_ptr;
 
-		if (t != SLT_ENDMARKER) {
-			*pp = vd->log_ptr;
-			vd->log_ptr = VSL_NEXT(vd->log_ptr);
-			return (1);
-		}
-
-		if (t == SLT_WRAPMARKER || vd->log_start[0] != seq) {
+		if (t == VSL_WRAPMARKER ||
+		    (t == VSL_ENDMARKER && vd->last_seq != vd->log_start[0])) {
 			vd->log_ptr = vd->log_start + 1;
-			seq = vd->log_start[0];
+			vd->last_seq = vd->log_start[0];
+			VRMB();
 			continue;
 		}
-
-		/* XXX: check log_start[0] */
-		if (vd->flags & F_NON_BLOCKING)
-			return (-1);
-		w += SLEEP_USEC;
-		usleep(SLEEP_USEC);
+		if (t == VSL_ENDMARKER) {
+			if (vd->flags & F_NON_BLOCKING)
+				return (-1);
+			w += SLEEP_USEC;
+			AZ(usleep(SLEEP_USEC));
+			continue;
+		}
+		*pp = (void*)(uintptr_t)vd->log_ptr; /* Loose volatile */
+		vd->log_ptr = VSL_NEXT(vd->log_ptr);
+		return (1);
 	}
 	*pp = NULL;
 	return (0);
@@ -142,7 +141,7 @@ VSL_NextLog(struct VSL_data *vd, uint32_t **pp)
 {
 	uint32_t *p;
 	unsigned char t;
-	unsigned u, l;
+	unsigned u;
 	int i;
 
 	CHECK_OBJ_NOTNULL(vd, VSL_MAGIC);
@@ -151,7 +150,6 @@ VSL_NextLog(struct VSL_data *vd, uint32_t **pp)
 		if (i != 1)
 			return (i);
 		u = VSL_ID(p);
-		l = VSL_LEN(p);
 		t = VSL_TAG(p);
 		switch(t) {
 		case SLT_SessionOpen:
@@ -186,18 +184,14 @@ VSL_NextLog(struct VSL_data *vd, uint32_t **pp)
 		if (vd->c_opt && !vbit_test(vd->vbm_client, u))
 			continue;
 		if (vd->regincl != NULL) {
-			i = VRE_exec(vd->regincl,
-				     VSL_DATA(p),
-				     VSL_LEN(p), /* Length */
-				     0, 0, NULL, 0);
+			i = VRE_exec(vd->regincl, VSL_DATA(p), VSL_LEN(p),
+			    0, 0, NULL, 0);
 			if (i == VRE_ERROR_NOMATCH)
 				continue;
 		}
 		if (vd->regexcl != NULL) {
-			i = VRE_exec(vd->regincl,
-				     VSL_DATA(p),
-				     VSL_LEN(p), /* Length */
-				     0, 0, NULL, 0);
+			i = VRE_exec(vd->regincl, VSL_DATA(p), VSL_LEN(p),
+			    0, 0, NULL, 0);
 			if (i != VRE_ERROR_NOMATCH)
 				continue;
 		}
@@ -247,7 +241,7 @@ VSL_H_Print(void *priv, enum shmlogtag tag, unsigned fd, unsigned len,
 	    (spec & VSL_S_BACKEND) ? 'b' : '-';
 
 	if (tag == SLT_Debug) {
-		fprintf(fo, "%5d %-12s %c \"", fd, VSL_tags[tag], type);
+		fprintf(fo, "%5u %-12s %c \"", fd, VSL_tags[tag], type);
 		while (len-- > 0) {
 			if (*ptr >= ' ' && *ptr <= '~')
 				fprintf(fo, "%c", *ptr);
@@ -258,7 +252,7 @@ VSL_H_Print(void *priv, enum shmlogtag tag, unsigned fd, unsigned len,
 		fprintf(fo, "\"\n");
 		return (0);
 	}
-	fprintf(fo, "%5d %-12s %c %.*s\n", fd, VSL_tags[tag], type, len, ptr);
+	fprintf(fo, "%5u %-12s %c %.*s\n", fd, VSL_tags[tag], type, len, ptr);
 	return (0);
 }
 
@@ -276,13 +270,14 @@ VSL_OpenLog(struct VSL_data *vd)
 	assert(sha != NULL);
 
 	vd->log_start = SHA_PTR(sha);
-	vd->log_end = (void*)((char *)vd->log_start + sha->len - sizeof *sha);
+	vd->log_end = SHA_NEXT(sha);
 	vd->log_ptr = vd->log_start + 1;
 
+	vd->last_seq = vd->log_start[0];
+	VRMB();
 	if (!vd->d_opt && vd->r_fd == -1) {
-		while (VSL_TAG(vd->log_ptr) != SLT_ENDMARKER)
+		while (*vd->log_ptr != VSL_ENDMARKER)
 			vd->log_ptr = VSL_NEXT(vd->log_ptr);
 	}
 	return (0);
 }
-