Messages in this thread | | | Date | Mon, 18 Apr 2016 17:22:41 +0900 | From | Byungchul Park <> | Subject | Re: [PATCH 2/3] sched: Correctly handle nohz ticks cpu load accounting |
| |
On Wed, Apr 13, 2016 at 03:56:51PM +0200, Frederic Weisbecker wrote: > @@ -4524,12 +4523,12 @@ decay_load_missed(unsigned long load, unsigned long missed_updates, int idx) > * load[i]_n = (1 - 1/2^i)^n * load[i]_0 > * > * see decay_load_misses(). For NOHZ_FULL we get to subtract and add the extra > - * term. See the @active paramter. > + * term. > */ > -static void __cpu_load_update(struct rq *this_rq, unsigned long this_load, > - unsigned long pending_updates, int active) > +static void cpu_load_update(struct rq *this_rq, unsigned long this_load, > + unsigned long pending_updates) > { > - unsigned long tickless_load = active ? this_rq->cpu_load[0] : 0; > + unsigned long tickless_load = this_rq->cpu_load[0];
Hello,
Good for my humble code to be fixed so we can write it like this here.
> @@ -4618,26 +4617,56 @@ static void cpu_load_update_idle(struct rq *this_rq) > if (weighted_cpuload(cpu_of(this_rq))) > return; > > - __cpu_load_update_nohz(this_rq, READ_ONCE(jiffies), 0, 0); > + cpu_load_update_nohz(this_rq, READ_ONCE(jiffies), 0); > } > > /* > - * Called from tick_nohz_idle_exit() -- try and fix up the ticks we missed. > + * Record CPU load on nohz entry so we know the tickless load to account > + * on nohz exit. cpu_load[0] happens then to be updated more frequently > + * than other cpu_load[idx] but it should be fine as cpu_load readers > + * shouldn't rely into synchronized cpu_load[*] updates. > */ > -void cpu_load_update_nohz(int active) > +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));
Like it.
> +/* > + * 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); > - unsigned long load = active ? weighted_cpuload(cpu_of(this_rq)) : 0; > + 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));
Like it.
> @@ -4645,11 +4674,11 @@ void cpu_load_update_nohz(int active) > void cpu_load_update_active(struct rq *this_rq) > { > unsigned long load = weighted_cpuload(cpu_of(this_rq)); > - /* > - * See the mess around cpu_load_update_idle() / cpu_load_update_nohz(). > - */ > - this_rq->last_load_update_tick = jiffies; > - __cpu_load_update(this_rq, load, 1, 1); > + > + if (tick_nohz_tick_stopped()) > + cpu_load_update_nohz(this_rq, READ_ONCE(jiffies), load); > + else > + cpu_load_update_periodic(this_rq, load);
Oh! We have missed it until now. Terrible.. :-(
Thank you, Byungchul
| |