[PATCH v1] rcu: use WRITE_ONCE() for ->next and ->pprev of hlist_nulls

[PATCH v1] rcu: use WRITE_ONCE() for ->next and ->pprev of hlist_nulls

[xuanqiang.luo]

From: Xuanqiang Luo <luoxuanqiang@kylinos.cn>

In rculist_nulls.h we can still see ordinary assignments to ->pprev and
->next of hlist_nulls.

As noted in the two patches below:
commit efd04f8a8b45 ("rcu: Use WRITE_ONCE() for assignments to ->next for
rculist_nulls")
commit 860c8802ace1 ("rcu: Use WRITE_ONCE() for assignments to ->pprev for
hlist_nulls")

We should use WRITE_ONCE().

Signed-off-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
---
 include/linux/rculist_nulls.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/rculist_nulls.h b/include/linux/rculist_nulls.h
index 89186c499dd4..d5a656cc4c6a 100644
--- a/include/linux/rculist_nulls.h
+++ b/include/linux/rculist_nulls.h
@@ -138,7 +138,7 @@ static inline void hlist_nulls_add_tail_rcu(struct hlist_nulls_node *n,
 
 	if (last) {
 		WRITE_ONCE(n->next, last->next);
-		n->pprev = &last->next;
+		WRITE_ONCE(n->pprev, &last->next);
 		rcu_assign_pointer(hlist_nulls_next_rcu(last), n);
 	} else {
 		hlist_nulls_add_head_rcu(n, h);
@@ -148,8 +148,8 @@ static inline void hlist_nulls_add_tail_rcu(struct hlist_nulls_node *n,
 /* after that hlist_nulls_del will work */
 static inline void hlist_nulls_add_fake(struct hlist_nulls_node *n)
 {
-	n->pprev = &n->next;
-	n->next = (struct hlist_nulls_node *)NULLS_MARKER(NULL);
+	WRITE_ONCE(n->pprev, &n->next);
+	WRITE_ONCE(n->next, (struct hlist_nulls_node *)NULLS_MARKER(NULL));
 }
 
 /**
-- 
2.25.1

Re: [PATCH v1] rcu: use WRITE_ONCE() for ->next and ->pprev of hlist_nulls

[Eric Dumazet]

On Thu, Oct 9, 2025 at 11:50 PM <xuanqiang.luo@linux.dev> wrote:
>
> From: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
>
> In rculist_nulls.h we can still see ordinary assignments to ->pprev and
> ->next of hlist_nulls.
>
> As noted in the two patches below:
> commit efd04f8a8b45 ("rcu: Use WRITE_ONCE() for assignments to ->next for
> rculist_nulls")
> commit 860c8802ace1 ("rcu: Use WRITE_ONCE() for assignments to ->pprev for
> hlist_nulls")
>
> We should use WRITE_ONCE().
>
> Signed-off-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
> ---
>  include/linux/rculist_nulls.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/rculist_nulls.h b/include/linux/rculist_nulls.h
> index 89186c499dd4..d5a656cc4c6a 100644
> --- a/include/linux/rculist_nulls.h
> +++ b/include/linux/rculist_nulls.h
> @@ -138,7 +138,7 @@ static inline void hlist_nulls_add_tail_rcu(struct hlist_nulls_node *n,
>
>         if (last) {
>                 WRITE_ONCE(n->next, last->next);
> -               n->pprev = &last->next;
> +               WRITE_ONCE(n->pprev, &last->next);

n is private to the current thread. No other cpu/thread can see it yet.

Following rcu_assign_pointer() already has the needed barrier making
sure that _when_ the object
can be seen by others, n->pprev value is fully up to date.

>                 rcu_assign_pointer(hlist_nulls_next_rcu(last), n);
>         } else {
>                 hlist_nulls_add_head_rcu(n, h);
> @@ -148,8 +148,8 @@ static inline void hlist_nulls_add_tail_rcu(struct hlist_nulls_node *n,
>  /* after that hlist_nulls_del will work */
>  static inline void hlist_nulls_add_fake(struct hlist_nulls_node *n)
>  {
> -       n->pprev = &n->next;
> -       n->next = (struct hlist_nulls_node *)NULLS_MARKER(NULL);
> +       WRITE_ONCE(n->pprev, &n->next);
> +       WRITE_ONCE(n->next, (struct hlist_nulls_node *)NULLS_MARKER(NULL));

Same here, n is private. No need for any WRITE_ONCE().

Re: [PATCH v1] rcu: use WRITE_ONCE() for ->next and ->pprev of hlist_nulls

[Paul E. McKenney]

On Fri, Oct 10, 2025 at 02:49:10PM +0800, xuanqiang.luo@linux.dev wrote:
> From: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
> 
> In rculist_nulls.h we can still see ordinary assignments to ->pprev and
> ->next of hlist_nulls.
> 
> As noted in the two patches below:
> commit efd04f8a8b45 ("rcu: Use WRITE_ONCE() for assignments to ->next for
> rculist_nulls")
> commit 860c8802ace1 ("rcu: Use WRITE_ONCE() for assignments to ->pprev for
> hlist_nulls")
> 
> We should use WRITE_ONCE().
> 
> Signed-off-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>

Hearing no objections, I have pulled this in for further review and
testing.

							Thanx, Paul

> ---
>  include/linux/rculist_nulls.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/rculist_nulls.h b/include/linux/rculist_nulls.h
> index 89186c499dd4..d5a656cc4c6a 100644
> --- a/include/linux/rculist_nulls.h
> +++ b/include/linux/rculist_nulls.h
> @@ -138,7 +138,7 @@ static inline void hlist_nulls_add_tail_rcu(struct hlist_nulls_node *n,
>  
>  	if (last) {
>  		WRITE_ONCE(n->next, last->next);
> -		n->pprev = &last->next;
> +		WRITE_ONCE(n->pprev, &last->next);
>  		rcu_assign_pointer(hlist_nulls_next_rcu(last), n);
>  	} else {
>  		hlist_nulls_add_head_rcu(n, h);
> @@ -148,8 +148,8 @@ static inline void hlist_nulls_add_tail_rcu(struct hlist_nulls_node *n,
>  /* after that hlist_nulls_del will work */
>  static inline void hlist_nulls_add_fake(struct hlist_nulls_node *n)
>  {
> -	n->pprev = &n->next;
> -	n->next = (struct hlist_nulls_node *)NULLS_MARKER(NULL);
> +	WRITE_ONCE(n->pprev, &n->next);
> +	WRITE_ONCE(n->next, (struct hlist_nulls_node *)NULLS_MARKER(NULL));
>  }
>  
>  /**
> -- 
> 2.25.1
> 
>