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


Groups > linux.kernel > #1230514 > unrolled thread

Re: [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity_orig

Started bybsegall@google.com
First post2015-09-22 19:20 +0200
Last post2015-09-30 15:00 +0200
Articles 5 — 3 participants

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 5/6] sched/fair: Get rid of scaling utilization by capacity_orig bsegall@google.com - 2015-09-22 19:20 +0200
    Re: [PATCH 5/6] sched/fair: Get rid of scaling utilization by  capacity_orig Yuyang Du <yuyang.du@intel.com> - 2015-09-23 09:20 +0200
      Re: [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity_orig bsegall@google.com - 2015-09-23 19:00 +0200
        Re: [PATCH 5/6] sched/fair: Get rid of scaling utilization by  capacity_orig Yuyang Du <yuyang.du@intel.com> - 2015-09-24 10:20 +0200
    Re: [PATCH 5/6] sched/fair: Get rid of scaling utilization by  capacity_orig Peter Zijlstra <peterz@infradead.org> - 2015-09-30 15:00 +0200

#1230514 — Re: [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity_orig

Frombsegall@google.com
Date2015-09-22 19:20 +0200
SubjectRe: [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity_orig
Message-ID<qbzou-3mH-13@gated-at.bofh.it>
Yuyang Du <yuyang.du@intel.com> writes:

> On Mon, Sep 21, 2015 at 10:30:04AM -0700, bsegall@google.com wrote:
>> > But first, I think as load_sum and load_avg can afford NICE_0_LOAD with either high
>> > or low resolution. So we have no reason to have low resolution (10bits) load_avg
>> > when NICE_0_LOAD has high resolution (20bits), because load_avg = runnable% * load,
>> > as opposed to now we have load_avg = runnable% * scale_load_down(load).
>> >
>> > We get rid of all scale_load_down() for runnable load average?
>> 
>> Hmm, LOAD_AVG_MAX * prio_to_weight[0] is 4237627662, ie barely within a
>> 32-bit unsigned long, but in fact LOAD_AVG_MAX * MAX_SHARES is already
>> going to give errors on 32-bit (even with the old code in fact). This
>> should probably be fixed... somehow (dividing by 4 for load_sum on
>> 32-bit would work, though be ugly. Reducing MAX_SHARES by 2 bits on
>> 32-bit might have made sense but would be a weird difference between 32
>> and 64, and could break userspace anyway, so it's presumably too late
>> for that).
>> 
>> 64-bit has ~30 bits free, so this would be fine so long as SLR is 0 on
>> 32-bit.
>> 
>
> load_avg has no LOAD_AVG_MAX term in it, it is runnable% * load, IOW, load_avg <= load.
> So, on 32bit, cfs_rq's load_avg can host 2^32/prio_to_weight[0]/1024 = 47, with 20bits
> load resolution. This is ok, because struct load_weight's load is also unsigned
> long. If overflown, cfs_rq->load.weight will be overflown in the first place.
>
> However, after a second thought, this is not quite right. Because load_avg is not
> necessarily no greater than load, since load_avg has blocked load in it. Although,
> load_avg is still at the same level as load (converging to be <= load), we may not
> want the risk to overflow on 32bit.

Yeah, I missed that load_sum was u64 and only load_avg was long. This
means we're fine on 32-bit with no SLR (or more precisely, cfs_rq
runnable_load_avg can overflow, but only when cfs_rq load.weight does,
so whatever). On 64-bit you can currently have 2^36 cgroups or 2^37
tasks before load.weight overflows, and ~2^31 tasks before
runnable_load_avg does, which is obviously fine (and in fact you hit
PID_MAX_LIMIT and even if you had the cpu/ram/etc to not fall over).

Now, applying SLR to runnable_load_avg would cut this down to ~2^21
tasks running at once or 2^20 with cgroups, which is technically
allowed, though it seems utterly implausible (especially since this
would have to all be on one cpu). If SLR was increased as peterz asked
about, this could be an issue though.

All that said, using SLR on load_sum/load_avg as opposed to cfs_rq
runnable_load_avg would be fine, as they're limited to only one
task/cgroup's weight. Having it SLRed and cfs_rq not would be a
little odd, but not impossible.

>
>> > +/*
>> > + * NICE_0's weight (visible to user) and its load (invisible to user) have
>> > + * independent resolution, but they should be well calibrated. We use scale_load()
>> > + * and scale_load_down(w) to convert between them, the following must be true:
>> > + * scale_load(prio_to_weight[20]) == NICE_0_LOAD
>> > + */
>> >  #define NICE_0_LOAD		SCHED_LOAD_SCALE
>> >  #define NICE_0_SHIFT		SCHED_LOAD_SHIFT
>> 
>> I still think tying the scale_load shift to be the same as the
>> SCHED_CAPACITY/etc shift is silly, and tying the NICE_0_LOAD/SHIFT in is
>> worse. Honestly if I was going to change anything it would be to define
>> NICE_0_LOAD/SHIFT entirely separately from SCHED_LOAD_SCALE/SHIFT.
>
> If NICE_0_LOAD is nice-0's load, and if SCHED_LOAD_SHIFT is to say how to get 
> nice-0's load, I don't understand why you want to separate them.

SCHED_LOAD_SHIFT is not how to get nice-0's load, it just happens to
have the same value as NICE_0_SHIFT. (I think anyway, SCHED_LOAD_* is
used in precisely one place other than the newish util_avg, and as I
mentioned it's not remotely clear what compute_imbalance is doing theer)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]


#1231186 — Re: [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity_orig

FromYuyang Du <yuyang.du@intel.com>
Date2015-09-23 09:20 +0200
SubjectRe: [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity_orig
Message-ID<qbMvn-5Ad-3@gated-at.bofh.it>
In reply to#1230514
On Tue, Sep 22, 2015 at 10:18:30AM -0700, bsegall@google.com wrote:
> Yuyang Du <yuyang.du@intel.com> writes:
> 
> > On Mon, Sep 21, 2015 at 10:30:04AM -0700, bsegall@google.com wrote:
> >> > But first, I think as load_sum and load_avg can afford NICE_0_LOAD with either high
> >> > or low resolution. So we have no reason to have low resolution (10bits) load_avg
> >> > when NICE_0_LOAD has high resolution (20bits), because load_avg = runnable% * load,
> >> > as opposed to now we have load_avg = runnable% * scale_load_down(load).
> >> >
> >> > We get rid of all scale_load_down() for runnable load average?
> >> 
> >> Hmm, LOAD_AVG_MAX * prio_to_weight[0] is 4237627662, ie barely within a
> >> 32-bit unsigned long, but in fact LOAD_AVG_MAX * MAX_SHARES is already
> >> going to give errors on 32-bit (even with the old code in fact). This
> >> should probably be fixed... somehow (dividing by 4 for load_sum on
> >> 32-bit would work, though be ugly. Reducing MAX_SHARES by 2 bits on
> >> 32-bit might have made sense but would be a weird difference between 32
> >> and 64, and could break userspace anyway, so it's presumably too late
> >> for that).
> >> 
> >> 64-bit has ~30 bits free, so this would be fine so long as SLR is 0 on
> >> 32-bit.
> >> 
> >
> > load_avg has no LOAD_AVG_MAX term in it, it is runnable% * load, IOW, load_avg <= load.
> > So, on 32bit, cfs_rq's load_avg can host 2^32/prio_to_weight[0]/1024 = 47, with 20bits
> > load resolution. This is ok, because struct load_weight's load is also unsigned
> > long. If overflown, cfs_rq->load.weight will be overflown in the first place.
> >
> > However, after a second thought, this is not quite right. Because load_avg is not
> > necessarily no greater than load, since load_avg has blocked load in it. Although,
> > load_avg is still at the same level as load (converging to be <= load), we may not
> > want the risk to overflow on 32bit.
 
This second thought made a mistake (what was wrong with me). load_avg is for sure
no greater than load with or without blocked load.

With that said, it really does not matter what the following numbers are, 32bit or
64bit machine. What matters is that cfs_rq->load.weight is one that needs to worry
whether overflow or not, not the load_avg. It is as simple as that.

With that, I think we can and should get rid of the scale_load_down() for load_avg.

> Yeah, I missed that load_sum was u64 and only load_avg was long. This
> means we're fine on 32-bit with no SLR (or more precisely, cfs_rq
> runnable_load_avg can overflow, but only when cfs_rq load.weight does,
> so whatever). On 64-bit you can currently have 2^36 cgroups or 2^37
> tasks before load.weight overflows, and ~2^31 tasks before
> runnable_load_avg does, which is obviously fine (and in fact you hit
> PID_MAX_LIMIT and even if you had the cpu/ram/etc to not fall over).
> 
> Now, applying SLR to runnable_load_avg would cut this down to ~2^21
> tasks running at once or 2^20 with cgroups, which is technically
> allowed, though it seems utterly implausible (especially since this
> would have to all be on one cpu). If SLR was increased as peterz asked
> about, this could be an issue though.
> 
> All that said, using SLR on load_sum/load_avg as opposed to cfs_rq
> runnable_load_avg would be fine, as they're limited to only one
> task/cgroup's weight. Having it SLRed and cfs_rq not would be a
> little odd, but not impossible.
 

> > If NICE_0_LOAD is nice-0's load, and if SCHED_LOAD_SHIFT is to say how to get 
> > nice-0's load, I don't understand why you want to separate them.
> 
> SCHED_LOAD_SHIFT is not how to get nice-0's load, it just happens to
> have the same value as NICE_0_SHIFT. (I think anyway, SCHED_LOAD_* is
> used in precisely one place other than the newish util_avg, and as I
> mentioned it's not remotely clear what compute_imbalance is doing theer)

Yes, it is not clear to me either.

With the above proposal to get rid of scale_load_down() for load_avg, so I think
now we can remove SCHED_LOAD_*, and rename scale_load() to user_to_kernel_load(),
and raname scale_load_down() to kernel_to_user_load().

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

[toc] | [prev] | [next] | [standalone]


#1231573

Frombsegall@google.com
Date2015-09-23 19:00 +0200
Message-ID<qbVyH-1BU-35@gated-at.bofh.it>
In reply to#1231186
Yuyang Du <yuyang.du@intel.com> writes:

> On Tue, Sep 22, 2015 at 10:18:30AM -0700, bsegall@google.com wrote:
>> Yuyang Du <yuyang.du@intel.com> writes:
>> 
>> > On Mon, Sep 21, 2015 at 10:30:04AM -0700, bsegall@google.com wrote:
>> >> > But first, I think as load_sum and load_avg can afford NICE_0_LOAD with either high
>> >> > or low resolution. So we have no reason to have low resolution (10bits) load_avg
>> >> > when NICE_0_LOAD has high resolution (20bits), because load_avg = runnable% * load,
>> >> > as opposed to now we have load_avg = runnable% * scale_load_down(load).
>> >> >
>> >> > We get rid of all scale_load_down() for runnable load average?
>> >> 
>> >> Hmm, LOAD_AVG_MAX * prio_to_weight[0] is 4237627662, ie barely within a
>> >> 32-bit unsigned long, but in fact LOAD_AVG_MAX * MAX_SHARES is already
>> >> going to give errors on 32-bit (even with the old code in fact). This
>> >> should probably be fixed... somehow (dividing by 4 for load_sum on
>> >> 32-bit would work, though be ugly. Reducing MAX_SHARES by 2 bits on
>> >> 32-bit might have made sense but would be a weird difference between 32
>> >> and 64, and could break userspace anyway, so it's presumably too late
>> >> for that).
>> >> 
>> >> 64-bit has ~30 bits free, so this would be fine so long as SLR is 0 on
>> >> 32-bit.
>> >> 
>> >
>> > load_avg has no LOAD_AVG_MAX term in it, it is runnable% * load, IOW, load_avg <= load.
>> > So, on 32bit, cfs_rq's load_avg can host 2^32/prio_to_weight[0]/1024 = 47, with 20bits
>> > load resolution. This is ok, because struct load_weight's load is also unsigned
>> > long. If overflown, cfs_rq->load.weight will be overflown in the first place.
>> >
>> > However, after a second thought, this is not quite right. Because load_avg is not
>> > necessarily no greater than load, since load_avg has blocked load in it. Although,
>> > load_avg is still at the same level as load (converging to be <= load), we may not
>> > want the risk to overflow on 32bit.
>  
> This second thought made a mistake (what was wrong with me). load_avg is for sure
> no greater than load with or without blocked load.
>
> With that said, it really does not matter what the following numbers are, 32bit or
> 64bit machine. What matters is that cfs_rq->load.weight is one that needs to worry
> whether overflow or not, not the load_avg. It is as simple as that.
>
> With that, I think we can and should get rid of the scale_load_down()
> for load_avg.

load_avg yes is bounded by load.weight, but on 64-bit load_sum is only
bounded by load.weight * LOAD_AVG_MAX and is the same size as
load.weight (as I said below). There's still space for anything
reasonable though with 10 bits of SLR.

>
>> Yeah, I missed that load_sum was u64 and only load_avg was long. This
>> means we're fine on 32-bit with no SLR (or more precisely, cfs_rq
>> runnable_load_avg can overflow, but only when cfs_rq load.weight does,
>> so whatever). On 64-bit you can currently have 2^36 cgroups or 2^37
>> tasks before load.weight overflows, and ~2^31 tasks before
>> runnable_load_avg does, which is obviously fine (and in fact you hit
>> PID_MAX_LIMIT and even if you had the cpu/ram/etc to not fall over).
>> 
>> Now, applying SLR to runnable_load_avg would cut this down to ~2^21
>> tasks running at once or 2^20 with cgroups, which is technically
>> allowed, though it seems utterly implausible (especially since this
>> would have to all be on one cpu). If SLR was increased as peterz asked
>> about, this could be an issue though.
>> 
>> All that said, using SLR on load_sum/load_avg as opposed to cfs_rq
>> runnable_load_avg would be fine, as they're limited to only one
>> task/cgroup's weight. Having it SLRed and cfs_rq not would be a
>> little odd, but not impossible.
>  
>
>> > If NICE_0_LOAD is nice-0's load, and if SCHED_LOAD_SHIFT is to say how to get 
>> > nice-0's load, I don't understand why you want to separate them.
>> 
>> SCHED_LOAD_SHIFT is not how to get nice-0's load, it just happens to
>> have the same value as NICE_0_SHIFT. (I think anyway, SCHED_LOAD_* is
>> used in precisely one place other than the newish util_avg, and as I
>> mentioned it's not remotely clear what compute_imbalance is doing theer)
>
> Yes, it is not clear to me either.
>
> With the above proposal to get rid of scale_load_down() for load_avg, so I think
> now we can remove SCHED_LOAD_*, and rename scale_load() to user_to_kernel_load(),
> and raname scale_load_down() to kernel_to_user_load().
>
> Hmm?

I have no opinion on renaming the scale_load functions, it's certainly
reasonable, but the scale_load names seem fine too.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1231938 — Re: [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity_orig

FromYuyang Du <yuyang.du@intel.com>
Date2015-09-24 10:20 +0200
SubjectRe: [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity_orig
Message-ID<qc9V0-5OU-11@gated-at.bofh.it>
In reply to#1231573
On Wed, Sep 23, 2015 at 09:54:08AM -0700, bsegall@google.com wrote:
> > This second thought made a mistake (what was wrong with me). load_avg is for sure
> > no greater than load with or without blocked load.
> >
> > With that said, it really does not matter what the following numbers are, 32bit or
> > 64bit machine. What matters is that cfs_rq->load.weight is one that needs to worry
> > whether overflow or not, not the load_avg. It is as simple as that.
> >
> > With that, I think we can and should get rid of the scale_load_down()
> > for load_avg.
> 
> load_avg yes is bounded by load.weight, but on 64-bit load_sum is only
> bounded by load.weight * LOAD_AVG_MAX and is the same size as
> load.weight (as I said below). There's still space for anything
> reasonable though with 10 bits of SLR.
 
You are absolutely right.

> >> > If NICE_0_LOAD is nice-0's load, and if SCHED_LOAD_SHIFT is to say how to get 
> >> > nice-0's load, I don't understand why you want to separate them.
> >> 
> >> SCHED_LOAD_SHIFT is not how to get nice-0's load, it just happens to
> >> have the same value as NICE_0_SHIFT. (I think anyway, SCHED_LOAD_* is
> >> used in precisely one place other than the newish util_avg, and as I
> >> mentioned it's not remotely clear what compute_imbalance is doing theer)
> >
> > Yes, it is not clear to me either.
> >
> > With the above proposal to get rid of scale_load_down() for load_avg, so I think
> > now we can remove SCHED_LOAD_*, and rename scale_load() to user_to_kernel_load(),
> > and raname scale_load_down() to kernel_to_user_load().
> >
> > Hmm?
> 
> I have no opinion on renaming the scale_load functions, it's certainly
> reasonable, but the scale_load names seem fine too.

Without scale_load_down() in load_avg, it seems they are only used when
reading/writing load between user and kernel. I will ponder more, but
lets see whether others have opinion.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1236256 — Re: [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity_orig

FromPeter Zijlstra <peterz@infradead.org>
Date2015-09-30 15:00 +0200
SubjectRe: [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity_orig
Message-ID<qep9g-5rz-7@gated-at.bofh.it>
In reply to#1230514
On Tue, Sep 22, 2015 at 10:18:30AM -0700, bsegall@google.com wrote:
> If SLR was increased as peterz asked
> about

Right, so I was under the impression that you (Google) run with it
increased and in mainline its currently dead code.

So if its valuable to you guys we should fix in mainline.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web