[PATCH 0/2] rcu: Fix rculist_nulls and doc

[PATCH 0/2] rcu: Fix rculist_nulls and doc

[Alan Huang]

Fix cases where using rculist_nulls combined with SLAB_TYPESAFE_BY_RCU.

Alan Huang (2):
  rcu: Use WRITE_ONCE() for assignments to ->next for rculist_nulls
  docs/RCU: Bring back smp_wmb()

 Documentation/RCU/rculist_nulls.rst | 9 +++++++--
 include/linux/rculist_nulls.h       | 4 ++--
 2 files changed, 9 insertions(+), 4 deletions(-)

-- 
2.34.1

[PATCH 1/2] rcu: Use WRITE_ONCE() for assignments to ->next for rculist_nulls

[Alan Huang]

When the objects managed by rculist_nulls are allocated with
SLAB_TYPESAFE_BY_RCU, readers may still hold references to this
object that is being added, which means the modification of ->next
is visible to readers. So, this patch uses WRITE_ONCE() for assignments
to ->next.

Signed-off-by: Alan Huang <mmpgouride@gmail.com>
---
 include/linux/rculist_nulls.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/rculist_nulls.h b/include/linux/rculist_nulls.h
index ba4c00dd8005..89186c499dd4 100644
--- a/include/linux/rculist_nulls.h
+++ b/include/linux/rculist_nulls.h
@@ -101,7 +101,7 @@ static inline void hlist_nulls_add_head_rcu(struct hlist_nulls_node *n,
 {
 	struct hlist_nulls_node *first = h->first;
 
-	n->next = first;
+	WRITE_ONCE(n->next, first);
 	WRITE_ONCE(n->pprev, &h->first);
 	rcu_assign_pointer(hlist_nulls_first_rcu(h), n);
 	if (!is_a_nulls(first))
@@ -137,7 +137,7 @@ static inline void hlist_nulls_add_tail_rcu(struct hlist_nulls_node *n,
 		last = i;
 
 	if (last) {
-		n->next = last->next;
+		WRITE_ONCE(n->next, last->next);
 		n->pprev = &last->next;
 		rcu_assign_pointer(hlist_nulls_next_rcu(last), n);
 	} else {
-- 
2.34.1

Re: [PATCH 1/2] rcu: Use WRITE_ONCE() for assignments to ->next for rculist_nulls

[Paul E. McKenney]

On Thu, Jul 06, 2023 at 10:28:48AM +0000, Alan Huang wrote:
> When the objects managed by rculist_nulls are allocated with
> SLAB_TYPESAFE_BY_RCU, readers may still hold references to this
> object that is being added, which means the modification of ->next
> is visible to readers. So, this patch uses WRITE_ONCE() for assignments
> to ->next.
> 
> Signed-off-by: Alan Huang <mmpgouride@gmail.com>

Very good, queued for the v6.6 merge window, thank you!

							Thanx, Paul

> ---
>  include/linux/rculist_nulls.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/rculist_nulls.h b/include/linux/rculist_nulls.h
> index ba4c00dd8005..89186c499dd4 100644
> --- a/include/linux/rculist_nulls.h
> +++ b/include/linux/rculist_nulls.h
> @@ -101,7 +101,7 @@ static inline void hlist_nulls_add_head_rcu(struct hlist_nulls_node *n,
>  {
>  	struct hlist_nulls_node *first = h->first;
>  
> -	n->next = first;
> +	WRITE_ONCE(n->next, first);
>  	WRITE_ONCE(n->pprev, &h->first);
>  	rcu_assign_pointer(hlist_nulls_first_rcu(h), n);
>  	if (!is_a_nulls(first))
> @@ -137,7 +137,7 @@ static inline void hlist_nulls_add_tail_rcu(struct hlist_nulls_node *n,
>  		last = i;
>  
>  	if (last) {
> -		n->next = last->next;
> +		WRITE_ONCE(n->next, last->next);
>  		n->pprev = &last->next;
>  		rcu_assign_pointer(hlist_nulls_next_rcu(last), n);
>  	} else {
> -- 
> 2.34.1
> 

Re: [PATCH 1/2] rcu: Use WRITE_ONCE() for assignments to ->next for rculist_nulls

[Joel Fernandes]

On Thu, Jul 6, 2023 at 6:29 AM Alan Huang <mmpgouride@gmail.com> wrote:
>
> When the objects managed by rculist_nulls are allocated with
> SLAB_TYPESAFE_BY_RCU, readers may still hold references to this
> object that is being added, which means the modification of ->next
> is visible to readers. So, this patch uses WRITE_ONCE() for assignments
> to ->next.
>
> Signed-off-by: Alan Huang <mmpgouride@gmail.com>
> ---
>  include/linux/rculist_nulls.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/rculist_nulls.h b/include/linux/rculist_nulls.h
> index ba4c00dd8005..89186c499dd4 100644
> --- a/include/linux/rculist_nulls.h
> +++ b/include/linux/rculist_nulls.h
> @@ -101,7 +101,7 @@ static inline void hlist_nulls_add_head_rcu(struct hlist_nulls_node *n,
>  {
>         struct hlist_nulls_node *first = h->first;
>
> -       n->next = first;
> +       WRITE_ONCE(n->next, first);
>         WRITE_ONCE(n->pprev, &h->first);
>         rcu_assign_pointer(hlist_nulls_first_rcu(h), n);
>         if (!is_a_nulls(first))
> @@ -137,7 +137,7 @@ static inline void hlist_nulls_add_tail_rcu(struct hlist_nulls_node *n,
>                 last = i;
>
>         if (last) {
> -               n->next = last->next;
> +               WRITE_ONCE(n->next, last->next);
>                 n->pprev = &last->next;
>                 rcu_assign_pointer(hlist_nulls_next_rcu(last), n);
>         } else {

Don't you need READ_ONCE() for the read-side accesses as well?

thanks,

 - Joel

Re: [PATCH 1/2] rcu: Use WRITE_ONCE() for assignments to ->next for rculist_nulls

[Paul E. McKenney]

On Mon, Jul 10, 2023 at 04:01:07PM -0400, Joel Fernandes wrote:
> On Thu, Jul 6, 2023 at 6:29 AM Alan Huang <mmpgouride@gmail.com> wrote:
> >
> > When the objects managed by rculist_nulls are allocated with
> > SLAB_TYPESAFE_BY_RCU, readers may still hold references to this
> > object that is being added, which means the modification of ->next
> > is visible to readers. So, this patch uses WRITE_ONCE() for assignments
> > to ->next.
> >
> > Signed-off-by: Alan Huang <mmpgouride@gmail.com>
> > ---
> >  include/linux/rculist_nulls.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/rculist_nulls.h b/include/linux/rculist_nulls.h
> > index ba4c00dd8005..89186c499dd4 100644
> > --- a/include/linux/rculist_nulls.h
> > +++ b/include/linux/rculist_nulls.h
> > @@ -101,7 +101,7 @@ static inline void hlist_nulls_add_head_rcu(struct hlist_nulls_node *n,
> >  {
> >         struct hlist_nulls_node *first = h->first;
> >
> > -       n->next = first;
> > +       WRITE_ONCE(n->next, first);
> >         WRITE_ONCE(n->pprev, &h->first);
> >         rcu_assign_pointer(hlist_nulls_first_rcu(h), n);
> >         if (!is_a_nulls(first))
> > @@ -137,7 +137,7 @@ static inline void hlist_nulls_add_tail_rcu(struct hlist_nulls_node *n,
> >                 last = i;
> >
> >         if (last) {
> > -               n->next = last->next;
> > +               WRITE_ONCE(n->next, last->next);
> >                 n->pprev = &last->next;
> >                 rcu_assign_pointer(hlist_nulls_next_rcu(last), n);
> >         } else {
> 
> Don't you need READ_ONCE() for the read-side accesses as well?

Both hlist_nulls_for_each_entry_rcu() and hlist_nulls_for_each_entry_safe()
use rcu_dereference_raw().  To your point, both hlist_nulls_first_rcu()
and hlist_nulls_next_rcu() look rather strange.  I would have expected
them to also use rcu_dereference_raw().

							Thanx, Paul

Re: [PATCH 1/2] rcu: Use WRITE_ONCE() for assignments to ->next for rculist_nulls

[Alan Huang]

> 2023年7月11日 04:30，Paul E. McKenney <paulmck@kernel.org> 写道：
> 
> On Mon, Jul 10, 2023 at 04:01:07PM -0400, Joel Fernandes wrote:
>> On Thu, Jul 6, 2023 at 6:29 AM Alan Huang <mmpgouride@gmail.com> wrote:
>>> 
>>> When the objects managed by rculist_nulls are allocated with
>>> SLAB_TYPESAFE_BY_RCU, readers may still hold references to this
>>> object that is being added, which means the modification of ->next
>>> is visible to readers. So, this patch uses WRITE_ONCE() for assignments
>>> to ->next.
>>> 
>>> Signed-off-by: Alan Huang <mmpgouride@gmail.com>
>>> ---
>>> include/linux/rculist_nulls.h | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/include/linux/rculist_nulls.h b/include/linux/rculist_nulls.h
>>> index ba4c00dd8005..89186c499dd4 100644
>>> --- a/include/linux/rculist_nulls.h
>>> +++ b/include/linux/rculist_nulls.h
>>> @@ -101,7 +101,7 @@ static inline void hlist_nulls_add_head_rcu(struct hlist_nulls_node *n,
>>> {
>>>        struct hlist_nulls_node *first = h->first;
>>> 
>>> -       n->next = first;
>>> +       WRITE_ONCE(n->next, first);
>>>        WRITE_ONCE(n->pprev, &h->first);
>>>        rcu_assign_pointer(hlist_nulls_first_rcu(h), n);
>>>        if (!is_a_nulls(first))
>>> @@ -137,7 +137,7 @@ static inline void hlist_nulls_add_tail_rcu(struct hlist_nulls_node *n,
>>>                last = i;
>>> 
>>>        if (last) {
>>> -               n->next = last->next;
>>> +               WRITE_ONCE(n->next, last->next);
>>>                n->pprev = &last->next;
>>>                rcu_assign_pointer(hlist_nulls_next_rcu(last), n);
>>>        } else {
>> 
>> Don't you need READ_ONCE() for the read-side accesses as well?
> 
> Both hlist_nulls_for_each_entry_rcu() and hlist_nulls_for_each_entry_safe()
> use rcu_dereference_raw().  To your point, both hlist_nulls_first_rcu()
> and hlist_nulls_next_rcu() look rather strange.  I would have expected
> them to also use rcu_dereference_raw().

hlist_nulls_first_rcu() and hlist_nulls_next_rcu() were added in commit:

	67bdbffd696f(“rculist: avoid __rcu annotations”)

According to the commit message, this avoids warnings from missing __rcu annotations.

> 
> Thanx, Paul

Re: [PATCH 1/2] rcu: Use WRITE_ONCE() for assignments to ->next for rculist_nulls

[Paul E. McKenney]

On Tue, Jul 11, 2023 at 10:56:02PM +0800, Alan Huang wrote:
> 
> > 2023年7月11日 04:30，Paul E. McKenney <paulmck@kernel.org> 写道：
> > 
> > On Mon, Jul 10, 2023 at 04:01:07PM -0400, Joel Fernandes wrote:
> >> On Thu, Jul 6, 2023 at 6:29 AM Alan Huang <mmpgouride@gmail.com> wrote:
> >>> 
> >>> When the objects managed by rculist_nulls are allocated with
> >>> SLAB_TYPESAFE_BY_RCU, readers may still hold references to this
> >>> object that is being added, which means the modification of ->next
> >>> is visible to readers. So, this patch uses WRITE_ONCE() for assignments
> >>> to ->next.
> >>> 
> >>> Signed-off-by: Alan Huang <mmpgouride@gmail.com>
> >>> ---
> >>> include/linux/rculist_nulls.h | 4 ++--
> >>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>> 
> >>> diff --git a/include/linux/rculist_nulls.h b/include/linux/rculist_nulls.h
> >>> index ba4c00dd8005..89186c499dd4 100644
> >>> --- a/include/linux/rculist_nulls.h
> >>> +++ b/include/linux/rculist_nulls.h
> >>> @@ -101,7 +101,7 @@ static inline void hlist_nulls_add_head_rcu(struct hlist_nulls_node *n,
> >>> {
> >>>        struct hlist_nulls_node *first = h->first;
> >>> 
> >>> -       n->next = first;
> >>> +       WRITE_ONCE(n->next, first);
> >>>        WRITE_ONCE(n->pprev, &h->first);
> >>>        rcu_assign_pointer(hlist_nulls_first_rcu(h), n);
> >>>        if (!is_a_nulls(first))
> >>> @@ -137,7 +137,7 @@ static inline void hlist_nulls_add_tail_rcu(struct hlist_nulls_node *n,
> >>>                last = i;
> >>> 
> >>>        if (last) {
> >>> -               n->next = last->next;
> >>> +               WRITE_ONCE(n->next, last->next);
> >>>                n->pprev = &last->next;
> >>>                rcu_assign_pointer(hlist_nulls_next_rcu(last), n);
> >>>        } else {
> >> 
> >> Don't you need READ_ONCE() for the read-side accesses as well?
> > 
> > Both hlist_nulls_for_each_entry_rcu() and hlist_nulls_for_each_entry_safe()
> > use rcu_dereference_raw().  To your point, both hlist_nulls_first_rcu()
> > and hlist_nulls_next_rcu() look rather strange.  I would have expected
> > them to also use rcu_dereference_raw().
> 
> hlist_nulls_first_rcu() and hlist_nulls_next_rcu() were added in commit:
> 
> 	67bdbffd696f(“rculist: avoid __rcu annotations”)
> 
> According to the commit message, this avoids warnings from missing __rcu annotations.

Indeed it does, apologies for the noise!

							Thanx, Paul

Re: [PATCH 1/2] rcu: Use WRITE_ONCE() for assignments to ->next for rculist_nulls

[Alan Huang]

> 2023年7月11日 04:01，Joel Fernandes <joel@joelfernandes.org> 写道：
> 
> On Thu, Jul 6, 2023 at 6:29 AM Alan Huang <mmpgouride@gmail.com> wrote:
>> 
>> When the objects managed by rculist_nulls are allocated with
>> SLAB_TYPESAFE_BY_RCU, readers may still hold references to this
>> object that is being added, which means the modification of ->next
>> is visible to readers. So, this patch uses WRITE_ONCE() for assignments
>> to ->next.
>> 
>> Signed-off-by: Alan Huang <mmpgouride@gmail.com>
>> ---
>> include/linux/rculist_nulls.h | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/include/linux/rculist_nulls.h b/include/linux/rculist_nulls.h
>> index ba4c00dd8005..89186c499dd4 100644
>> --- a/include/linux/rculist_nulls.h
>> +++ b/include/linux/rculist_nulls.h
>> @@ -101,7 +101,7 @@ static inline void hlist_nulls_add_head_rcu(struct hlist_nulls_node *n,
>> {
>>        struct hlist_nulls_node *first = h->first;
>> 
>> -       n->next = first;
>> +       WRITE_ONCE(n->next, first);
>>        WRITE_ONCE(n->pprev, &h->first);
>>        rcu_assign_pointer(hlist_nulls_first_rcu(h), n);
>>        if (!is_a_nulls(first))
>> @@ -137,7 +137,7 @@ static inline void hlist_nulls_add_tail_rcu(struct hlist_nulls_node *n,
>>                last = i;
>> 
>>        if (last) {
>> -               n->next = last->next;
>> +               WRITE_ONCE(n->next, last->next);
>>                n->pprev = &last->next;
>>                rcu_assign_pointer(hlist_nulls_next_rcu(last), n);
>>        } else {
> 
> Don't you need READ_ONCE() for the read-side accesses as well?

Like Paul said, read-side uses rcu_dereference_raw()

> 
> thanks,
> 
> - Joel

[PATCH 2/2] docs/RCU: Bring back smp_wmb()

[Alan Huang]

The objects are allocated with SLAB_TYPESAFE_BY_RCU, and there is
n->next = first within hlist_add_head_rcu() before rcu_assign_pointer(),
which modifies obj->obj_node.next. There may be readers holding the
reference of obj in lockless_lookup, and when updater modifies ->next,
readers can see the change immediately because of SLAB_TYPESAFE_BY_RCU.

There are two memory ordering required in the insertion algorithm,
we need to make sure obj->key is updated before obj->obj_node.next
and obj->refcnt, atomic_set_release is not enough to provide the
required memory barrier.

Signed-off-by: Alan Huang <mmpgouride@gmail.com>
---
 Documentation/RCU/rculist_nulls.rst | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/RCU/rculist_nulls.rst b/Documentation/RCU/rculist_nulls.rst
index 21e40fcc08de..fa3729dc7e74 100644
--- a/Documentation/RCU/rculist_nulls.rst
+++ b/Documentation/RCU/rculist_nulls.rst
@@ -64,7 +64,7 @@ but a version with an additional memory barrier (smp_rmb())
   {
     struct hlist_node *node, *next;
     for (pos = rcu_dereference((head)->first);
-         pos && ({ next = pos->next; smp_rmb(); prefetch(next); 1; }) &&
+         pos && ({ next = READ_ONCE(pos->next); smp_rmb(); prefetch(next); 1; }) &&
          ({ obj = hlist_entry(pos, typeof(*obj), obj_node); 1; });
          pos = rcu_dereference(next))
       if (obj->key == key)
@@ -112,7 +112,12 @@ detect the fact that it missed following items in original chain.
   obj = kmem_cache_alloc(...);
   lock_chain(); // typically a spin_lock()
   obj->key = key;
-  atomic_set_release(&obj->refcnt, 1); // key before refcnt
+  /*
+   * We need to make sure obj->key is updated before obj->obj_node.next
+   * and obj->refcnt.
+   */
+  smp_wmb();
+  atomic_set(&obj->refcnt, 1);
   hlist_add_head_rcu(&obj->obj_node, list);
   unlock_chain(); // typically a spin_unlock()
 
-- 
2.34.1

Re: [PATCH 2/2] docs/RCU: Bring back smp_wmb()

[Paul E. McKenney]

On Thu, Jul 06, 2023 at 10:28:49AM +0000, Alan Huang wrote:
> The objects are allocated with SLAB_TYPESAFE_BY_RCU, and there is
> n->next = first within hlist_add_head_rcu() before rcu_assign_pointer(),
> which modifies obj->obj_node.next. There may be readers holding the
> reference of obj in lockless_lookup, and when updater modifies ->next,
> readers can see the change immediately because of SLAB_TYPESAFE_BY_RCU.
> 
> There are two memory ordering required in the insertion algorithm,
> we need to make sure obj->key is updated before obj->obj_node.next
> and obj->refcnt, atomic_set_release is not enough to provide the
> required memory barrier.
> 
> Signed-off-by: Alan Huang <mmpgouride@gmail.com>
> ---
>  Documentation/RCU/rculist_nulls.rst | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/RCU/rculist_nulls.rst b/Documentation/RCU/rculist_nulls.rst
> index 21e40fcc08de..fa3729dc7e74 100644
> --- a/Documentation/RCU/rculist_nulls.rst
> +++ b/Documentation/RCU/rculist_nulls.rst
> @@ -64,7 +64,7 @@ but a version with an additional memory barrier (smp_rmb())
>    {
>      struct hlist_node *node, *next;
>      for (pos = rcu_dereference((head)->first);
> -         pos && ({ next = pos->next; smp_rmb(); prefetch(next); 1; }) &&
> +         pos && ({ next = READ_ONCE(pos->next); smp_rmb(); prefetch(next); 1; }) &&

This one looks good, though the READ_ONCE() becoming rcu_dereference()
would allow the smp_rmb() to be dropped, correct?

>           ({ obj = hlist_entry(pos, typeof(*obj), obj_node); 1; });
>           pos = rcu_dereference(next))
>        if (obj->key == key)
> @@ -112,7 +112,12 @@ detect the fact that it missed following items in original chain.
>    obj = kmem_cache_alloc(...);
>    lock_chain(); // typically a spin_lock()
>    obj->key = key;
> -  atomic_set_release(&obj->refcnt, 1); // key before refcnt
> +  /*
> +   * We need to make sure obj->key is updated before obj->obj_node.next
> +   * and obj->refcnt.
> +   */
> +  smp_wmb();
> +  atomic_set(&obj->refcnt, 1);

...but what is smp_wmb() doing that the combination of atomic_set_release()
and hlist_add_head_rcu() was not already doing?  What am I missing?

							Thanx, Paul

>    hlist_add_head_rcu(&obj->obj_node, list);
>    unlock_chain(); // typically a spin_unlock()
>  
> -- 
> 2.34.1
> 

Re: [PATCH 2/2] docs/RCU: Bring back smp_wmb()

[Alan Huang]

> 2023年7月11日 03:11，Paul E. McKenney <paulmck@kernel.org> 写道：
> 
> On Thu, Jul 06, 2023 at 10:28:49AM +0000, Alan Huang wrote:
>> The objects are allocated with SLAB_TYPESAFE_BY_RCU, and there is
>> n->next = first within hlist_add_head_rcu() before rcu_assign_pointer(),
>> which modifies obj->obj_node.next. There may be readers holding the
>> reference of obj in lockless_lookup, and when updater modifies ->next,
>> readers can see the change immediately because of SLAB_TYPESAFE_BY_RCU.
>> 
>> There are two memory ordering required in the insertion algorithm,
>> we need to make sure obj->key is updated before obj->obj_node.next
>> and obj->refcnt, atomic_set_release is not enough to provide the
>> required memory barrier.
>> 
>> Signed-off-by: Alan Huang <mmpgouride@gmail.com>
>> ---
>> Documentation/RCU/rculist_nulls.rst | 9 +++++++--
>> 1 file changed, 7 insertions(+), 2 deletions(-)
>> 
>> diff --git a/Documentation/RCU/rculist_nulls.rst b/Documentation/RCU/rculist_nulls.rst
>> index 21e40fcc08de..fa3729dc7e74 100644
>> --- a/Documentation/RCU/rculist_nulls.rst
>> +++ b/Documentation/RCU/rculist_nulls.rst
>> @@ -64,7 +64,7 @@ but a version with an additional memory barrier (smp_rmb())
>>   {
>>     struct hlist_node *node, *next;
>>     for (pos = rcu_dereference((head)->first);
>> -         pos && ({ next = pos->next; smp_rmb(); prefetch(next); 1; }) &&
>> +         pos && ({ next = READ_ONCE(pos->next); smp_rmb(); prefetch(next); 1; }) &&
> 
> This one looks good, though the READ_ONCE() becoming rcu_dereference()
> would allow the smp_rmb() to be dropped, correct?

The pattern here is:

reader 									               updater

// pos->next is also obj->obj_node.next				     
READ_ONCE(pos->next); 							WRITE_ONCE(obj->key, key);
smp_rmb();										smp_wmb();
												// this is n->next = first; within hlist_add_head_rcu
READ_ONCE(obj->key);							WRITE_ONCE(obj->obj_node.next, h->first);

The point here is that the objects are allocated with SLAB_TYPESAFE_BY_RCU, and there is 

	 n->next = first;

within hlist_add_head_rcu, the modification is visible to readers immediately (before the invocation of rcu_assign_pointer)

Therefore, the smp_rmb() is necessary to ensure that we won’t get the new value of ->next and the old ->key.
(If we get the new ->next and old ->key, we can not detect movement of the object.)

But in this patch, I forgot to add READ_ONCE to obj->key, will send v2.

> 
>>          ({ obj = hlist_entry(pos, typeof(*obj), obj_node); 1; });
>>          pos = rcu_dereference(next))
>>       if (obj->key == key)
>> @@ -112,7 +112,12 @@ detect the fact that it missed following items in original chain.
>>   obj = kmem_cache_alloc(...);
>>   lock_chain(); // typically a spin_lock()
>>   obj->key = key;
>> -  atomic_set_release(&obj->refcnt, 1); // key before refcnt
>> +  /*
>> +   * We need to make sure obj->key is updated before obj->obj_node.next
>> +   * and obj->refcnt.
>> +   */
>> +  smp_wmb();
>> +  atomic_set(&obj->refcnt, 1);
> 
> ...but what is smp_wmb() doing that the combination of atomic_set_release()
> and hlist_add_head_rcu() was not already doing?  What am I missing?

Like above.

> 
> Thanx, Paul
> 
>>   hlist_add_head_rcu(&obj->obj_node, list);
>>   unlock_chain(); // typically a spin_unlock()
>> 
>> -- 
>> 2.34.1