Re: [PATCH 1/2] rcu: Don't chase unnecessary quiescent statesafter extended grace periods

From: Paul E. McKenney
Date: Tue Nov 23 2010 - 19:58:29 EST


On Wed, Nov 24, 2010 at 01:31:12AM +0100, Frederic Weisbecker wrote:
> When a cpu is in an extended quiescent state, which includes idle
> nohz or CPU offline, others CPUs will take care of the grace periods
> on its behalf.
>
> When this CPU exits its extended quiescent state, it will catch up
> with the last started grace period and start chasing its own
> quiescent states to end the current grace period.
>
> However in this case we always start to track quiescent states if the
> grace period number has changed since we started our extended
> quiescent state. And we do this because we always assume that the last
> grace period is not finished and needs us to complete it, which is
> sometimes wrong.
>
> This patch verifies if the last grace period has been completed and
> if so, start hunting local quiescent states like we always did.
> Otherwise don't do anything, this economizes us some work and
> an unnecessary softirq.

Interesting approach! I can see how this helps in the case where the
CPU just came online, but I don't see it in the nohz case, because the
nohz case does not update the rdp->completed variable. In contrast,
the online path calls rcu_init_percpu_data() which sets up this variable.

So, what am I missing here?

Thanx, Paul

PS. It might well be worthwhile for the online case alone, but
the commit message does need to be accurate.

> Signed-off-by: Frederic Weisbecker <fweisbec@xxxxxxxxx>
> Cc: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
> Cc: Lai Jiangshan <laijs@xxxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Peter Zijlstra <a.p.zijlstra@xxxxxxxxx>
> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> ---
> kernel/rcutree.c | 13 +++++++++++--
> 1 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> index ccdc04c..5f038a1 100644
> --- a/kernel/rcutree.c
> +++ b/kernel/rcutree.c
> @@ -620,8 +620,17 @@ static void __init check_cpu_stall_init(void)
> static void __note_new_gpnum(struct rcu_state *rsp, struct rcu_node *rnp, struct rcu_data *rdp)
> {
> if (rdp->gpnum != rnp->gpnum) {
> - rdp->qs_pending = 1;
> - rdp->passed_quiesc = 0;
> + /*
> + * Another CPU might have taken take of this new grace period
> + * while we were idle and handled us as in an extended quiescent
> + * state. In that case, we don't need to chase a local quiescent
> + * state, otherwise:
> + */
> + if (rdp->completed != rnp->gpnum) {
> + rdp->qs_pending = 1;
> + rdp->passed_quiesc = 0;
> + }
> +
> rdp->gpnum = rnp->gpnum;
> }
> }
> --
> 1.7.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/