linux-kernel - Re: [PATCH v2 -rcu] srcu: Use rcu_seq_done

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <dde0f073-c3d9-44c9-bd85-cf0e8a0fd628@paulmck-laptop>
Date: Wed, 19 Feb 2025 05:22:21 -0800
From: "Paul E. McKenney" <paulmck@...nel.org>
To: Joel Fernandes <joelagnelf@...dia.com>
Cc: linux-kernel@...r.kernel.org, Lai Jiangshan <jiangshanlai@...il.com>,
	Josh Triplett <josh@...htriplett.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	Neeraj Upadhyay <Neeraj.Upadhyay@....com>, rcu@...r.kernel.org
Subject: Re: [PATCH v2 -rcu] srcu: Use rcu_seq_done_exact() for polling API

On Wed, Feb 19, 2025 at 07:43:08AM -0500, Joel Fernandes wrote:
> poll_state_synchronize_srcu() uses rcu_seq_done() unlike
> poll_state_synchronize_rcu() which uses rcu_seq_done_exact().
> 
> The  rcu_seq_done_exact() makes more sense for polling API, as with
> this API, there is a higher chance that there is a significant delay
> between the get_state..() and poll_state..() calls since a cookie
> can be stored and reused at a later time. During such a delay, if
> the gp_seq counter progresses more than ULONG_MAX/2 distance, then
> poll_state..() may return false for a long time unwantedly.
> 
> Fix by using the more accurate rcu_seq_done_exact() API which is
> exactly what straight RCU's polling does.
> 
> It may make sense, as future work, to add debug code here as well, where
> we compare a physical timestamp between get_state..() and poll_state()
> calls and yell if significant time has past but the grace period has
> still not progressed.
> 
> Reviewed-by: Neeraj Upadhyay <Neeraj.Upadhyay@....com>
> Signed-off-by: Joel Fernandes <joelagnelf@...dia.com>

Reviewed-by: Paul E. McKenney <paulmck@...nel.org>

But we should also run this by Kent Overstreet, given that bcachefs
uses this.  Should be OK, given that bcachefs uses this API in the same
way that it does poll_state_synchronize_rcu(), but still...

							Thanx, Paul

> ---
> v1->v2: Resend with Neeraj review tag. Rebased on rcu/dev.
> 
>  kernel/rcu/srcutree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index d2a694944553..591371d62e89 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -1589,7 +1589,7 @@ EXPORT_SYMBOL_GPL(start_poll_synchronize_srcu);
>  bool poll_state_synchronize_srcu(struct srcu_struct *ssp, unsigned long cookie)
>  {
>  	if (cookie != SRCU_GET_STATE_COMPLETED &&
> -	    !rcu_seq_done(&ssp->srcu_sup->srcu_gp_seq, cookie))
> +	    !rcu_seq_done_exact(&ssp->srcu_sup->srcu_gp_seq, cookie))
>  		return false;
>  	// Ensure that the end of the SRCU grace period happens before
>  	// any subsequent code that the caller might execute.
> -- 
> 2.34.1
>