On Tue, Oct 13, 2015 at 02:04:38PM +0200, Peter Zijlstra wrote: > On Tue, Oct 13, 2015 at 08:26:45PM +0900, Byungchul Park wrote: > > On Tue, Oct 13, 2015 at 11:06:54AM +0200, Peter Zijlstra wrote: > > > On Mon, Oct 05, 2015 at 06:16:23PM +0900, byungchul.p...@lge.com wrote: > > > > From: Byungchul Park <byungchul.p...@lge.com> > > > > > > > > Original fair sched class can handle the cgroup change occured within > > > > its > > > > class with task_move_group_fair(), but there is no way to know it if the > > > > change happened outside. This patch makes the fair sched class can > > > > handle > > > > the change of cgroup which happened even at other sched class. > > > > > > > > Additionally, it makes sched_move_task() more flexable so that any other > > > > sched class can add task_move_group_xx() callback easily in future when > > > > it is needed. > > > > > > I don't get the problem... when !fair, set_task_rq() will do what needs > > > doing. > > > > set_task_rq() changes se's cfs_rq properly. > > > > > > > > The only reason we need task_move_group_fair() is the extra accounting > > > required when we actually _are_ of the fair class, it needs to > > > unaccount, move and reaccount. > > > > i agree with you mostly. but let's consider following sequence. > > > > 1. switch se's class from fair to rt > > 2. change se's group within the rt class > > 3. switch se's class back to fair > > > > now, se->avg.last_update_time has a wrong value which is not synced with > > the current cfs_rq yet before calling attach_entity_load_avg(). so > > ATTACH_AGE_LOAD won't work expectedly. to be honest with you, no problem > > if we disable ATTACH_AGE_LOAD. but i think ATTACH_AGE_LOAD is a valuable > > feature, so i hope this patch will be added so that the ATTACH_AGE_LOAD > > feature works properly. > > Ah, see details like that make or break a Changelog, since you've > clearly thought about it, you might as well write it down and safe me > the trouble of trying to puzzle it out on me own ;-) > > OK, now that I understand the problem, let me consider it a bit.
So the problem I have with your approach is that I would prefer to isolate the classes as much as possible. If its not currently of a class, we should not call into it. Now, there's a few exceptions to that already, but I would really prefer not to make it worse. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
