[experimental-ims] d9b4655 Overhaul the stevedore statistics, the SMA and SMF interpreted certain fields in different ways.

Geoff Simmons geoff at varnish-cache.org
Wed Aug 31 16:00:17 CEST 2011


commit d9b4655a7959a32e796c5456d0cb1390ebd7ee6c
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Tue Aug 9 10:01:53 2011 +0000

    Overhaul the stevedore statistics, the SMA and SMF interpreted certain
    fields in different ways.

diff --git a/bin/varnishd/storage_file.c b/bin/varnishd/storage_file.c
index 27cc86f..ea78526 100644
--- a/bin/varnishd/storage_file.c
+++ b/bin/varnishd/storage_file.c
@@ -182,9 +182,9 @@ insfree(struct smf_sc *sc, struct smf *sp)
 	b = sp->size / sc->pagesize;
 	if (b >= NBUCKET) {
 		b = NBUCKET - 1;
-		sc->stats->n_smf_large++;
+		sc->stats->g_smf_large++;
 	} else {
-		sc->stats->n_smf_frag++;
+		sc->stats->g_smf_frag++;
 	}
 	sp->flist = &sc->free[b];
 	ns = b * sc->pagesize;
@@ -212,9 +212,9 @@ remfree(const struct smf_sc *sc, struct smf *sp)
 	b = sp->size / sc->pagesize;
 	if (b >= NBUCKET) {
 		b = NBUCKET - 1;
-		sc->stats->n_smf_large--;
+		sc->stats->g_smf_large--;
 	} else {
-		sc->stats->n_smf_frag--;
+		sc->stats->g_smf_frag--;
 	}
 	assert(sp->flist == &sc->free[b]);
 	VTAILQ_REMOVE(sp->flist, sp, status);
@@ -260,7 +260,7 @@ alloc_smf(struct smf_sc *sc, size_t bytes)
 	/* Split from front */
 	sp2 = malloc(sizeof *sp2);
 	XXXAN(sp2);
-	sc->stats->n_smf++;
+	sc->stats->g_smf++;
 	*sp2 = *sp;
 
 	sp->offset += bytes;
@@ -302,7 +302,7 @@ free_smf(struct smf *sp)
 		VTAILQ_REMOVE(&sc->order, sp2, order);
 		remfree(sc, sp2);
 		free(sp2);
-		sc->stats->n_smf--;
+		sc->stats->g_smf--;
 	}
 
 	sp2 = VTAILQ_PREV(sp, smfhead, order);
@@ -314,7 +314,7 @@ free_smf(struct smf *sp)
 		sp2->size += sp->size;
 		VTAILQ_REMOVE(&sc->order, sp, order);
 		free(sp);
-		sc->stats->n_smf--;
+		sc->stats->g_smf--;
 		sp = sp2;
 	}
 
@@ -339,7 +339,7 @@ trim_smf(struct smf *sp, size_t bytes)
 	CHECK_OBJ_NOTNULL(sp, SMF_MAGIC);
 	sp2 = malloc(sizeof *sp2);
 	XXXAN(sp2);
-	sc->stats->n_smf++;
+	sc->stats->g_smf++;
 	*sp2 = *sp;
 
 	sp2->size -= bytes;
@@ -365,7 +365,7 @@ new_smf(struct smf_sc *sc, unsigned char *ptr, off_t off, size_t len)
 	XXXAN(sp);
 	sp->magic = SMF_MAGIC;
 	sp->s.magic = STORAGE_MAGIC;
-	sc->stats->n_smf++;
+	sc->stats->g_smf++;
 
 	sp->sc = sc;
 	sp->size = len;
@@ -452,7 +452,7 @@ smf_open(const struct stevedore *st)
 	if (sum < MINPAGES * (off_t)getpagesize())
 		exit (2);
 
-	sc->stats->bfree += sc->filesize;
+	sc->stats->g_space += sc->filesize;
 }
 
 /*--------------------------------------------------------------------*/
@@ -468,16 +468,18 @@ smf_alloc(struct stevedore *st, size_t size)
 	size += (sc->pagesize - 1);
 	size &= ~(sc->pagesize - 1);
 	Lck_Lock(&sc->mtx);
-	sc->stats->nreq++;
+	sc->stats->c_req++;
 	smf = alloc_smf(sc, size);
 	if (smf == NULL) {
+		sc->stats->c_fail++;
 		Lck_Unlock(&sc->mtx);
 		return (NULL);
 	}
 	CHECK_OBJ_NOTNULL(smf, SMF_MAGIC);
-	sc->stats->nobj++;
-	sc->stats->balloc += smf->size;
-	sc->stats->bfree -= smf->size;
+	sc->stats->g_alloc++;
+	sc->stats->c_bytes += smf->size;
+	sc->stats->g_bytes += smf->size;
+	sc->stats->g_space -= smf->size;
 	Lck_Unlock(&sc->mtx);
 	CHECK_OBJ_NOTNULL(&smf->s, STORAGE_MAGIC);	/*lint !e774 */
 	XXXAN(smf);
