Messages in this thread | ![/](/images/icornerl.gif) | | From | Paul Turner <> | Date | Fri, 12 Nov 2010 17:01:37 -0800 | Subject | Re: [tg_shares_up rewrite v3 09/11] sched: demand based update_cfs_load() |
| |
On Fri, Nov 12, 2010 at 2:53 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote: > On Thu, 2010-11-11 at 19:24 -0800, Paul Turner wrote: >> +#ifdef CONFIG_FAIR_GROUP_SCHED >> + cfs_rq->load_unacc_exec_time += delta_exec; >> + if (cfs_rq->load_unacc_exec_time > sysctl_sched_shares_window) >> { >> + update_cfs_load(cfs_rq); >> + update_cfs_shares(cfs_rq, 0); > > Why not: > + cfs_rq->load_unacc_exec_time -= sysctl_sched_shares_window; > > (although you probably want to read the sysctl value into a local > variable using ACCESS_ONCE() and use that for both cases). >
I think this is hard to do in a clean fashion (without a strange arg to update_cfs_load).
I'm also not sure it's worth synchronizing on a shares_window change since, when we trigger an update from update_curr() is independent of folding that time into the load average anyway. (Note even: using sysctl_sched_shares_window is already a larger window than update_cfs_load will normally fold at since we fold at window/2 after the initial period, it just sets a grace period on computation without updates in the busy case.)
>> + } >> +#endif >> } >> >> static void update_curr(struct cfs_rq *cfs_rq) >> @@ -713,6 +724,7 @@ static void update_cfs_load(struct cfs_r >> } >> >> cfs_rq->load_stamp = now; >> + cfs_rq->load_unacc_exec_time = 0; > > and drop this one? > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| ![\](/images/icornerr.gif) |