Re: [RFC PATCH 1/1] kernel/rcu/tree.c: correct a check for grace period in progress

["Paul E. McKenney" <paulmck@linux.vnet.ibm.com>]

On Tue, Jun 10, 2014 at 11:20:19PM -0400, Pranith Kumar wrote:
> The comment above the code says that we are checking both the current node and
> the parent node to see if a grace period is in progress. Change the code
> accordingly.

Almost...  Please see below.

							Thanx, Paul

> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
> ---
>  kernel/rcu/tree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index f1ba773..b632189 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -1227,7 +1227,7 @@ rcu_start_future_gp(struct rcu_node *rnp, struct rcu_data *rdp,
>  	 * need to explicitly start one.
>  	 */
>  	if (rnp->gpnum != rnp->completed ||
> -	    ACCESS_ONCE(rnp->gpnum) != ACCESS_ONCE(rnp->completed)) {
> +	    ACCESS_ONCE(rnp_root->gpnum) != ACCESS_ONCE(rnp_root->completed)) {

At this point in the code, we are checking the current rcu_node structure,
which might or might not be the root.  If it is not the root, we absolutely
cannot compare against the root because we don't yet hold the root's lock.

So I cannot take this change.

That said, I do heartily encourage you to keep looking.  After all, there
are bound to be at least a few bugs in RCU somewhere.

>  		rnp->need_future_gp[c & 0x1]++;
>  		trace_rcu_future_gp(rnp, rdp, c, TPS("Startedleaf"));
>  		goto out;