public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Morten Rasmussen <morten.rasmussen@arm.com>
Cc: Yuyang Du <yuyang.du@intel.com>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	mingo@redhat.com, linux-kernel@vger.kernel.org,
	Paul Turner <pjt@google.com>, Ben Segall <bsegall@google.com>
Subject: Re: [PATCH] sched/fair: fix mul overflow on 32-bit systems
Date: Mon, 14 Dec 2015 15:20:21 +0100	[thread overview]
Message-ID: <20151214142021.GO6357@twins.programming.kicks-ass.net> (raw)
In-Reply-To: <20151214130723.GB9870@e105550-lin.cambridge.arm.com>

On Mon, Dec 14, 2015 at 01:07:26PM +0000, Morten Rasmussen wrote:

> Agreed, >100% is a transient state (which can be rather long) which only
> means over-utilized, nothing more. Would you like the metric itself to
> be changed to saturate at 100% or just cap it to 100% when used?

We already cap it when using it IIRC. But no, I was thinking of the
measure itself.

> It is not straight forward to provide a bound on the sum.

Agreed..

> There isn't one for load_avg either.

But that one is fundamentally unbound, whereas the util thing is
fundamentally bound, except our implementation isn't.

> If we want to guarantee an upper bound for
> cfs_rq->avg.util_sum we have to somehow cap the se->avg.util_avg
> contributions for each sched_entity. This cap depends on the cpu and how
> many other tasks are associated with that cpu. The cap may have to
> change when tasks migrate.

Yep, blows :-)

> > However, I think that makes sense, but would propose doing it
> > differently. That condition is generally a maximum (assuming proper
> > functioning of the weight based scheduling etc..) for any one task, so
> > on migrate we can hard clip to this value.

> Why use load.weight to scale util_avg? It is affected by priority. Isn't
> just the ratio 1/nr_running that you are after?

Remember, the util thing is based on running, so assuming each task
always wants to run, each task gets to run w_i/\Sum_j w_j due to CFS
being a weighted fair queueing thingy.

> IIUC, you propose to clip the sum itself. In which case you are running
> into trouble when removing tasks. You don't know how much to remove from
> the clipped sum.

Right, then we'll have to slowly gain it again.

> Another problem is that load.weight is just a snapshot while
> avg.util_avg includes tasks that are not currently on the rq so the
> scaling factor is probably bigger than what you want.

Our weight guestimates also include non running (aka blocked) tasks,
right?

> If we leave the sum as it is (unclipped) add/remove shouldn't give us
> any problems. The only problem is the overflow, which is solved by using
> a 64bit type for load_avg. That is not an acceptable solution?

It might be. After all, any time any of this is needed we're CPU bound
and the utilization measure is pointless anyway. That measure only
matters if its small and the sum is 'small'. After that its back to the
normal load based thingy.

  reply	other threads:[~2015-12-14 14:20 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-11 12:55 [PATCH] sched/fair: fix mul overflow on 32-bit systems Andrey Ryabinin
2015-12-11 13:25 ` Peter Zijlstra
2015-12-11 13:36   ` Peter Zijlstra
2015-12-11 14:00     ` Andrey Ryabinin
2015-12-11 17:57       ` Morten Rasmussen
2015-12-11 18:32         ` Dietmar Eggemann
2015-12-11 19:18           ` bsegall
2015-12-13 21:02             ` Yuyang Du
2015-12-14 12:32             ` Morten Rasmussen
2015-12-14 17:51               ` bsegall
2015-12-13 22:42         ` Yuyang Du
2015-12-14 11:54           ` Peter Zijlstra
2015-12-14 13:07             ` Morten Rasmussen
2015-12-14 14:20               ` Peter Zijlstra [this message]
2015-12-14 14:46                 ` Morten Rasmussen
2015-12-15  2:22             ` Yuyang Du
2015-12-15 21:56               ` Steve Muckle
2015-12-18  2:33                 ` Yuyang Du
2016-01-03 23:14                   ` Yuyang Du
2015-12-11 17:58       ` bsegall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151214142021.GO6357@twins.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=aryabinin@virtuozzo.com \
    --cc=bsegall@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=morten.rasmussen@arm.com \
    --cc=pjt@google.com \
    --cc=yuyang.du@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox