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
