On 6 June 2016 at 21:05, Yuyang Du <yuyang...@intel.com> wrote: > Hi Vincent, > > On Mon, Jun 06, 2016 at 02:32:39PM +0200, Vincent Guittot wrote: >> > Unlike the switch to fair class case, if the task is on_rq, it will be >> > enqueued after we move task groups, so the simplest solution is to reset >> > the task's last_update_time when we do task_move_group(), but not to >> > attach sched avgs in task_move_group(), and then let enqueue_task() do >> > the sched avgs attachment. >> >> According to the review of the previous version >> http://www.gossamer-threads.com/lists/linux/kernel/2450678#2450678, >> only the use case switch to fair with a task that has never been >> queued as a CFS task before, can have the issue and this will not >> happen for other use cases described above. >> So can you limit the description in the commit message to just this >> use case unless you have discovered new use cases and in this case >> please add the description. > > I assure you that I will, :) > >> Then, the problem with this use case, comes that last_update_time == 0 >> has a special meaning ( task has migrated ) and we initialize >> last_update_time with this value. > > In general, the meaning of last_update_time == 0 is: not yet attached to > any cfs queue. > >> A much more simple solution would be >> to prevent last_update_time to be initialized with this special value. >> We can initialize the last_update_time of a sched_entity to 1 as an >> example which is easier than these changes. > > Then at least, we must differentiate CONFIG_FAIR_GROUP_SCHED and > !CONFIG_FAIR_GROUP_SCHED. And I am not sure whether this is a simpler
Why do you want to differentiate ? we already have sa->last_update_time = 0; in init_entity_runnable_average. We just have to change it by sa->last_update_time = 1; > solution, as I haven't sorted out the whole thing. IMO, this solution > seems a little too hacky and I'd prefer not if we have alternative.
