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

[Emilio Cota <cota@braap.org>]

On Tue, Jan 10, 2023 at 20:58:01 +0000, Alex Bennée wrote:
> Emilio Cota <cota@braap.org> writes:
(snip)
> > +static inline void qht_do_if_first_in_stripe(const struct qht_map *map,
> > +                                             struct qht_bucket *b,
> > +                                             void (*func)(QemuSpin *spin))
> > +{
> > +#ifdef CONFIG_TSAN
> > +    unsigned long bucket_idx = b - map->buckets;
> > +    bool is_first_in_stripe = (bucket_idx >> QHT_TSAN_BUCKET_LOCKS_BITS) == 0;
> > +    if (is_first_in_stripe) {
> > +        unsigned long lock_idx = bucket_idx & (QHT_TSAN_BUCKET_LOCKS - 1);
> > +        func(&map->tsan_bucket_locks[lock_idx]);
> 
> Hmm I ran into an issue with:
> 
>      ../util/qht.c:286:10: error: incompatible pointer types passing 'const struct qht_tsan_lock *' to parameter of type 'QemuSpin *' (aka 'struct QemuSpin *') [-Werror,-Wincompatible-pointer-types]

Gaah, sorry. I didn't notice this because of unrelated noise due
to having to configure with --disable-werror. Fixed now by removing
a bunch of const's and also using .lock.

> > +static inline void qht_bucket_lock_destroy(const struct qht_map *map,
> > +                                           struct qht_bucket *b)
> > +{
> > +    qht_do_if_first_in_stripe(map, b, qemu_spin_destroy);
> > +}
> 
> Who is meant to be calling this?

Should have been removed in v2; fixed now.

I've uploaded the v3 series to https://github.com/cota/qemu/tree/tsan-v3

Please let me know if you want me to also mail it to the list.
Thanks,

		Emilio