On 26 October 2016 at 13:16, Peter Zijlstra peterz@infradead.org wrote:
On Wed, Oct 26, 2016 at 09:05:49AM +0200, Vincent Guittot wrote:
The 'detach across' and 'attach across' in detach_task_cfs_rq() and attach_entity_cfs_rq() do the same so couldn't you not create a function propagate_foo() for it? This would avoid this ifdef as well.
You could further create in your '[PATCH 1/6 v5] sched: factorize attach entity':
detach_entity_cfs_rq() { update_load_avg() detach_entity_load_avg() update_tg_load_avg() propagate_load_avg() }
and then we would have:
attach_task_cfs_rq() -> attach_entity_cfs_rq() -> propagate_foo() detach_task_cfs_rq() -> detach_entity_cfs_rq() -> propagate_foo()
I guess you didn't because it would be only called one time but this symmetric approaches are easier to remember (at least for me).
Yes i haven't created attach_entity_cfs_rq because it would be used only once. Regarding the creation of a propagate_foo function, i have just followed a similar skeleton as what is done in enqueue/dequeue_task_fair
I don't have strong opinion about creating this indirection for code readability. Others, have you got a preference ?
I think I agree with Dietmar. Duplicate code needs a helper function and it would be nice to keep symmetry, even if there's only a single call site.
OK