On 22/07/2016 18:36, Emilio G. Cota wrote:
> 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:
> (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 <xiecl.f...@cn.fujitsu.com>
> Signed-off-by: Emilio G. Cota <c...@braap.org>
> ---
> tests/test-qht.c | 4 ++++
> util/qht.c | 7 ++++++-
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/tests/test-qht.c b/tests/test-qht.c
> index c8eb930..e2f9e14 100644
> --- a/tests/test-qht.c
> +++ b/tests/test-qht.c
> @@ -96,8 +96,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/util/qht.c b/util/qht.c
> index 6f74909..0cb95e2 100644
> --- a/util/qht.c
> +++ b/util/qht.c
> @@ -783,11 +783,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];
>
Queued, thanks.
Paolo
