From d52d7aa8714929fc0f26e605aafa38461213e606 Mon Sep 17 00:00:00 2001 From: Attilio Rao Date: Thu, 21 Mar 2013 19:58:25 +0000 Subject: [PATCH] Fix a bug in UMTX_PROFILING: UMTX_PROFILING should really analyze the distribution of locks as they index entries in the umtxq_chains hash-table. However, the current implementation does add/dec the length counters for *every* thread insert/removal, measuring at all really userland contention and not the hash distribution. Fix this by correctly add/dec the length counters in the points where it is really needed. Please note that this bug brought us questioning in the past the quality of the umtx hash table distribution. To date with all the benchmarks I could try I was not able to reproduce any issue about the hash distribution on umtx. Sponsored by: EMC / Isilon storage division Reviewed by: jeff, davide MFC after: 2 weeks --- sys/kern/kern_umtx.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/sys/kern/kern_umtx.c b/sys/kern/kern_umtx.c index f50a6f90ac81..6026b8080dbf 100644 --- a/sys/kern/kern_umtx.c +++ b/sys/kern/kern_umtx.c @@ -542,19 +542,19 @@ umtxq_insert_queue(struct umtx_q *uq, int q) uh = uq->uq_spare_queue; uh->key = uq->uq_key; LIST_INSERT_HEAD(&uc->uc_queue[q], uh, link); +#ifdef UMTX_PROFILING + uc->length++; + if (uc->length > uc->max_length) { + uc->max_length = uc->length; + if (uc->max_length > max_length) + max_length = uc->max_length; + } +#endif } uq->uq_spare_queue = NULL; TAILQ_INSERT_TAIL(&uh->head, uq, uq_link); uh->length++; -#ifdef UMTX_PROFILING - uc->length++; - if (uc->length > uc->max_length) { - uc->max_length = uc->length; - if (uc->max_length > max_length) - max_length = uc->max_length; - } -#endif uq->uq_flags |= UQF_UMTXQ; uq->uq_cur_queue = uh; return; @@ -572,13 +572,13 @@ umtxq_remove_queue(struct umtx_q *uq, int q) uh = uq->uq_cur_queue; TAILQ_REMOVE(&uh->head, uq, uq_link); uh->length--; -#ifdef UMTX_PROFILING - uc->length--; -#endif uq->uq_flags &= ~UQF_UMTXQ; if (TAILQ_EMPTY(&uh->head)) { KASSERT(uh->length == 0, ("inconsistent umtxq_queue length")); +#ifdef UMTX_PROFILING + uc->length--; +#endif LIST_REMOVE(uh, link); } else { uh = LIST_FIRST(&uc->uc_spare_queue);