Re: [PATCH v4 3/3] rcu: Use _full() API to debug synchronize_rcu()

[Joel Fernandes]

Hi Uladzislau,

On Thu, Feb 27, 2025 at 02:16:13PM +0100, Uladzislau Rezki (Sony) wrote:
> Switch for using of get_state_synchronize_rcu_full() and
> poll_state_synchronize_rcu_full() pair to debug a normal
> synchronize_rcu() call.
> 
> Just using "not" full APIs to identify if a grace period is
> passed or not might lead to a false-positive kernel splat.
> 
> It can happen, because get_state_synchronize_rcu() compresses
> both normal and expedited states into one single unsigned long
> value, so a poll_state_synchronize_rcu() can miss GP-completion
> when synchronize_rcu()/synchronize_rcu_expedited() concurrently
> run.

Agreed, I provided a scenario below but let me know if I missed anything.

> To address this, switch to poll_state_synchronize_rcu_full() and
> get_state_synchronize_rcu_full() APIs, which use separate variables
> for expedited and normal states.

Reviewed-by: Joel Fernandes <joelagnelf@xxxxxxxxxx>

For completeness and just to clarify how this may happen, firstly as noted:
rcu_poll_gp_seq_start/end() is called for both begin/end of normal and exp
GPs thus compressing the use of the rcu_state.gp_seq_polled counter for
both normal and exp GPs.

Then if we intersperse synchronize_rcu() with synchronize_rcu_expedited(),
something like the following may happen.

CPU 0					CPU 1

					synchronize_rcu_expedited()
					// -> rcu_poll_gp_seq_start()
					// This does rcu_seq_start on the
					// gp_seq_polled and
					// notes the started gp_seq_polled
					// (say its 5)
synchronize_rcu()
 -> synchronize_rcu_normal()
  -> rs.head.func =
     get_state_synchronize_rcu();
     // saves the value 12
 

 -> rcu_gp_init()
  -> rcu_poll_gp_seq_start()
  // rcu_seq_start does nothing
  // but notes the pre-started
  // gp_seq_polled (value 5)

-> rcu_gp_cleanup()
  // -> rcu_poll_gp_seq_end()
  // ends the gp_seq_polled since it
  // matches prior saved gp_seq_polled (5)
  // new gp_seq_polled is 8.

                        /*  NORMAL GP COMPLETES  */

rcu_gp_cleanup()
 -> rcu_sr_normal_gp_cleanup()
   -> rcu_sr_normal_complete()
     -> poll_state_synchronize_rcu()
       -> returns FALSE because gp_seq_polled is still 8.
       -> Warning (false positive)



thanks,

  - Joel