Re: [PATCH 3/4] util/qht: use striped locks under TSAN

[Richard Henderson]

On 1/8/23 08:39, Emilio Cota wrote:

+static inline void qht_bucket_lock_init(const struct qht_map *map,
+                                        struct qht_bucket *b)
+{
+    qht_do_if_first_in_stripe(map, b, qemu_spin_init);
+}
+
+static inline void qht_bucket_lock_destroy(const struct qht_map *map,
+                                           struct qht_bucket *b)
+{
+    qht_do_if_first_in_stripe(map, b, (void (*)(QemuSpin *))qemu_spin_destroy);
+}

I think it's clear the declaration of qemu_spin_destroy is wrong vs const.

Why do you create two wrappers for qht_do_if_first_in_stripe, but...

+static inline void qht_head_init(struct qht_map *map, struct qht_bucket *b)
  {
      memset(b, 0, sizeof(*b));
-    qemu_spin_init(&b->lock);
+    qht_bucket_lock_init(map, b);
      seqlock_init(&b->sequence);
  }

@@ -250,7 +328,7 @@ static void qht_map_lock_buckets(struct qht_map *map)

      for (i = 0; i < map->n_buckets; i++) {
          struct qht_bucket *b = &map->buckets[i];

- qemu_spin_lock(&b->lock);

+        qht_do_if_first_in_stripe(map, b, qemu_spin_lock);
      }

... not others?

The actual logic of the stripe seems ok.


r~