Messages in this thread | | | Date | Mon, 24 Jan 2011 08:18:11 +0200 | Subject | Re: [PATCH V2] sched: fix autogroup nice tune on UP | From | Pekka Enberg <> |
| |
Hi,
On Mon, Jan 24, 2011 at 8:11 AM, Yong Zhang <yong.zhang0@gmail.com> wrote: > On Mon, Jan 24, 2011 at 1:54 PM, Pekka Enberg <penberg@kernel.org> wrote: >> On Mon, Jan 24, 2011 at 7:40 AM, Yong Zhang <yong.zhang0@gmail.com> wrote: >>> +static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta) >>> +{ >>> + struct task_group *tg; >>> + struct sched_entity *se; >>> + >>> + if (!cfs_rq) >>> + return; >>> + >>> + tg = cfs_rq->tg; >>> + se = tg->se[0]; >>> + if (!se) >>> + return; >>> + if (likely(se->load.weight == tg->shares)) >>> + return; >>> + reweight_entity(cfs_rq_of(se), se, tg->shares); >>> +} >> >> Wouldn't it be cleaner if we'd separate the shares calculation in a >> separate helper function that's just >> >> return tg->shares; > > I'm not sure I get your point correctly. > You mean the two tg->shares above, right? > > If so, yeah, we can declare a variable for that. > >> >> for UP and extract the current logic for the SMP version? >> > > This is the UP specific version, I don't touch SMP version. > On SMP, update_cfs_shares() is more complex.
I proposed extracting the shares calculation logic in update_cfs_shares() to reduce code duplication for the SMP and UP patch. So something like this:
#ifdef CONFIG_SMP static long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group *tg, long weight_delta) { long load_weight, load, shares;
load = cfs_rq->load.weight + weight_delta;
load_weight = atomic_read(&tg->load_weight); load_weight -= cfs_rq->load_contribution; load_weight += load;
shares = (tg->shares * load); if (load_weight) shares /= load_weight;
if (shares < MIN_SHARES) shares = MIN_SHARES; if (shares > tg->shares) shares = tg->shares;
return shares; } #else static long calc_cfs_shares(struct cfs_rq *cfs_rq, struct task_group *tg, long weight_delta) { return tg->shares; } #endif
static void update_cfs_shares(struct cfs_rq *cfs_rq, long weight_delta) { struct task_group *tg; struct sched_entity *se; long shares;
if (!cfs_rq) return;
tg = cfs_rq->tg; se = tg->se[cpu_of(rq_of(cfs_rq))]; if (!se) return;
#ifndef CONFIG_SMP if (likely(se->load.weight == tg->shares)) return; #else
shares = calc_cfs_shares(cfs_rq, tg, weight_delta);
reweight_entity(cfs_rq_of(se), se, shares); } -- 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/
| |