@@ -513,8 +515,9 @@ smf_trim(struct storage *s, size_t size)
 	size &= ~(sc->pagesize - 1);
 	if (smf->size > size) {
 		Lck_Lock(&sc->mtx);
-		sc->stats->balloc -= (smf->size - size);
-		sc->stats->bfree += (smf->size - size);
+		sc->stats->c_freed += (smf->size - size);
+		sc->stats->g_bytes -= (smf->size - size);
+		sc->stats->g_space += (smf->size - size);
 		trim_smf(smf, size);
 		assert(smf->size == size);
 		Lck_Unlock(&sc->mtx);
@@ -534,9 +537,10 @@ smf_free(struct storage *s)
 	CAST_OBJ_NOTNULL(smf, s->priv, SMF_MAGIC);
 	sc = smf->sc;
 	Lck_Lock(&sc->mtx);
-	sc->stats->nobj--;
-	sc->stats->balloc -= smf->size;
-	sc->stats->bfree += smf->size;
+	sc->stats->g_alloc--;
+	sc->stats->c_freed += smf->size;
+	sc->stats->g_bytes -= smf->size;
+	sc->stats->g_space += smf->size;
 	free_smf(smf);
 	Lck_Unlock(&sc->mtx);
 }
diff --git a/bin/varnishd/storage_malloc.c b/bin/varnishd/storage_malloc.c
index 56b4daa..7035ff0 100644
--- a/bin/varnishd/storage_malloc.c
+++ b/bin/varnishd/storage_malloc.c
@@ -65,14 +65,17 @@ sma_alloc(struct stevedore *st, size_t size)
 
 	CAST_OBJ_NOTNULL(sma_sc, st->priv, SMA_SC_MAGIC);
 	Lck_Lock(&sma_sc->sma_mtx);
-	sma_sc->stats->nreq++;
-	if (sma_sc->sma_alloc + size > sma_sc->sma_max)
+	sma_sc->stats->c_req++;
+	if (sma_sc->sma_alloc + size > sma_sc->sma_max) {
 		size = 0;
-	else {
+		sma_sc->stats->c_fail += size;
+	} else {
 		sma_sc->sma_alloc += size;
-		sma_sc->stats->nobj++;
-		sma_sc->stats->nbytes += size;
-		sma_sc->stats->balloc += size;
+		sma_sc->stats->c_bytes += size;
+		sma_sc->stats->g_alloc++;
+		sma_sc->stats->g_bytes += size;
+		if (sma_sc->sma_max != SIZE_MAX)
+			sma_sc->stats->g_space -= size;
 	}
 	Lck_Unlock(&sma_sc->sma_mtx);
 
@@ -96,9 +99,16 @@ sma_alloc(struct stevedore *st, size_t size)
 	}
 	if (sma == NULL) {
 		Lck_Lock(&sma_sc->sma_mtx);
-		sma_sc->stats->nobj--;
-		sma_sc->stats->nbytes -= size;
-		sma_sc->stats->balloc -= size;
+		/*
+		 * XXX: Not nice to have counters go backwards, but we do
+		 * XXX: Not want to pick up the lock twice just for stats.
+		 */
+		sma_sc->stats->c_fail++;
+		sma_sc->stats->c_bytes -= size;
+		sma_sc->stats->g_alloc--;
+		sma_sc->stats->g_bytes -= size;
+		if (sma_sc->sma_max != SIZE_MAX)
+			sma_sc->stats->g_space += size;
 		Lck_Unlock(&sma_sc->sma_mtx);
 		return (NULL);
 	}
@@ -127,9 +137,11 @@ sma_free(struct storage *s)
 	assert(sma->sz == sma->s.space);
 	Lck_Lock(&sma_sc->sma_mtx);
 	sma_sc->sma_alloc -= sma->sz;
-	sma_sc->stats->nobj--;
-	sma_sc->stats->nbytes -= sma->sz;
-	sma_sc->stats->bfree += sma->sz;
+	sma_sc->stats->g_alloc--;
+	sma_sc->stats->g_bytes -= sma->sz;
+	sma_sc->stats->c_freed += sma->sz;
+	if (sma_sc->sma_max != SIZE_MAX)
+		sma_sc->stats->g_space += sma->sz;
 	Lck_Unlock(&sma_sc->sma_mtx);
 	free(sma->s.ptr);
 	free(sma);
