Re: [PATCH 4/6 v5] sched: propagate load during synchronous attach/detach

From	Peter Zijlstra <peterz@infradead.org>
Newsgroups	linux.kernel
Subject	Re: [PATCH 4/6 v5] sched: propagate load during synchronous attach/detach
Date	2016-10-26 13:20 +0200
Message-ID	<swupr-2Sj-7@gated-at.bofh.it> (permalink)
References	<stcfo-1yQ-11@gated-at.bofh.it> <stcfo-1yQ-29@gated-at.bofh.it> <suGXM-5ph-19@gated-at.bofh.it> <swqvw-i6-29@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

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.

Thread

[PATCH 0/6 v5] sched: reflect sched_entity move into task_group's load Vincent Guittot <vincent.guittot@linaro.org> - 2016-10-17 11:20 +0200
  [PATCH 4/6 v5] sched: propagate load during synchronous attach/detach Vincent Guittot <vincent.guittot@linaro.org> - 2016-10-17 11:20 +0200
    Re: [PATCH 4/6 v5] sched: propagate load during synchronous  attach/detach Dietmar Eggemann <dietmar.eggemann@arm.com> - 2016-10-21 14:20 +0200
      Re: [PATCH 4/6 v5] sched: propagate load during synchronous attach/detach Vincent Guittot <vincent.guittot@linaro.org> - 2016-10-26 09:10 +0200
        Re: [PATCH 4/6 v5] sched: propagate load during synchronous  attach/detach Peter Zijlstra <peterz@infradead.org> - 2016-10-26 13:20 +0200
          Re: [PATCH 4/6 v5] sched: propagate load during synchronous attach/detach Vincent Guittot <vincent.guittot@linaro.org> - 2016-10-26 14:50 +0200
    Re: [PATCH 4/6 v5] sched: propagate load during synchronous  attach/detach Peter Zijlstra <peterz@infradead.org> - 2016-10-26 13:00 +0200
      Re: [PATCH 4/6 v5] sched: propagate load during synchronous attach/detach Vincent Guittot <vincent.guittot@linaro.org> - 2016-10-26 14:40 +0200
        Re: [PATCH 4/6 v5] sched: propagate load during synchronous  attach/detach Peter Zijlstra <peterz@infradead.org> - 2016-10-26 16:30 +0200
          Re: [PATCH 4/6 v5] sched: propagate load during synchronous attach/detach Vincent Guittot <vincent.guittot@linaro.org> - 2016-10-26 20:50 +0200
    Re: [PATCH 4/6 v5] sched: propagate load during synchronous  attach/detach Peter Zijlstra <peterz@infradead.org> - 2016-10-26 13:50 +0200

csiph-web