Messages in this thread | | | Date | Tue, 28 Sep 2010 16:23:21 -0700 | Subject | Re: broken behavior in cfs when moving threads between cgroups | From | Dima Zavin <> |
| |
Actually, that won't work either as it can allow a thread to gain vruntime by being moved out and then immediately back into a group.
--Dima
2010/9/28 Dima Zavin <dmitriyz@google.com>: > Mike, > > Thanks for the reply. > >> I don't really see the relevance of an entity's lag in it's previous >> group, so would tend toward entry at parity. >> >> Maybe the move _should_ be treated as a fork, for the same reason we do >> START_DEBIT, but if so, seems to me it's irrelevant whether the task is >> currently sleeping or not, it's going to run for the first time in it's >> new home just as any runnable task, and may do so very soon. OTOH, >> frequent moves with that hefty START_DEBIT price tag could hurt very >> badly, and makes caring about previous lag seem pointless. >> >> -Mike >> >> (untested) >> >> diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c >> index 9b5b4f8..4dc6b2f 100644 >> --- a/kernel/sched_fair.c >> +++ b/kernel/sched_fair.c >> @@ -3827,10 +3827,14 @@ static void set_curr_task_fair(struct rq *rq) >> static void moved_group_fair(struct task_struct *p, int on_rq) >> { >> struct cfs_rq *cfs_rq = task_cfs_rq(p); >> + struct sched_entity *se = &p->se; >> + u64 vruntime = 0; >> >> update_curr(cfs_rq); >> if (!on_rq) >> - place_entity(cfs_rq, &p->se, 1); >> + vruntime = cfs_rq->min_vruntime; >> + >> + se->vruntime = vruntime; >> } >> #endif > > Hmm. Would we really want to give the sleeping task such a bump in the > new group? Why not just start it off at min_vruntime and let > place_entity do it's thing? Something like... > > diff --git a/kernel/sched_fair.c b/kernel/sched_fair.c > index a7be83c..4656231 100644 > --- a/kernel/sched_fair.c > +++ b/kernel/sched_fair.c > @@ -3622,10 +3622,15 @@ static void set_curr_task_fair(struct rq *rq) > static void moved_group_fair(struct task_struct *p, int on_rq) > { > struct cfs_rq *cfs_rq = task_cfs_rq(p); > + struct sched_entity *se = &p->se; > > update_curr(cfs_rq); > - if (!on_rq) > + if (on_rq) { > + se->vruntime = 0; > + } else { > + se->vruntime = cfs_rq->min_vruntime; > place_entity(cfs_rq, &p->se, 1); > + } > } > #endif > -- 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/
| |