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

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