Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1742199 > unrolled thread

Re: [PATCH -v2 04/18] sched/fair: Remove se->load.weight from se->avg.load_sum

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH -v2 04/18] sched/fair: Remove se->load.weight from  se->avg.load_sum Morten Rasmussen <morten.rasmussen@arm.com> - 2017-09-29 17:30 +0200
    Re: [PATCH -v2 04/18] sched/fair: Remove se->load.weight from  se->avg.load_sum Peter Zijlstra <peterz@infradead.org> - 2017-09-29 18:40 +0200

#1742199 — Re: [PATCH -v2 04/18] sched/fair: Remove se->load.weight from se->avg.load_sum

On Fri, Sep 01, 2017 at 03:21:03PM +0200, Peter Zijlstra wrote:
> +/*
> + * sched_entity:
> + *
> + *   load_sum := runnable_sum
> + *   load_avg = se_weight(se) * runnable_avg
> + *
> + * cfq_rs:

I think this should be "cfs_rq" instead.

> + *
> + *   load_sum = \Sum se_weight(se) * se->avg.load_sum
> + *   load_avg = \Sum se->avg.load_avg
> + */

I find it a bit confusing that load_sum and load_avg have different
definitions, but I guess I will discover why dropping weight from
se->avg.load_sum helps a bit later. We can't do the same for cfs_rq as
it is a \Sum of sums where we add/remove contributions when tasks
migrate.

Have we defined the relation between runnable_sum and runnable_avg in a
comment somewhere already? Otherwise it might be helpful to add. It is
in the code of course :-)

[toc] | [next] | [standalone]

#1742238

On Fri, Sep 29, 2017 at 04:26:10PM +0100, Morten Rasmussen wrote:
> On Fri, Sep 01, 2017 at 03:21:03PM +0200, Peter Zijlstra wrote:
> > +/*
> > + * sched_entity:
> > + *
> > + *   load_sum := runnable_sum
> > + *   load_avg = se_weight(se) * runnable_avg
> > + *
> > + * cfq_rs:
> 
> I think this should be "cfs_rq" instead.

Quite.

> > + *
> > + *   load_sum = \Sum se_weight(se) * se->avg.load_sum
> > + *   load_avg = \Sum se->avg.load_avg
> > + */
> 
> I find it a bit confusing that load_sum and load_avg have different
> definitions, but I guess I will discover why dropping weight from
> se->avg.load_sum helps a bit later.

It makes changing the weight cheaper, we don't have to divide the old
weight out and then multiple by the new weight.

Up until this patch set, we mixed the old and new weight in the sum,
with the result that a weight change takes a while to propagate.

But we want to change weight instantly. See:

  sched/fair: More accurate reweight_entity()

> We can't do the same for cfs_rq as it is a \Sum of sums where we
> add/remove contributions when tasks migrate.

Just so. The patch:

  sched/fair: Rewrite PELT migration propagation

Has text on this:

+ * Because while for entities historical weight is not important and we
+ * really only care about our future and therefore can consider a pure
+ * runnable sum, runqueues can NOT do this.
+ *
+ * We specifically want runqueues to have a load_avg that includes
+ * historical weights. Those represent the blocked load, the load we expect
+ * to (shortly) return to us. This only works by keeping the weights as
+ * integral part of the sum. We therefore cannot decompose as per (3).

> Have we defined the relation between runnable_sum and runnable_avg in a
> comment somewhere already? Otherwise it might be helpful to add. It is
> in the code of course :-)

I think this comment here is about it... But who knows. Please keep this
as a note while you read the rest of the series. If you find its still
insufficiently addressed at the end, I'm sure we can do another patch
with moar comments still ;-)

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web