@@ -150,8 +162,10 @@ sma_trim(struct storage *s, size_t size)
 	assert(size < sma->sz);
 	if ((p = realloc(sma->s.ptr, size)) != NULL) {
 		Lck_Lock(&sma_sc->sma_mtx);
-		sma_sc->stats->nbytes -= (sma->sz - size);
-		sma_sc->stats->bfree += sma->sz - size;
+		sma_sc->stats->g_bytes -= (sma->sz - size);
+		sma_sc->stats->c_freed += sma->sz - size;
+		if (sma_sc->sma_max != SIZE_MAX)
+			sma_sc->stats->g_space += sma->sz - size;
 		sma->sz = size;
 		Lck_Unlock(&sma_sc->sma_mtx);
 		sma->s.ptr = p;
@@ -219,6 +233,8 @@ sma_open(const struct stevedore *st)
 	sma_sc->stats = VSM_Alloc(sizeof *sma_sc->stats,
 	    VSC_CLASS, VSC_TYPE_SMA, st->ident);
 	memset(sma_sc->stats, 0, sizeof *sma_sc->stats);
+	if (sma_sc->sma_max != SIZE_MAX)
+		sma_sc->stats->g_space = sma_sc->sma_max;
 }
 
 const struct stevedore sma_stevedore = {
diff --git a/bin/varnishtest/tests/b00002.vtc b/bin/varnishtest/tests/b00002.vtc
index 243de19..54a7ab7 100644
--- a/bin/varnishtest/tests/b00002.vtc
+++ b/bin/varnishtest/tests/b00002.vtc
@@ -21,7 +21,7 @@ client c1 {
 delay .1
 
 varnish v1 -expect n_object == 0
-varnish v1 -expect SMA.Transient.nobj == 0
+varnish v1 -expect SMA.Transient.g_alloc == 0
 varnish v1 -expect client_conn == 1
 varnish v1 -expect client_req == 1
 varnish v1 -expect s_sess == 1
diff --git a/bin/varnishtest/tests/g00002.vtc b/bin/varnishtest/tests/g00002.vtc
index dba75ae..3f2f951 100644
--- a/bin/varnishtest/tests/g00002.vtc
+++ b/bin/varnishtest/tests/g00002.vtc
@@ -32,9 +32,9 @@ client c1 {
 } -run
 
 # If this fails, the multiple storage allocations did not happen
-varnish v1 -expect SMF.s0.nreq != 0
-varnish v1 -expect SMF.s0.nreq != 1
-varnish v1 -expect SMF.s0.nreq != 2
+varnish v1 -expect SMF.s0.c_req != 0
+varnish v1 -expect SMF.s0.c_req != 1
+varnish v1 -expect SMF.s0.c_req != 2
 
 client c1 {
 	# See varnish can gunzip it.
diff --git a/include/vsc_fields.h b/include/vsc_fields.h
index 0289b81..0952698 100644
--- a/include/vsc_fields.h
+++ b/include/vsc_fields.h
@@ -28,6 +28,13 @@
  *
  * 3rd argument marks fields for inclusion in the per worker-thread
  * stats structure.
+ *
+ * XXX: We need a much more consistent naming of these fields, this has
+ * XXX: turned into a major mess, causing trouble already for backends.
+ * XXX:
+ * XXX: Please converge on:
+ * XXX:		c_* counter	(total bytes ever allocated from sma)
+ * XXX:		g_* gauge	(presently allocated bytes from sma)
  */
 
 /**********************************************************************/
@@ -173,11 +180,13 @@ VSC_F(colls,		uint64_t, 0, 'a', "Collisions")
  */
 
 #if defined(VSC_DO_SMA) || defined (VSC_DO_SMF)
-VSC_F(nreq,		uint64_t, 0, 'a', "Allocator requests")
-VSC_F(nobj,		uint64_t, 0, 'i', "Outstanding allocations")
-VSC_F(nbytes,		uint64_t, 0, 'i', "Outstanding bytes")
-VSC_F(balloc,		uint64_t, 0, 'i', "Bytes allocated")
-VSC_F(bfree,		uint64_t, 0, 'i', "Bytes free")
+VSC_F(c_req,		uint64_t, 0, 'a', "Allocator requests")
+VSC_F(c_fail,		uint64_t, 0, 'a', "Allocator failures")
+VSC_F(c_bytes,		uint64_t, 0, 'a', "Bytes allocated")
+VSC_F(c_freed,		uint64_t, 0, 'a', "Bytes freed")
+VSC_F(g_alloc,		uint64_t, 0, 'i', "Allocations outstanding")
+VSC_F(g_bytes,		uint64_t, 0, 'i', "Bytes outstanding")
+VSC_F(g_space,		uint64_t, 0, 'i', "Bytes available")
 #endif
 
 
@@ -190,9 +199,9 @@ VSC_F(bfree,		uint64_t, 0, 'i', "Bytes free")
 /**********************************************************************/
 
 #ifdef VSC_DO_SMF
-VSC_F(n_smf,		uint64_t, 0, 'i', "N struct smf")
-VSC_F(n_smf_frag,		uint64_t, 0, 'i', "N small free smf")
-VSC_F(n_smf_large,		uint64_t, 0, 'i', "N large free smf")
+VSC_F(g_smf,			uint64_t, 0, 'i', "N struct smf")
+VSC_F(g_smf_frag,		uint64_t, 0, 'i', "N small free smf")
+VSC_F(g_smf_large,		uint64_t, 0, 'i', "N large free smf")
 #endif
 
 /**********************************************************************/



More information about the varnish-commit mailing list