[Qemu-devel] [PATCH] qht: support resetting an uninitialized qht

From: "Emilio G. Cota" <cota@braap.org>
To: Igor Mammedov <imammedo@redhat.com>, Richard Henderson <rth@twiddle.net>
Cc: qemu-devel@nongnu.org, peter.maydell@linaro.org,
	Paolo Bonzini <pbonzini@redhat.com>,
	Peter Crosthwaite <crosthwaite.peter@gmail.com>,
	MTTCG Devel <mttcg@greensocs.com>
Subject: [Qemu-devel] [PATCH] qht: support resetting an uninitialized qht
Date: Wed, 10 Aug 2016 15:25:49 -0400	[thread overview]
Message-ID: <1470857149-32003-1-git-send-email-cota@braap.org> (raw)
In-Reply-To: <20160810153659.4b0172bd@nial.brq.redhat.com>

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 <cosine@freesoft.org>
Reported-by: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Emilio G. Cota <cota@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++) {
-- 
2.5.0
