lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 7 Sep 2015 19:54:18 +0100 From: Dietmar Eggemann <dietmar.eggemann@....com> To: Vincent Guittot <vincent.guittot@...aro.org> Cc: Steve Muckle <steve.muckle@...aro.org>, Morten Rasmussen <Morten.Rasmussen@....com>, "peterz@...radead.org" <peterz@...radead.org>, "mingo@...hat.com" <mingo@...hat.com>, "daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>, "yuyang.du@...el.com" <yuyang.du@...el.com>, "mturquette@...libre.com" <mturquette@...libre.com>, "rjw@...ysocki.net" <rjw@...ysocki.net>, Juri Lelli <Juri.Lelli@....com>, "sgurrappadi@...dia.com" <sgurrappadi@...dia.com>, "pang.xunlei@....com.cn" <pang.xunlei@....com.cn>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org> Subject: Re: [PATCH 5/6] sched/fair: Get rid of scaling utilization by capacity_orig On 07/09/15 17:21, Vincent Guittot wrote: > On 7 September 2015 at 17:37, Dietmar Eggemann <dietmar.eggemann@....com> wrote: >> On 04/09/15 00:51, Steve Muckle wrote: >>> Hi Morten, Dietmar, >>> >>> On 08/14/2015 09:23 AM, Morten Rasmussen wrote: >>> ... >>>> + * cfs_rq.avg.util_avg is the sum of running time of runnable tasks plus the >>>> + * recent utilization of currently non-runnable tasks on a CPU. It represents >>>> + * the amount of utilization of a CPU in the range [0..capacity_orig] where >>> >>> I see util_sum is scaled by SCHED_LOAD_SHIFT at the end of >>> __update_load_avg(). If there is now an assumption that util_avg may be >>> used directly as a capacity value, should it be changed to >>> SCHED_CAPACITY_SHIFT? These are equal right now, not sure if they will >>> always be or if they can be combined. >> >> You're referring to the code line >> >> 2647 sa->util_avg = (sa->util_sum << SCHED_LOAD_SHIFT) / LOAD_AVG_MAX; >> >> in __update_load_avg()? >> >> Here we actually scale by 'SCHED_LOAD_SCALE/LOAD_AVG_MAX' so both values are >> load related. > > I agree with Steve that there is an issue from a unit point of view > > sa->util_sum and LOAD_AVG_MAX have the same unit so sa->util_avg is a > load because of << SCHED_LOAD_SHIFT) > > Before this patch , the translation from load to capacity unit was > done in get_cpu_usage with "* capacity) >> SCHED_LOAD_SHIFT" > > So you still have to change the unit from load to capacity with a "/ > SCHED_LOAD_SCALE * SCHED_CAPACITY_SCALE" somewhere. > > sa->util_avg = ((sa->util_sum << SCHED_LOAD_SHIFT) /SCHED_LOAD_SCALE * > SCHED_CAPACITY_SCALE / LOAD_AVG_MAX = (sa->util_sum << > SCHED_CAPACITY_SHIFT) / LOAD_AVG_MAX; I see the point but IMHO this will only be necessary if the SCHED_LOAD_RESOLUTION stuff gets re-enabled again. It's not really about utilization or capacity units but rather about using the same SCALE/SHIFT values for both sides, right? I always thought that scale_load_down() takes care of that. So shouldn't: diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 3445d2fb38f4..b80f799aface 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -2644,7 +2644,7 @@ __update_load_avg(u64 now, int cpu, struct sched_avg *sa, cfs_rq->runnable_load_avg = div_u64(cfs_rq->runnable_load_sum, LOAD_AVG_MAX); } - sa->util_avg = (sa->util_sum << SCHED_LOAD_SHIFT) / LOAD_AVG_MAX; + sa->util_avg = (sa->util_sum * scale_load_down(SCHED_LOAD_SCALE)) / LOAD_AVG_MAX; } return decayed; fix that issue in case SCHED_LOAD_RESOLUTION != 0 ? I would vote for removing this SCHED_LOAD_RESOLUTION thing completely so that we can assume that load/util and capacity are always using 1024/10. Cheers, -- Dietmar > > > Regards, > Vincent > > >> >> LOAD (UTIL) and CAPACITY have the same SCALE and SHIFT values because >> SCHED_LOAD_RESOLUTION is always defined to 0. scale_load() and >> scale_load_down() are also NOPs so this area is probably >> worth a separate clean-up. >> Beyond that, I'm not sure if the current functionality is >> broken if we use different SCALE and SHIFT values for LOAD and CAPACITY? >> [...] -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists