On Sun, Jan 08, 2023 at 11:51:44 -0800, Richard Henderson wrote:
> 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.
Done, added a separate patch for this.
> 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?
Thought the remaining two cases were obvious enough.
I've now removed the wrappers.
> The actual logic of the stripe seems ok.
Thanks!
Emilio
