[PATCH] rcu/segcblist: Use WRITE_ONCE() for rclp->len decrement

[PATCH] rcu/segcblist: Use WRITE_ONCE() for rclp->len decrement

[Kaushlendra Kumar]

The rclp->len field is accessed concurrently by multiple contexts
in RCU operations. Using WRITE_ONCE() provides the necessary memory
ordering guarantees.

This change ensures that the callback list length is updated atomically
and provides consistent visibility across different CPU contexts,
maintaining the integrity of RCU callback list management.

Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
---
 kernel/rcu/rcu_segcblist.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/rcu/rcu_segcblist.c b/kernel/rcu/rcu_segcblist.c
index 1693ea22ef1b..e10b36e9de54 100644
--- a/kernel/rcu/rcu_segcblist.c
+++ b/kernel/rcu/rcu_segcblist.c
@@ -71,7 +71,7 @@ struct rcu_head *rcu_cblist_dequeue(struct rcu_cblist *rclp)
 	rhp = rclp->head;
 	if (!rhp)
 		return NULL;
-	rclp->len--;
+	WRITE_ONCE(rclp->len, rclp->len - 1);
 	rclp->head = rhp->next;
 	if (!rclp->head)
 		rclp->tail = &rclp->head;
-- 
2.34.1

Re: [PATCH] rcu/segcblist: Use WRITE_ONCE() for rclp->len decrement

[Paul E. McKenney]

On Thu, Sep 11, 2025 at 10:40:09AM +0530, Kaushlendra Kumar wrote:
> The rclp->len field is accessed concurrently by multiple contexts
> in RCU operations. Using WRITE_ONCE() provides the necessary memory
> ordering guarantees.

Could you please be specific here?  What calls to rcu_cblist_dequeue()
are such that hte ->qlen field can be concurrently accessed?

(Full disclosure: I don't see any, and KCSAN hasn't found any.  Of course,
that does not necessarily mean that there is no concurrent access.
But we need such concurrent access called out explicitly here, because
it might well be that the concurrent access is itself the bug.)

							Thanx, Paul

> This change ensures that the callback list length is updated atomically
> and provides consistent visibility across different CPU contexts,
> maintaining the integrity of RCU callback list management.
> 
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> ---
>  kernel/rcu/rcu_segcblist.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/rcu_segcblist.c b/kernel/rcu/rcu_segcblist.c
> index 1693ea22ef1b..e10b36e9de54 100644
> --- a/kernel/rcu/rcu_segcblist.c
> +++ b/kernel/rcu/rcu_segcblist.c
> @@ -71,7 +71,7 @@ struct rcu_head *rcu_cblist_dequeue(struct rcu_cblist *rclp)
>  	rhp = rclp->head;
>  	if (!rhp)
>  		return NULL;
> -	rclp->len--;
> +	WRITE_ONCE(rclp->len, rclp->len - 1);
>  	rclp->head = rhp->next;
>  	if (!rclp->head)
>  		rclp->tail = &rclp->head;
> -- 
> 2.34.1
> 

RE: [PATCH] rcu/segcblist: Use WRITE_ONCE() for rclp->len decrement

[Kumar, Kaushlendra]

On Thu, Sep 11, 2025 at 10:40:09AM +0530, Paul E. McKenney wrote:
> On Thu, Sep 11, 2025 at 10:40:09AM +0530, Kaushlendra Kumar wrote:
> > The rclp->len field is accessed concurrently by multiple contexts in 
> > RCU operations. Using WRITE_ONCE() provides the necessary memory 
> > ordering guarantees.
> 
> Could you please be specific here?  What calls to rcu_cblist_dequeue() are such that hte ->qlen field can be concurrently accessed?
> 
> (Full disclosure: I don't see any, and KCSAN hasn't found any.  Of course, that does not necessarily mean that there is no concurrent access.
> But we need such concurrent access called out explicitly here, because it might well be that the concurrent access is itself the bug.)
> 
> 							Thanx, Paul

Hi Paul,

Thank you for the clarification. You are absolutely correct. After reviewing the 
code more carefully, I cannot identify specific concurrent access patterns for 
the rclp->len field during rcu_cblist_dequeue() operations.

The primary motivation for this patch was to maintain consistency with 
rcu_cblist_enqueue(), which uses WRITE_ONCE() for the rclp->len increment. 

I will modify the message accordingly and send a patch.

Best regards,
Kaushlendra

Re: [PATCH] rcu/segcblist: Use WRITE_ONCE() for rclp->len decrement

[Paul E. McKenney]

On Thu, Sep 11, 2025 at 03:46:08PM +0000, Kumar, Kaushlendra wrote:
> On Thu, Sep 11, 2025 at 10:40:09AM +0530, Paul E. McKenney wrote:
> > On Thu, Sep 11, 2025 at 10:40:09AM +0530, Kaushlendra Kumar wrote:
> > > The rclp->len field is accessed concurrently by multiple contexts in 
> > > RCU operations. Using WRITE_ONCE() provides the necessary memory 
> > > ordering guarantees.
> > 
> > Could you please be specific here?  What calls to rcu_cblist_dequeue() are such that hte ->qlen field can be concurrently accessed?
> > 
> > (Full disclosure: I don't see any, and KCSAN hasn't found any.  Of course, that does not necessarily mean that there is no concurrent access.
> > But we need such concurrent access called out explicitly here, because it might well be that the concurrent access is itself the bug.)
> > 
> > 							Thanx, Paul
> 
> Hi Paul,
> 
> Thank you for the clarification. You are absolutely correct. After reviewing the 
> code more carefully, I cannot identify specific concurrent access patterns for 
> the rclp->len field during rcu_cblist_dequeue() operations.
> 
> The primary motivation for this patch was to maintain consistency with 
> rcu_cblist_enqueue(), which uses WRITE_ONCE() for the rclp->len increment. 
> 
> I will modify the message accordingly and send a patch.

Why exactly is a patch needed for this case?

							Thanx, Paul

Re: [PATCH] rcu/segcblist: Use WRITE_ONCE() for rclp->len decrement

[Markus Elfring]

…
> This change ensures that …

See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc5#n94

Regards,
Markus

RE: [PATCH] rcu/segcblist: Use WRITE_ONCE() for rclp->len decrement

[Kumar, Kaushlendra]

On Thu, Sep 11, 2025, Markus Elfring wrote:
> …
> > This change ensures that …
> 
> See also:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.17-rc5#n94
> 
> Regards,
> Markus

Hi Markus,

Thank you for pointing out the documentation guidelines.
revised the commit message accordingly and sent a V2 patch.
I appreciate your attention to proper documentation compliance.

Best regards,
Kaushlendra