Message-ID: <d203b3bd-eae0-4114-a243-212db99339ba@suse.cz>
Date: Thu, 6 Feb 2025 11:41:57 +0100
From: Vlastimil Babka <vbabka@...e.cz>
To: Suren Baghdasaryan <surenb@...gle.com>, peterz@...radead.org
Cc: will@...nel.org, paulmck@...nel.org, boqun.feng@...il.com,
 mark.rutland@....com, mjguzik@...il.com, akpm@...ux-foundation.org,
 willy@...radead.org, liam.howlett@...cle.com, lorenzo.stoakes@...cle.com,
 david.laight.linux@...il.com, mhocko@...e.com, hannes@...xchg.org,
 oliver.sang@...el.com, mgorman@...hsingularity.net, david@...hat.com,
 peterx@...hat.com, oleg@...hat.com, dave@...olabs.net, brauner@...nel.org,
 dhowells@...hat.com, hdanton@...a.com, hughd@...gle.com,
 lokeshgidra@...gle.com, minchan@...gle.com, jannh@...gle.com,
 shakeel.butt@...ux.dev, souravpanda@...gle.com, pasha.tatashin@...een.com,
 klarasmodin@...il.com, richard.weiyang@...il.com, corbet@....net,
 linux-doc@...r.kernel.org, linux-mm@...ck.org, linux-kernel@...r.kernel.org,
 kernel-team@...roid.com
Subject: Re: [PATCH 1/1] refcount: provide ops for cases when object's memory
 can be reused

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@...gle.com>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Will Deacon <will@...nel.org>
> Cc: Paul E. McKenney <paulmck@...nel.org>

Acked-by: Vlastimil Babka <vbabka@...e.cz> #slab

Powered by blists - more mailing lists