From 7266ae91a111001abda65c79299c9b7e365456b6 Mon Sep 17 00:00:00 2001 From: "Emilio G. Cota" Date: Fri, 22 Jul 2016 12:36:30 -0400 Subject: [PATCH] qht: do not segfault when gathering stats from an uninitialized qht So far, QHT functions assume that the passed qht has previously been initialized--otherwise they segfault. This patch makes an exception for qht_statistics_init, with the goal of simplifying calling code. For instance, qht_statistics_init is called from the 'info jit' dump, and given that under KVM the TB qht is never initialized, we get a segfault. Thus, instead of complicating the 'info jit' code with additional checks, let's allow passing an uninitialized qht to qht_statistics_init. While at it, add a test for this to test-qht. Before the patch (for $ qemu -enable-kvm [...]): (qemu) info jit [...] direct jump count 0 (0%) (2 jumps=0 0%) Program received signal SIGSEGV, Segmentation fault. After the patch the "TB hash buckets", "TB hash occupancy" and "TB hash avg chain" lines are omitted. (qemu) info jit [...] direct jump count 0 (0%) (2 jumps=0 0%) TB hash buckets 0/0 (-nan% head buckets used) TB hash occupancy nan% avg chain occ. Histogram: (null) TB hash avg chain nan buckets. Histogram: (null) [...] Reported by: Changlong Xie Signed-off-by: Emilio G. Cota Message-Id: <1469205390-14369-1-git-send-email-cota@braap.org> [Extract printing statistics to an entirely separate function. - Paolo] Signed-off-by: Paolo Bonzini --- tests/test-qht.c | 4 +++ translate-all.c | 70 +++++++++++++++++++++++++++--------------------- util/qht.c | 7 ++++- 3 files changed, 49 insertions(+), 32 deletions(-) diff --git a/tests/test-qht.c b/tests/test-qht.c index f1d628371d..46a64b6731 100644 --- a/tests/test-qht.c +++ b/tests/test-qht.c @@ -95,8 +95,12 @@ static void iter_check(unsigned int count) static void qht_do_test(unsigned int mode, size_t init_entries) { + /* under KVM we might fetch stats from an uninitialized qht */ + check_n(0); + qht_init(&ht, 0, mode); + check_n(0); insert(0, N); check(0, N, true); check_n(N); diff --git a/translate-all.c b/translate-all.c index 0d47c1c0cf..efeba298b9 100644 --- a/translate-all.c +++ b/translate-all.c @@ -1663,15 +1663,50 @@ void tb_flush_jmp_cache(CPUState *cpu, target_ulong addr) TB_JMP_PAGE_SIZE * sizeof(TranslationBlock *)); } +static void print_qht_statistics(FILE *f, fprintf_function cpu_fprintf, + struct qht_stats hst) +{ + uint32_t hgram_opts; + size_t hgram_bins; + char *hgram; + + if (!hst.head_buckets) { + return; + } + cpu_fprintf(f, "TB hash buckets %zu/%zu (%0.2f%% head buckets used)\n", + hst.used_head_buckets, hst.head_buckets, + (double)hst.used_head_buckets / hst.head_buckets * 100); + + hgram_opts = QDIST_PR_BORDER | QDIST_PR_LABELS; + hgram_opts |= QDIST_PR_100X | QDIST_PR_PERCENT; + if (qdist_xmax(&hst.occupancy) - qdist_xmin(&hst.occupancy) == 1) { + hgram_opts |= QDIST_PR_NODECIMAL; + } + hgram = qdist_pr(&hst.occupancy, 10, hgram_opts); + cpu_fprintf(f, "TB hash occupancy %0.2f%% avg chain occ. Histogram: %s\n", + qdist_avg(&hst.occupancy) * 100, hgram); + g_free(hgram); + + hgram_opts = QDIST_PR_BORDER | QDIST_PR_LABELS; + hgram_bins = qdist_xmax(&hst.chain) - qdist_xmin(&hst.chain); + if (hgram_bins > 10) { + hgram_bins = 10; + } else { + hgram_bins = 0; + hgram_opts |= QDIST_PR_NODECIMAL | QDIST_PR_NOBINRANGE; + } + hgram = qdist_pr(&hst.chain, hgram_bins, hgram_opts); + cpu_fprintf(f, "TB hash avg chain %0.3f buckets. Histogram: %s\n", + qdist_avg(&hst.chain), hgram); + g_free(hgram); +} + void dump_exec_info(FILE *f, fprintf_function cpu_fprintf) { int i, target_code_size, max_target_code_size; int direct_jmp_count, direct_jmp2_count, cross_page; TranslationBlock *tb; struct qht_stats hst; - uint32_t hgram_opts; - size_t hgram_bins; - char *hgram; target_code_size = 0; max_target_code_size = 0; @@ -1724,34 +1759,7 @@ void dump_exec_info(FILE *f, fprintf_function cpu_fprintf) tcg_ctx.tb_ctx.nb_tbs : 0); qht_statistics_init(&tcg_ctx.tb_ctx.htable, &hst); - - cpu_fprintf(f, "TB hash buckets %zu/%zu (%0.2f%% head buckets used)\n", - hst.used_head_buckets, hst.head_buckets, - (double)hst.used_head_buckets / hst.head_buckets * 100); - - hgram_opts = QDIST_PR_BORDER | QDIST_PR_LABELS; - hgram_opts |= QDIST_PR_100X | QDIST_PR_PERCENT; - if (qdist_xmax(&hst.occupancy) - qdist_xmin(&hst.occupancy) == 1) { - hgram_opts |= QDIST_PR_NODECIMAL; - } - hgram = qdist_pr(&hst.occupancy, 10, hgram_opts); - cpu_fprintf(f, "TB hash occupancy %0.2f%% avg chain occ. Histogram: %s\n", - qdist_avg(&hst.occupancy) * 100, hgram); - g_free(hgram); - - hgram_opts = QDIST_PR_BORDER | QDIST_PR_LABELS; - hgram_bins = qdist_xmax(&hst.chain) - qdist_xmin(&hst.chain); - if (hgram_bins > 10) { - hgram_bins = 10; - } else { - hgram_bins = 0; - hgram_opts |= QDIST_PR_NODECIMAL | QDIST_PR_NOBINRANGE; - } - hgram = qdist_pr(&hst.chain, hgram_bins, hgram_opts); - cpu_fprintf(f, "TB hash avg chain %0.3f buckets. Histogram: %s\n", - qdist_avg(&hst.chain), hgram); - g_free(hgram); - + print_qht_statistics(f, cpu_fprintf, hst); qht_statistics_destroy(&hst); cpu_fprintf(f, "\nStatistics:\n"); diff --git a/util/qht.c b/util/qht.c index 28ce289245..16a8d7950e 100644 --- a/util/qht.c +++ b/util/qht.c @@ -789,11 +789,16 @@ void qht_statistics_init(struct qht *ht, struct qht_stats *stats) map = atomic_rcu_read(&ht->map); - stats->head_buckets = map->n_buckets; stats->used_head_buckets = 0; stats->entries = 0; qdist_init(&stats->chain); qdist_init(&stats->occupancy); + /* bail out if the qht has not yet been initialized */ + if (unlikely(map == NULL)) { + stats->head_buckets = 0; + return; + } + stats->head_buckets = map->n_buckets; for (i = 0; i < map->n_buckets; i++) { struct qht_bucket *head = &map->buckets[i];