Re: [Eas-dev] [PATCH] sched/tune: fix payoff calculation for performance constraint region
On 2016年05月26日 17:44, eas-dev-request@lists.linaro.org wrote:
Send eas-dev mailing list submissions to eas-dev@lists.linaro.org
To subscribe or unsubscribe via the World Wide Web, visit https://lists.linaro.org/mailman/listinfo/eas-dev or, via email, send a message with subject or body 'help' to eas-dev-request@lists.linaro.org
You can reach the person managing the list at eas-dev-owner@lists.linaro.org
When replying, please edit your Subject line so it is more specific than "Re: Contents of eas-dev digest..."
Today's Topics:
- Re: [PATCH] sched/tune: fix payoff calculation for performance constraint region (Patrick Bellasi)
- Re: [PATCH] sched/tune: fix payoff calculation for performance constraint region (Leo Yan)
- Re: [PATCH] sched/tune: fix payoff calculation for performance constraint region (Patrick Bellasi)
Message: 1 Date: Wed, 25 May 2016 18:53:28 +0100 From: Patrick Bellasi patrick.bellasi@arm.com To: Leo Yan leo.yan@linaro.org Cc: eas-dev@lists.linaro.org Subject: Re: [Eas-dev] [PATCH] sched/tune: fix payoff calculation for performance constraint region Message-ID: 20160525175328.GA15730@e105326-lin Content-Type: text/plain; charset="utf-8"
Hi Leo, thanks a lot for reviewing SchedTune and pointing out this issue.
Actually going through the code I've noticed another big issue related to the definition of the acceptable regions, following commented inline. Basically, with the current implementation we was getting a correct "by chance" C region, while the acceptance for the B region was completely wrong.
In attachment a new version of the patch, please have a look and let me know if you have any doubt and/or suggestions. If the patch is ok for you, lemme also know if it's ok for you to add your sign-off.
Cheers Patrick
On 23-May 21:47, Leo Yan wrote:
On Fri, May 20, 2016 at 12:24:49AM +0800, Leo Yan wrote:
When calculate payoff criteria for performance constraint region, the inequality formula is wrong:
cap_delta / nrg_delta > cap_gain / nrg_gain
Here nrg_delta < 0, so when multiply it both side then should then multiplying nrg_delta inverts the inequality:
nrg_delta * cap_gain > cap_delta * nrg_gain
So finally we can get unified formula for both performance constraint region and performance boost region. So this patch unified these the calculation after fixed inequality formula.
Signed-off-by: Leo Yan leo.yan@linaro.org
kernel/sched/tune.c | 54 ++++++++++++++++++++++++++++------------------------- 1 file changed, 29 insertions(+), 25 deletions(-)
diff --git a/kernel/sched/tune.c b/kernel/sched/tune.c index 9d8eeb4..1da85a8 100644 --- a/kernel/sched/tune.c +++ b/kernel/sched/tune.c @@ -58,36 +58,40 @@ __schedtune_accept_deltas(int nrg_delta, int cap_delta, int perf_boost_idx, int perf_constrain_idx) { int payoff = -INT_MAX;
int idx = -1;
/* Performance Boost (B) region */
- if (nrg_delta > 0 && cap_delta > 0) {
/** Evaluate "Performance Boost" vs "Energy Increase"* payoff criteria:* cap_delta / nrg_delta < cap_gain / nrg_gain* which is:* nrg_delta * cap_gain > cap_delta * nrg_gain*/payoff = nrg_delta * threshold_gains[perf_boost_idx].cap_gain;payoff -= cap_delta * threshold_gains[perf_boost_idx].nrg_gain;return payoff;- }
- if (nrg_delta > 0 && cap_delta > 0)
Looking better at the called, I think it's worth con accept also points in the P axis, thus:
if (nrg_delta >= 0 && cap_delta > 0)
idx = perf_boost_idx;
- if (nrg_delta < 0 && cap_delta < 0) {
/** Evaluate "Performance Boost" vs "Energy Increase"* payoff criteria:* cap_delta / nrg_delta > cap_gain / nrg_gain* which is:* cap_delta * nrg_gain > nrg_delta * cap_gain*/payoff = cap_delta * threshold_gains[perf_constrain_idx].nrg_gain;payoff -= nrg_delta * threshold_gains[perf_constrain_idx].cap_gain;return payoff;- }
- else if (nrg_delta < 0 && cap_delta < 0)
For the same considerations we should better accept points in the E axis, thus:
else if (nrg_delta < 0 && cap_delta <= 0)
idx = perf_constrain_idx;/* Default: reject schedule candidate */
if (idx == -1)
return payoff;/*
- Evaluate "Performance Boost" vs "Energy Increase"
- Performance Boost (B) region
- Condition: nrg_delta > 0 && cap_delta > 0
- Payoff criteria:
cap_delta / nrg_delta < cap_gain / nrg_gain =
Looking better to put the condition "nrg_delta == 0" or "cap_delta == 0" in function "accept_deltas", to avoid fetch rcu lock and more functions called, thus:
/* Optimal (O) region */ if ((nrg_delta < 0 && cap_delta >= 0) || (nrg_delta <=0 && cap_delta > 0)) { trace_sched_tune_filter(nrg_delta, cap_delta, 0, 0, 1, 0); return INT_MAX; }
Here the inequality has a wrong direction. The schedule candidate acceptable in the B region are those for which:
cap_gain / nrg_gain < cap_delta / nrg_deltawhich represents points in the "upper cut".
Thus:
nrg_delta * cap_gain > cap_delta * nrg_gainhas to be: cap_gain * nrg_delta < cap_delta * nrg_gain
Which results into a "positive accept" payoff defined as:
payoff = (cap_delta * nrg_gain) - (cap_gain * nrg_delta)
(note: nrg_delta > 0, nrg_gain > 0)
- Performance Constraint (C) region
- Condition: nrg_delta < 0 && cap_delta < 0
- payoff criteria:
cap_delta / nrg_delta > cap_gain / nrg_gain =
nrg_delta * cap_gain > cap_delta * nrg_gainIn the C region we have both a wrong definition and the sign error you reported, which turned out to provide a "by change" correct implementation, which is:
cap_gain / nrg_gain > cap_delta / nrg_delta = cap_gain * nrg_delta < cap_delta * nrg_gainWhich results into a "positive accept" payoff defined as:
payoff = (cap_delta * nrg_gain) - (cap_gain * nrg_delta)The same as for the B region...
(note: nrg_delta < 0, nrg_gain > 0)- */
- payoff = nrg_delta * threshold_gains[perf_boost_idx].cap_gain;
- payoff -= cap_delta * threshold_gains[perf_boost_idx].nrg_gain;
Sorry, here should be:
- payoff = nrg_delta * threshold_gains[idx].cap_gain;
- payoff -= cap_delta * threshold_gains[idx].nrg_gain;
... which means that this two operations have to be inverted:
payoff = cap_delta * threshold_gains[gain_idx].nrg_gain; payoff -= nrg_delta * threshold_gains[gain_idx].cap_gain;return payoff;}
-- 1.9.1
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
-
toby