On Wed, 10 Aug 2016 15:25:49 -0400 "Emilio G. Cota" <c...@braap.org> wrote:
Emilio, Pls also see https://www.mail-archive.com/qemu-devel@nongnu.org/msg391669.html which fixed issue for me, I've CCed you. > tb_flush() is called when debugging the guest (under both KVM > and TCG accelerators) with gdb. tb_flush() resets TCG's qht, which > segfaults if we're using KVM due to the qht not being initialized. > > Fix this adding a magic number field to struct qht to track whether a qht > has been initialized with qht_init(). Then, explicitly allow > passing uninitialized qht's to qht_reset() and qht_reset_size(), > just like we do with qht_statistics_init(). > > Reported-by: Brent Baccala <cos...@freesoft.org> > Reported-by: Igor Mammedov <imamm...@redhat.com> > Signed-off-by: Emilio G. Cota <c...@braap.org> > --- > include/qemu/qht.h | 7 +++++++ > tests/test-qht.c | 3 +++ > util/qht.c | 20 +++++++++++++++++--- > 3 files changed, 27 insertions(+), 3 deletions(-) > > diff --git a/include/qemu/qht.h b/include/qemu/qht.h > index 311139b..39dd5e8 100644 > --- a/include/qemu/qht.h > +++ b/include/qemu/qht.h > @@ -15,6 +15,7 @@ struct qht { > struct qht_map *map; > QemuMutex lock; /* serializes setters of ht->map */ > unsigned int mode; > + unsigned int magic; > }; > > /** > @@ -124,6 +125,8 @@ bool qht_remove(struct qht *ht, const void *p, uint32_t > hash); > * If concurrent readers may exist, the objects pointed to by the hash table > * must remain valid for the existing RCU grace period -- see qht_remove(). > * See also: qht_reset_size() > + * > + * Note: it is OK to pass an uninitialized @ht. > */ > void qht_reset(struct qht *ht); > > @@ -138,6 +141,8 @@ void qht_reset(struct qht *ht); > * If concurrent readers may exist, the objects pointed to by the hash table > * must remain valid for the existing RCU grace period -- see qht_remove(). > * See also: qht_reset(), qht_resize(). > + * > + * Note: it is OK to pass an uninitialized @ht. > */ > bool qht_reset_size(struct qht *ht, size_t n_elems); > > @@ -173,6 +178,8 @@ void qht_iter(struct qht *ht, qht_iter_func_t func, void > *userp); > * > * When done with @stats, pass the struct to qht_statistics_destroy(). > * Failing to do this will leak memory. > + * > + * Note: it is OK to pass an uninitialized @ht. > */ > void qht_statistics_init(struct qht *ht, struct qht_stats *stats); > > diff --git a/tests/test-qht.c b/tests/test-qht.c > index 46a64b6..a923b2e 100644 > --- a/tests/test-qht.c > +++ b/tests/test-qht.c > @@ -97,6 +97,9 @@ 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); > + /* resetting an uninitialized qht can happen as well, e.g. KVM + gdb */ > + qht_reset(&ht); > + qht_reset_size(&ht, 0); > > qht_init(&ht, 0, mode); > > diff --git a/util/qht.c b/util/qht.c > index 16a8d79..e4c90d6 100644 > --- a/util/qht.c > +++ b/util/qht.c > @@ -89,6 +89,8 @@ > #define QHT_BUCKET_ENTRIES 4 > #endif > > +#define QHT_MAGIC 0xbebec4fe > + > /* > * Note: reading partially-updated pointers in @pointers could lead to > * segfaults. We thus access them with atomic_read/set; this guarantees > @@ -182,6 +184,11 @@ static inline void qht_map_debug__all_locked(struct > qht_map *map) > { } > #endif /* QHT_DEBUG */ > > +static inline bool qht_inited(const struct qht *ht) > +{ > + return ht->magic == QHT_MAGIC; > +} > + > static inline size_t qht_elems_to_buckets(size_t n_elems) > { > return pow2ceil(n_elems / QHT_BUCKET_ENTRIES); > @@ -356,6 +363,7 @@ void qht_init(struct qht *ht, size_t n_elems, unsigned > int mode) > size_t n_buckets = qht_elems_to_buckets(n_elems); > > ht->mode = mode; > + ht->magic = QHT_MAGIC; > qemu_mutex_init(&ht->lock); > map = qht_map_create(n_buckets); > atomic_rcu_set(&ht->map, map); > @@ -403,6 +411,10 @@ void qht_reset(struct qht *ht) > { > struct qht_map *map; > > + if (unlikely(!qht_inited(ht))) { > + return; > + } > + > qht_map_lock_buckets__no_stale(ht, &map); > qht_map_reset__all_locked(map); > qht_map_unlock_buckets(map); > @@ -415,6 +427,9 @@ bool qht_reset_size(struct qht *ht, size_t n_elems) > size_t n_buckets; > bool resize = false; > > + if (unlikely(!qht_inited(ht))) { > + return false; > + } > n_buckets = qht_elems_to_buckets(n_elems); > > qemu_mutex_lock(&ht->lock); > @@ -787,17 +802,16 @@ void qht_statistics_init(struct qht *ht, struct > qht_stats *stats) > struct qht_map *map; > int i; > > - map = atomic_rcu_read(&ht->map); > - > 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)) { > + if (unlikely(!qht_inited(ht))) { > stats->head_buckets = 0; > return; > } > + map = atomic_rcu_read(&ht->map); > stats->head_buckets = map->n_buckets; > > for (i = 0; i < map->n_buckets; i++) {
