lkml.org 
[lkml]   [2016]   [Oct]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 4/6 v5] sched: propagate load during synchronous attach/detach
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.

\
 
 \ /
  Last update: 2016-10-26 13:17    [W:0.198 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site