linux-kernel - Re: [PATCH] sched: Folding nohz load accounting more accurate

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: <1340097532.21745.19.camel@twins>
Date:	Tue, 19 Jun 2012 11:18:52 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Yong Zhang <yong.zhang0@...il.com>
Cc:	Charles Wang <muming.wq@...il.com>, linux-kernel@...r.kernel.org,
	Ingo Molnar <mingo@...hat.com>, Tao Ma <tm@....ma>,
	含黛 <handai.szj@...bao.com>,
	Doug Smythies <dsmythies@...us.net>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] sched: Folding nohz load accounting more accurate

On Tue, 2012-06-19 at 14:08 +0800, Yong Zhang wrote:

> > ---
> >  kernel/sched/core.c      |  290 ++++++++++++++++++++++++++++++++++------------
> >  kernel/sched/idle_task.c |    1 -
> >  kernel/sched/sched.h     |    2 -
> >  kernel/time/tick-sched.c |    2 +
> >  4 files changed, 220 insertions(+), 75 deletions(-)
> > 
> > + *  - When we go NO_HZ idle during the window, we can negate our sample
> > + *    contribution, causing under-accounting.
> > + *
> > + *    We avoid this by keeping two idle-delta counters and flipping them
> > + *    when the window starts, thus separating old and new NO_HZ load.
> > + *
> > + *    The only trick is the slight shift in index flip for read vs write.
> > + *
> > + *       0             5             10            15
> > + *         +10           +10           +10           +10
> > + *       |-|-----------|-|-----------|-|-----------|-|
> > + *    r:001           110           001           110
> > + *    w:011           100           011           100
> 
> I'm confused by this comments, looking at your code, index is increased by
> 1 for each samaple window.

Also looking at the code you'll find we only ever use idx & 1.

> > + *
> > + *    This ensures we'll fold the old idle contribution in this window while
> > + *    accumlating the new one.
> > + *
> > + *  - When we wake up from NO_HZ idle during the window, we push up our
> > + *    contribution, since we effectively move our sample point to a known
> > + *    busy state.
> > + *
> > + *    This is solved by pushing the window forward, and thus skipping the
> > + *    sample, for this cpu (effectively using the idle-delta for this cpu which
> > + *    was in effect at the time the window opened). This also solves the issue
> > + *    of having to deal with a cpu having been in NOHZ idle for multiple
> > + *    LOAD_FREQ intervals.
> >   *
> >   * When making the ILB scale, we should try to pull this in as well.
> >   */
> > +void calc_load_exit_idle(void)
> >  {
> > +	struct rq *this_rq = this_rq();
> >  
> >  	/*
> > +	 * If we're still outside the sample window, we're done.
> >  	 */
> > +	if (time_before(jiffies, this_rq->calc_load_update))
> > +		return;

> 	else if (time_before(jiffies, calc_load_update + 10)
> 		this_rq->calc_load_update = calc_load_update + LOAD_FREQ;
> 	else
> 		this_rq->calc_load_update = calc_load_update;
> 
> Otherwise if you woke after the sample window, we loose on sample?
> And maybe we need local variable to cache calc_load_update.

Ah indeed, although I'd write it like:

	this_rq->calc_load_update = calc_load_update;
	if (time_before(jiffies, this_rq->calc_load_update + 10)
		this_rq->calc_load_update += LOAD_FREQ;

Thanks!

--
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/