Re: [RFC PATCH 1/4] hazptr: Add initial implementation of hazard pointers

[Lai Jiangshan <jiangshanlai@gmail.com>]

On Tue, Sep 17, 2024 at 10:34 PM Boqun Feng <boqun.feng@gmail.com> wrote:

> +static void hazptr_context_snap_readers_locked(struct hazptr_reader_tree *tree,
> +                                              struct hazptr_context *hzcp)
> +{
> +       lockdep_assert_held(hzcp->lock);
> +
> +       for (int i = 0; i < HAZPTR_SLOT_PER_CTX; i++) {
> +               /*
> +                * Pairs with smp_store_release() in hazptr_{clear,free}().
> +                *
> +                * Ensure
> +                *
> +                * <reader>             <updater>
> +                *
> +                * [access protected pointers]
> +                * hazptr_clear();
> +                *   smp_store_release()
> +                *                      // in reader scan.
> +                *                      smp_load_acquire(); // is null or unused.
> +                *                      [run callbacks] // all accesses from
> +                *                                      // reader must be
> +                *                                      // observed.
> +                */
> +               hazptr_t val = smp_load_acquire(&hzcp->slots[i]);
> +
> +               if (!is_null_or_unused(val)) {
> +                       struct hazptr_slot_snap *snap = &hzcp->snaps[i];
> +
> +                       // Already in the tree, need to remove first.
> +                       if (!is_null_or_unused(snap->slot)) {
> +                               reader_del(tree, snap);
> +                       }
> +                       snap->slot = val;
> +                       reader_add(tree, snap);
> +               }
> +       }
> +}

Hello

I'm curious about whether there are any possible memory leaks here.

It seems that call_hazptr() never frees the memory until the slot is
set to another valid value.

In the code here, the snap is not deleted when hzcp->snaps[i] is null/unused
and snap->slot is not which I think it should be.

And it can cause unneeded deletion and addition of the snap if the slot
value is unchanged.

I'm not so sure...

Thanks
Lai