Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1374199

Re: [PATCH 2/3] sched: Correctly handle nohz ticks cpu load accounting

From Frederic Weisbecker <fweisbec@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCH 2/3] sched: Correctly handle nohz ticks cpu load accounting
Date 2016-04-08 15:00 +0200
Message-ID <rlEaZ-3RN-9@gated-at.bofh.it> (permalink)
References <rlt5T-4eY-1@gated-at.bofh.it> <rlt5T-4eY-9@gated-at.bofh.it> <rlBd8-1LF-13@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Fri, Apr 08, 2016 at 11:41:54AM +0200, Peter Zijlstra wrote:
> On Fri, Apr 08, 2016 at 03:07:12AM +0200, Frederic Weisbecker wrote:
> > +void cpu_load_update_nohz_start(void)
> >  {
> >  	struct rq *this_rq = this_rq();
> > +
> > +	/*
> > +	 * This is all lockless but should be fine. If weighted_cpuload changes
> > +	 * concurrently we'll exit nohz. And cpu_load write can race with
> > +	 * cpu_load_update_idle() but both updater would be writing the same.
> > +	 */
> > +	this_rq->cpu_load[0] = weighted_cpuload(cpu_of(this_rq));
> > +}
> 
> There is more to this; this also updates ->cpu_load[0] at possibly much
> higher frequency than we've done before, while not updating the other
> ->cpu_load[] members.
> 
> Now, I'm not sure we care, but it is a bit odd.

This is right. cpu_load[0] is aimed at showing the most recent load, without knowing
when was the last update (the last tick/update could have been recent or not, the readers shouldn't
care). Now we can indeed worry about it if this field is read altogether with the other indexes and
those are put into some relation. But it seems that each of the rq->cpu_load[i] are read independently
without relation or comparison. Now really I'm saying that without much assurance as I don't know
the details of the load balancing code.

If in doubt I can create a field in struct rq to record the tickless load instead
of storing it in cpu_load[0]. That was in fact the first direction I took in my drafts.

> 
> > +/*
> > + * Account the tickless load in the end of a nohz frame.
> > + */
> > +void cpu_load_update_nohz_stop(void)
> > +{
> >  	unsigned long curr_jiffies = READ_ONCE(jiffies);
> > +	struct rq *this_rq = this_rq();
> > +	unsigned long load;
> >  
> >  	if (curr_jiffies == this_rq->last_load_update_tick)
> >  		return;
> >  
> > +	load = weighted_cpuload(cpu_of(this_rq));
> >  	raw_spin_lock(&this_rq->lock);
> > +	cpu_load_update_nohz(this_rq, curr_jiffies, load);
> >  	raw_spin_unlock(&this_rq->lock);
> >  }
> 
> And this makes us take rq->lock when waking from nohz; a bit
> unfortunate. Do we really need this though? 

Note it was already the case before this patchset, the function was called
cpu_load_update_nohz() instead.

I think we need it, at least due to remote updates performed by cpu_load_update_idle()
(which only updates when load is 0 though).

> Will not a tick be forthcoming real-soon-now?

No guarantee about that. If the next task only runs for less than 10ms (in HZ=100),
there might be no tick until the next idle period.

Besides in nohz_full, the next task may be running tickless as well, in which case
we really need to update now.

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 0/3] sched: Fix/improve nohz cpu load updates v2 Frederic Weisbecker <fweisbec@gmail.com> - 2016-04-08 03:10 +0200
  [PATCH 2/3] sched: Correctly handle nohz ticks cpu load accounting Frederic Weisbecker <fweisbec@gmail.com> - 2016-04-08 03:10 +0200
    Re: [PATCH 2/3] sched: Correctly handle nohz ticks cpu load  accounting Peter Zijlstra <peterz@infradead.org> - 2016-04-08 11:50 +0200
      Re: [PATCH 2/3] sched: Correctly handle nohz ticks cpu load  accounting Frederic Weisbecker <fweisbec@gmail.com> - 2016-04-08 15:00 +0200
        Re: [PATCH 2/3] sched: Correctly handle nohz ticks cpu load  accounting Peter Zijlstra <peterz@infradead.org> - 2016-04-08 19:50 +0200
  [PATCH 1/3] sched: Gather cpu load functions under a more conventional namespace Frederic Weisbecker <fweisbec@gmail.com> - 2016-04-08 03:10 +0200
  [PATCH 3/3] sched: Optimize !CONFIG_NO_HZ_COMMON cpu load updates Frederic Weisbecker <fweisbec@gmail.com> - 2016-04-08 03:10 +0200
    Re: [PATCH 3/3] sched: Optimize !CONFIG_NO_HZ_COMMON cpu load updates Peter Zijlstra <peterz@infradead.org> - 2016-04-08 12:50 +0200
      Re: [PATCH 3/3] sched: Optimize !CONFIG_NO_HZ_COMMON cpu load updates Frederic Weisbecker <fweisbec@gmail.com> - 2016-04-08 15:00 +0200
        Re: [PATCH 3/3] sched: Optimize !CONFIG_NO_HZ_COMMON cpu load updates Peter Zijlstra <peterz@infradead.org> - 2016-04-08 19:50 +0200
          Re: [PATCH 3/3] sched: Optimize !CONFIG_NO_HZ_COMMON cpu load updates Frederic Weisbecker <fweisbec@gmail.com> - 2016-04-11 15:20 +0200
            Re: [PATCH 3/3] sched: Optimize !CONFIG_NO_HZ_COMMON cpu load updates Frederic Weisbecker <fweisbec@gmail.com> - 2016-04-11 20:30 +0200
              Re: [PATCH 3/3] sched: Optimize !CONFIG_NO_HZ_COMMON cpu load updates Peter Zijlstra <peterz@infradead.org> - 2016-04-12 16:30 +0200

csiph-web