Re: [PATCH v5 3/7] sched: set initial value of runnable avg for newforked task

[Morten Rasmussen]

On Tue, May 07, 2013 at 04:06:33AM +0100, Paul Turner wrote:
> On Mon, May 6, 2013 at 7:18 PM, Alex Shi <alex.shi@xxxxxxxxx> wrote:
> > On 05/06/2013 06:17 PM, Paul Turner wrote:
> >>>> >> Rather than exposing the representation of load_avg_contrib to
> >>>> >> __sched_fork it might also be better to call:
> >>>> >>   __update_task_entity_contrib(&p->se)
> >>>> >> After the initialization above; this would also avoid potential bugs
> >>>> >> like the missing scale_load() above.
> >>> >
> >>> > Above simple change can not work.
> >> Could you provide additional detail here?  Note that the sum change I
> >> was suggesting above was:
> >>
> >> __sched_fork():
> >> +       p->se.avg.decay_count = 0;
> >> +       p->se.avg.runnable_avg_period = 1024;
> >> +       p->se.avg.runnable_avg_sum = 1024;
> >> +       __update_task_entity_contrib(&p->se);
> >>
> >> [ Also: move __sched_fork() beyond p->sched_reset_on_fork in sched_fork(). ]
> >
> > Thanks Paul!
> > It seems work with this change if new __sched_fork move after the
> > p->sched_reset_on_fork setting.
> >
> > But why we initial avg sum to 1024? new task may goes to sleep, the
> > initial 1024 give a unreasonable initial value.
> >
> > guess let the task accumulate itself avg sum and period is more natural.
> 
> 1024 is a full single unit period representing ~1ms of time.
> 
> The reason to store a small initial "observation" here is so that as
> when we reach our next period edge our load converges (presumably
> down) towards its true target more smoothly; as well as providing a
> task additional protection from being considered "small" through
> start-up.
> 

Agree. The tracked load of new tasks is often very unstable during first
couple of ms on loaded systems. I'm not sure if 1024 is the right
initial value. It may need to be larger.

Morten

> >>
> >>> > We had talked this solution months ago. And get agreement on this patch.
> >>> > https://lkml.org/lkml/2013/2/20/48  :)
> >> Yes, I made the same suggestion in the last round, see:
> >> https://lkml.org/lkml/2013/2/19/176
> >>
> >> Your reply there seems like an ack of my suggestion, the only
> >> difference I'm seeing is that using __update_task_entity_contrib() as
> >> originally suggested is safer since it keeps the representation of
> >> load_avg_contrib opaque.
> >
> > Yes, using __update_task_entity_contrib make load_avg_contrib opaque.
> > but just initial value 1024 is a bit arbitrary.
> >>
> >
> >
> > --
> > Thanks
> >     Alex
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/