linux-kernel - Re: [PATCH 1/2] rcu: Don't chase unnecessary quiescent states after extended grace periods

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: <20101124005820.GH8056@linux.vnet.ibm.com>
Date:	Tue, 23 Nov 2010 16:58:20 -0800
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Frederic Weisbecker <fweisbec@...il.com>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Lai Jiangshan <laijs@...fujitsu.com>,
	Ingo Molnar <mingo@...e.hu>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [PATCH 1/2] rcu: Don't chase unnecessary quiescent states
 after extended grace periods

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@...il.com>
> Cc: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> Cc: Lai Jiangshan <laijs@...fujitsu.com>
> Cc: Ingo Molnar <mingo@...e.hu>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> Cc: Steven Rostedt <rostedt@...dmis.org>
> ---
>  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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/