Re: [PATCH 1/1] refcount: provide ops for cases when object's memory can be reused

[Vlastimil Babka]

On 2/6/25 03:52, Suren Baghdasaryan wrote:
> For speculative lookups where a successful inc_not_zero() pins the
> object, but where we still need to double check if the object acquired
> is indeed the one we set out to acquire (identity check), needs this
> validation to happen *after* the increment.
> Similarly, when a new object is initialized and its memory might have
> been previously occupied by another object, all stores to initialize the
> object should happen *before* refcount initialization.
> 
> Notably SLAB_TYPESAFE_BY_RCU is one such an example when this ordering
> is required for reference counting.
> 
> Add refcount_{add|inc}_not_zero_acquire() to guarantee the proper ordering
> between acquiring a reference count on an object and performing the
> identity check for that object.
> Add refcount_set_release() to guarantee proper ordering between stores
> initializing object attributes and the store initializing the refcount.
> refcount_set_release() should be done after all other object attributes
> are initialized. Once refcount_set_release() is called, the object should
> be considered visible to other tasks even if it was not yet added into an
> object collection normally used to discover it. This is because other
> tasks might have discovered the object previously occupying the same
> memory and after memory reuse they can succeed in taking refcount for the
> new object and start using it.
> 
> Object reuse example to consider:
> 
> consumer:
>     obj = lookup(collection, key);
>     if (!refcount_inc_not_zero_acquire(&obj->ref))
>         return;
>     if (READ_ONCE(obj->key) != key) { /* identity check */
>         put_ref(obj);
>         return;
>     }
>     use(obj->value);
> 
>                  producer:
>                      remove(collection, obj->key);
>                      if (!refcount_dec_and_test(&obj->ref))
>                          return;
>                      obj->key = KEY_INVALID;
>                      free(obj);
>                      obj = malloc(); /* obj is reused */
>                      obj->key = new_key;
>                      obj->value = new_value;
>                      refcount_set_release(obj->ref, 1);
>                      add(collection, new_key, obj);
> 
> refcount_{add|inc}_not_zero_acquire() is required to prevent the following
> reordering when refcount_inc_not_zero() is used instead:
> 
> consumer:
>     obj = lookup(collection, key);
>     if (READ_ONCE(obj->key) != key) { /* reordered identity check */
>         put_ref(obj);
>         return;
>     }
>                  producer:
>                      remove(collection, obj->key);
>                      if (!refcount_dec_and_test(&obj->ref))
>                          return;
>                      obj->key = KEY_INVALID;
>                      free(obj);
>                      obj = malloc(); /* obj is reused */
>                      obj->key = new_key;
>                      obj->value = new_value;
>                      refcount_set_release(obj->ref, 1);
>                      add(collection, new_key, obj);
> 
>     if (!refcount_inc_not_zero(&obj->ref))
>         return;
>     use(obj->value); /* USING WRONG OBJECT */
> 
> refcount_set_release() is required to prevent the following reordering
> when refcount_set() is used instead:
> 
> consumer:
>     obj = lookup(collection, key);
> 
>                  producer:
>                      remove(collection, obj->key);
>                      if (!refcount_dec_and_test(&obj->ref))
>                          return;
>                      obj->key = KEY_INVALID;
>                      free(obj);
>                      obj = malloc(); /* obj is reused */
>                      obj->key = new_key; /* new_key == old_key */
>                      refcount_set(obj->ref, 1);
> 
>     if (!refcount_inc_not_zero_acquire(&obj->ref))
>         return;
>     if (READ_ONCE(obj->key) != key) { /* pass since new_key == old_key */
>         put_ref(obj);
>         return;
>     }
>     use(obj->value); /* USING STALE obj->value */
> 
>                      obj->value = new_value; /* reordered store */
>                      add(collection, key, obj);
> 
> Signed-off-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Will Deacon <will@xxxxxxxxxx>
> Cc: Paul E. McKenney <paulmck@xxxxxxxxxx>

Acked-by: Vlastimil Babka <vbabka@xxxxxxx> #slab