[master] 6393f26 Fix a misfeature in the hasher-API: Pass the digest in, so we don't have to touch the (potential) new objhead, unless we intend to insert it.
Poul-Henning Kamp
phk at varnish-cache.org
Wed Mar 14 19:35:13 CET 2012
commit 6393f26c38d4c5bd7a6391ab1a016ed79b9830d7
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Wed Mar 14 18:33:48 2012 +0000
Fix a misfeature in the hasher-API: Pass the digest in, so we don't have
to touch the (potential) new objhead, unless we intend to insert it.
Straighten out the colvolved logic of critbit while here.
diff --git a/bin/varnishd/cache/cache_hash.c b/bin/varnishd/cache/cache_hash.c
index 514dbeb..d154717 100644
--- a/bin/varnishd/cache/cache_hash.c
+++ b/bin/varnishd/cache/cache_hash.c
@@ -253,15 +253,10 @@ HSH_Insert(struct worker *wrk, const void *digest, struct objcore *oc)
CHECK_OBJ_NOTNULL(oc, OBJCORE_MAGIC);
hsh_prealloc(wrk);
- if (cache_param->diag_bitmap & 0x80000000)
- hsh_testmagic(wrk->nobjhead->digest);
AN(wrk->nobjhead);
- memcpy(wrk->nobjhead->digest, digest, SHA256_LEN);
- oh = hash->lookup(wrk, wrk->nobjhead);
+ oh = hash->lookup(wrk, digest, &wrk->nobjhead);
CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
- if (oh == wrk->nobjhead)
- wrk->nobjhead = NULL;
Lck_Lock(&oh->mtx);
assert(oh->refcnt > 0);
@@ -303,9 +298,8 @@ HSH_Lookup(struct sess *sp)
AN(hash);
hsh_prealloc(wrk);
- memcpy(wrk->nobjhead->digest, req->digest, sizeof req->digest);
if (cache_param->diag_bitmap & 0x80000000)
- hsh_testmagic(wrk->nobjhead->digest);
+ hsh_testmagic(req->digest);
if (req->hash_objhead != NULL) {
/*
@@ -317,9 +311,7 @@ HSH_Lookup(struct sess *sp)
req->hash_objhead = NULL;
} else {
AN(wrk->nobjhead);
- oh = hash->lookup(wrk, wrk->nobjhead);
- if (oh == wrk->nobjhead)
- wrk->nobjhead = NULL;
+ oh = hash->lookup(wrk, req->digest, &wrk->nobjhead);
}
CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
diff --git a/bin/varnishd/hash/hash_classic.c b/bin/varnishd/hash/hash_classic.c
index c84238a..87e0561 100644
--- a/bin/varnishd/hash/hash_classic.c
+++ b/bin/varnishd/hash/hash_classic.c
@@ -111,24 +111,26 @@ hcl_start(void)
*/
static struct objhead * __match_proto__(hash_lookup_f)
-hcl_lookup(struct worker *wrk, struct objhead *noh)
+hcl_lookup(struct worker *wrk, const void *digest, struct objhead **noh)
{
struct objhead *oh;
struct hcl_hd *hp;
- unsigned u1, digest;
+ unsigned u1, hdigest;
int i;
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
- CHECK_OBJ_NOTNULL(noh, OBJHEAD_MAGIC);
+ AN(digest);
+ AN(noh);
+ CHECK_OBJ_NOTNULL(*noh, OBJHEAD_MAGIC);
- assert(sizeof noh->digest > sizeof digest);
- memcpy(&digest, noh->digest, sizeof digest);
- u1 = digest % hcl_nhash;
+ assert(sizeof oh->digest >= sizeof hdigest);
+ memcpy(&hdigest, digest, sizeof hdigest);
+ u1 = hdigest % hcl_nhash;
hp = &hcl_head[u1];
Lck_Lock(&hp->mtx);
VTAILQ_FOREACH(oh, &hp->head, hoh_list) {
- i = memcmp(oh->digest, noh->digest, sizeof oh->digest);
+ i = memcmp(oh->digest, digest, sizeof oh->digest);
if (i < 0)
continue;
if (i > 0)
@@ -139,14 +141,18 @@ hcl_lookup(struct worker *wrk, struct objhead *noh)
}
if (oh != NULL)
- VTAILQ_INSERT_BEFORE(oh, noh, hoh_list);
+ VTAILQ_INSERT_BEFORE(oh, *noh, hoh_list);
else
- VTAILQ_INSERT_TAIL(&hp->head, noh, hoh_list);
+ VTAILQ_INSERT_TAIL(&hp->head, *noh, hoh_list);
- noh->hoh_head = hp;
+ oh = *noh;
+ *noh = NULL;
+ memcpy(oh->digest, digest, sizeof oh->digest);
+
+ oh->hoh_head = hp;
Lck_Unlock(&hp->mtx);
- return (noh);
+ return (oh);
}
/*--------------------------------------------------------------------
diff --git a/bin/varnishd/hash/hash_critbit.c b/bin/varnishd/hash/hash_critbit.c
index 535661e..5bdab86 100644
--- a/bin/varnishd/hash/hash_critbit.c
+++ b/bin/varnishd/hash/hash_critbit.c
@@ -168,16 +168,15 @@ hcb_l_y(uintptr_t u)
*/
static unsigned
-hcb_crit_bit(const struct objhead *oh1, const struct objhead *oh2,
- struct hcb_y *y)
+hcb_crit_bit(const uint8_t *digest, const struct objhead *oh2, struct hcb_y *y)
{
unsigned char u, r;
CHECK_OBJ_NOTNULL(y, HCB_Y_MAGIC);
- for (u = 0; u < DIGEST_LEN && oh1->digest[u] == oh2->digest[u]; u++)
+ for (u = 0; u < DIGEST_LEN && digest[u] == oh2->digest[u]; u++)
;
assert(u < DIGEST_LEN);
- r = hcb_bits(oh1->digest[u], oh2->digest[u]);
+ r = hcb_bits(digest[u], oh2->digest[u]);
y->ptr = u;
y->bitmask = 0x80 >> r;
y->critbit = u * 8 + r;
@@ -191,8 +190,8 @@ hcb_crit_bit(const struct objhead *oh1, const struct objhead *oh2,
*/
static struct objhead *
-hcb_insert(struct worker *wrk, struct hcb_root *root, struct objhead *oh,
- int has_lock)
+hcb_insert(struct worker *wrk, struct hcb_root *root, const uint8_t *digest,
+ struct objhead **noh)
{
volatile uintptr_t *p;
uintptr_t pp;
@@ -203,17 +202,20 @@ hcb_insert(struct worker *wrk, struct hcb_root *root, struct objhead *oh,
p = &root->origo;
pp = *p;
if (pp == 0) {
- if (!has_lock)
+ if (noh == NULL)
return (NULL);
- *p = hcb_r_node(oh);
- return (oh);
+ oh2 = *noh;
+ *noh = NULL;
+ memcpy(oh2->digest, digest, sizeof oh2->digest);
+ *p = hcb_r_node(oh2);
+ return (oh2);
}
while(hcb_is_y(pp)) {
y = hcb_l_y(pp);
CHECK_OBJ_NOTNULL(y, HCB_Y_MAGIC);
assert(y->ptr < DIGEST_LEN);
- s = (oh->digest[y->ptr] & y->bitmask) != 0;
+ s = (digest[y->ptr] & y->bitmask) != 0;
assert(s < 2);
p = &y->leaf[s];
pp = *p;
@@ -221,7 +223,7 @@ hcb_insert(struct worker *wrk, struct hcb_root *root, struct objhead *oh,
if (pp == 0) {
/* We raced hcb_delete and got a NULL pointer */
- assert(!has_lock);
+ assert(noh == NULL);
return (NULL);
}
@@ -230,20 +232,23 @@ hcb_insert(struct worker *wrk, struct hcb_root *root, struct objhead *oh,
/* We found a node, does it match ? */
oh2 = hcb_l_node(pp);
CHECK_OBJ_NOTNULL(oh2, OBJHEAD_MAGIC);
- if (!memcmp(oh2->digest, oh->digest, DIGEST_LEN))
+ if (!memcmp(oh2->digest, digest, DIGEST_LEN))
return (oh2);
- if (!has_lock)
+ if (noh == NULL)
return (NULL);
/* Insert */
CAST_OBJ_NOTNULL(y2, wrk->nhashpriv, HCB_Y_MAGIC);
wrk->nhashpriv = NULL;
- (void)hcb_crit_bit(oh, oh2, y2);
- s2 = (oh->digest[y2->ptr] & y2->bitmask) != 0;
+ (void)hcb_crit_bit(digest, oh2, y2);
+ s2 = (digest[y2->ptr] & y2->bitmask) != 0;
assert(s2 < 2);
- y2->leaf[s2] = hcb_r_node(oh);
+ oh2 = *noh;
+ *noh = NULL;
+ memcpy(oh2->digest, digest, sizeof oh2->digest);
+ y2->leaf[s2] = hcb_r_node(oh2);
s2 = 1-s2;
p = &root->origo;
@@ -256,14 +261,14 @@ hcb_insert(struct worker *wrk, struct hcb_root *root, struct objhead *oh,
if (y->critbit > y2->critbit)
break;
assert(y->ptr < DIGEST_LEN);
- s = (oh->digest[y->ptr] & y->bitmask) != 0;
+ s = (digest[y->ptr] & y->bitmask) != 0;
assert(s < 2);
p = &y->leaf[s];
}
y2->leaf[s2] = *p;
VWMB();
*p = hcb_r_y(y2);
- return(oh);
+ return(oh2);
}
/*--------------------------------------------------------------------*/
@@ -412,45 +417,50 @@ hcb_deref(struct objhead *oh)
}
static struct objhead * __match_proto__(hash_lookup_f)
-hcb_lookup(struct worker *wrk, struct objhead *noh)
+hcb_lookup(struct worker *wrk, const void *digest, struct objhead **noh)
{
struct objhead *oh;
struct hcb_y *y;
unsigned u;
- unsigned with_lock;
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
+ AN(digest);
+ AN(noh);
+ CHECK_OBJ_NOTNULL(*noh, OBJHEAD_MAGIC);
+ assert((*noh)->refcnt == 1);
+
+ /* First try in read-only mode without holding a lock */
+
+ VSC_C_main->hcb_nolock++;
+ oh = hcb_insert(wrk, &hcb_root, digest, NULL);
+ if (oh != NULL) {
+ Lck_Lock(&oh->mtx);
+ /*
+ * A refcount of zero indicates that the tree changed
+ * under us, so fall through and try with the lock held.
+ */
+ u = oh->refcnt;
+ if (u > 0)
+ oh->refcnt++;
+ Lck_Unlock(&oh->mtx);
+ if (u > 0)
+ return (oh);
+ }
- with_lock = 0;
while (1) {
- if (with_lock) {
- CAST_OBJ_NOTNULL(y, wrk->nhashpriv, HCB_Y_MAGIC);
- Lck_Lock(&hcb_mtx);
- VSC_C_main->hcb_lock++;
- assert(noh->refcnt == 1);
- oh = hcb_insert(wrk, &hcb_root, noh, 1);
- Lck_Unlock(&hcb_mtx);
- } else {
- VSC_C_main->hcb_nolock++;
- oh = hcb_insert(wrk, &hcb_root, noh, 0);
- }
+ /* No luck, try with lock held, so we can modify tree */
+ CAST_OBJ_NOTNULL(y, wrk->nhashpriv, HCB_Y_MAGIC);
+ Lck_Lock(&hcb_mtx);
+ VSC_C_main->hcb_lock++;
+ oh = hcb_insert(wrk, &hcb_root, digest, noh);
+ Lck_Unlock(&hcb_mtx);
- if (oh != NULL && oh == noh) {
- /* Assert that we only muck with the tree with a lock */
- assert(with_lock);
- VSC_C_main->hcb_insert++;
+ CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
+ if (*noh == NULL) {
assert(oh->refcnt > 0);
+ VSC_C_main->hcb_insert++;
return (oh);
}
-
- if (oh == NULL) {
- assert(!with_lock);
- /* Try again, with lock */
- with_lock = 1;
- continue;
- }
-
- CHECK_OBJ_NOTNULL(noh, OBJHEAD_MAGIC);
Lck_Lock(&oh->mtx);
/*
* A refcount of zero indicates that the tree changed
@@ -459,8 +469,6 @@ hcb_lookup(struct worker *wrk, struct objhead *noh)
u = oh->refcnt;
if (u > 0)
oh->refcnt++;
- else
- with_lock = 1;
Lck_Unlock(&oh->mtx);
if (u > 0)
return (oh);
diff --git a/bin/varnishd/hash/hash_simple_list.c b/bin/varnishd/hash/hash_simple_list.c
index fef2b65..d70e76f 100644
--- a/bin/varnishd/hash/hash_simple_list.c
+++ b/bin/varnishd/hash/hash_simple_list.c
@@ -60,16 +60,19 @@ hsl_start(void)
*/
static struct objhead * __match_proto__(hash_lookup_f)
-hsl_lookup(struct worker *wrk, struct objhead *noh)
+hsl_lookup(struct worker *wrk, const void *digest, struct objhead **noh)
{
struct objhead *oh;
int i;
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
- CHECK_OBJ_NOTNULL(noh, OBJHEAD_MAGIC);
+ AN(digest);
+ AN(noh);
+ CHECK_OBJ_NOTNULL(*noh, OBJHEAD_MAGIC);
+
Lck_Lock(&hsl_mtx);
VTAILQ_FOREACH(oh, &hsl_head, hoh_list) {
- i = memcmp(oh->digest, noh->digest, sizeof oh->digest);
+ i = memcmp(oh->digest, digest, sizeof oh->digest);
if (i < 0)
continue;
if (i > 0)
@@ -80,12 +83,15 @@ hsl_lookup(struct worker *wrk, struct objhead *noh)
}
if (oh != NULL)
- VTAILQ_INSERT_BEFORE(oh, noh, hoh_list);
+ VTAILQ_INSERT_BEFORE(oh, *noh, hoh_list);
else
- VTAILQ_INSERT_TAIL(&hsl_head, noh, hoh_list);
+ VTAILQ_INSERT_TAIL(&hsl_head, *noh, hoh_list);
+ oh = *noh;
+ *noh = NULL;
+ memcpy(oh->digest, digest, sizeof oh->digest);
Lck_Unlock(&hsl_mtx);
- return (noh);
+ return (oh);
}
/*--------------------------------------------------------------------
diff --git a/bin/varnishd/hash/hash_slinger.h b/bin/varnishd/hash/hash_slinger.h
index 4935bfb..628d080 100644
--- a/bin/varnishd/hash/hash_slinger.h
+++ b/bin/varnishd/hash/hash_slinger.h
@@ -35,8 +35,8 @@ struct object;
typedef void hash_init_f(int ac, char * const *av);
typedef void hash_start_f(void);
typedef void hash_prep_f(struct worker *);
-typedef struct objhead *
- hash_lookup_f(struct worker *wrk, struct objhead *nobj);
+typedef struct objhead *hash_lookup_f(struct worker *wrk, const void *digest,
+ struct objhead **nobj);
typedef int hash_deref_f(struct objhead *obj);
struct hash_slinger {
More information about the varnish-commit
mailing list