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) + idx = perf_boost_idx; /* Performance Constraint (C) 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: - * 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) + 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 = + * nrg_delta * cap_gain > cap_delta * nrg_gain + * (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_gain + * (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; return payoff; }
-- 1.9.1
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)
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)
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 =* nrg_delta * cap_gain > cap_delta * nrg_gain* (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_gain* (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;
return payoff; }
-- 1.9.1
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 =
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_delta
which represents points in the "upper cut".
Thus:
* nrg_delta * cap_gain > cap_delta * nrg_gain
has 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_gain
In 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_gain
Which 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
-- #include <best/regards.h>
Patrick Bellasi
On Wed, May 25, 2016 at 06:53:28PM +0100, Patrick Bellasi wrote:
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.
Yes, please add my sign-off :-)
Just reminding, after review new patch, looks like in your code base there have trace event for payoff, but new patch has totally removed them from the function __schedtune_accept_deltas().
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)
Yes, previous code has ignored both cases for (nrg_delta = 0) and (cap_delta = 0). We just ignore the case for (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 =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)
Now it's much easier to understand these formulas :-)
For B region, we want to define for boosting performance how much energy we want to spend for it. Here I have a concern is: if with corrected formula, as result it will let small task to migrate from little core to big core more easier but add barrier to forbid big load task's migration to big core. The reason is small task will get smaller nrg_delta.
I have no question for formula itself, but suspect the sequential behavior for task placement, especially if we think it will give more chance to migrate small tasks to big core.
* (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;
Yes, correct.
return payoff; }
-- 1.9.1
-- #include <best/regards.h>
Patrick Bellasi
From 4395fb4877df83607341a61d0ab69a65bdf092eb Mon Sep 17 00:00:00 2001 From: Patrick Bellasi patrick.bellasi@arm.com Date: Wed, 25 May 2016 18:30:07 +0100 Subject: [PATCH] FIX: sched/tune: fix payoff calculation for boost region
The definition of the acceptance regions as well as the translation of these regions into a payoff value was both wrong which turned out in: a) a wrong definition of payoff for the performance boost region b) a correct "by chance" definition of the payoff for the performance constraint region (i.e. two sign errors together fixing the formula)
This patch provides a better description of the cut regions as well as a fixed version of the payoff computations, which are now reduced to a single formula usable for both cases.
Reported-by: Leo Yan leo.yan@linaro.org Suggested-by: Leo Yan leo.yan@linaro.org Signed-off-by: Patrick Bellasi patrick.bellasi@arm.com
kernel/sched/tune.c | 77 +++++++++++++++++++++++++++-------------------------- 1 file changed, 39 insertions(+), 38 deletions(-)
diff --git a/kernel/sched/tune.c b/kernel/sched/tune.c index da63e61..bf268fc 100644 --- a/kernel/sched/tune.c +++ b/kernel/sched/tune.c @@ -51,50 +51,51 @@ __schedtune_accept_deltas(int nrg_delta, int cap_delta, int perf_boost_idx, int perf_constrain_idx) { int payoff = -INT_MAX;
int gain_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;trace_sched_tune_filter(nrg_delta, cap_delta,threshold_gains[perf_boost_idx].nrg_gain,threshold_gains[perf_boost_idx].cap_gain,payoff, 8);return payoff;- }
- if (nrg_delta >= 0 && cap_delta > 0)
gain_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;trace_sched_tune_filter(nrg_delta, cap_delta,threshold_gains[perf_constrain_idx].nrg_gain,threshold_gains[perf_constrain_idx].cap_gain,payoff, 6);
else if (nrg_delta < 0 && cap_delta <= 0)
gain_idx = perf_constrain_idx;/* Default: reject schedule candidate */
if (gain_idx == -1) return payoff;
}
/* Default: reject schedule candidate */
- /*
* Evaluate "Performance Boost" vs "Energy Increase"** - Performance Boost (B) region** Condition: nrg_delta > 0 && cap_delta > 0* Payoff criteria:* cap_gain / nrg_gain < cap_delta / nrg_delta =* cap_gain * nrg_delta < cap_delta * nrg_gain* Note that since both nrg_gain and nrg_delta are positive, the* inequality does not change. Thus:** payoff = (cap_delta * nrg_gain) - (cap_gain * nrg_delta)** - Performance Constraint (C) region** Condition: nrg_delta < 0 && cap_delta < 0* payoff criteria:* cap_gain / nrg_gain > cap_delta / nrg_delta =* cap_gain * nrg_delta < cap_delta * nrg_gain* Note that since nrg_gain > 0 while nrg_delta < 0, the* inequality change. Thus:** payoff = (cap_delta * nrg_gain) - (cap_gain * nrg_delta)** This means that, in case of same positive defined {cap,nrg}_gain* for both the B and C regions, we can use the same payoff formula* where a positive value represents the accept condition.*/- payoff = cap_delta * threshold_gains[gain_idx].nrg_gain;
- payoff -= nrg_delta * threshold_gains[gain_idx].cap_gain;
- return payoff;
}
-- 2.5.0
On 26-May 10:06, Leo Yan wrote:
On Wed, May 25, 2016 at 06:53:28PM +0100, Patrick Bellasi wrote:
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.
Yes, please add my sign-off :-)
Will do.
Just reminding, after review new patch, looks like in your code base there have trace event for payoff, but new patch has totally removed them from the function __schedtune_accept_deltas().
Tracepoints are added by following patches...
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)Yes, previous code has ignored both cases for (nrg_delta = 0) and (cap_delta = 0). We just ignore the case for (cap_delta = 0).
The two cases ignored right now are the border of the suboptimal region... that's why it should be safe to ignore them.
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 =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)Now it's much easier to understand these formulas :-)
For B region, we want to define for boosting performance how much energy we want to spend for it. Here I have a concern is: if with corrected formula, as result it will let small task to migrate from little core to big core more easier but add barrier to forbid big load task's migration to big core. The reason is small task will get smaller nrg_delta.
Well, we should consider that small tasks can end up in a big core only if they are "boosted" as much as to not fit anymore in a LITTLE core. In this case it is still worth to verify if they can be allocated in a big one provided that the performance/energy ratio is acceptable. To the contrary, a relatively big task which can fit on a LITTLE is impacting much more energy if scheduled on a big... thus again it's worth to evaluate the performance/energy trade-off. If we are below the B cut this means that we can provide the required performances with a better energy efficiency by running the task on a LITTLE.
I have no question for formula itself, but suspect the sequential behavior for task placement, especially if we think it will give more chance to migrate small tasks to big core.
A set of rt-app synthetics to describe and evaluate this scenario would be really useful. ;-)
* (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;Yes, correct.
In attachment an updated version of the patch with your sign-off.
-- #include <best/regards.h>
Patrick Bellasi
-
Leo Yan -
Patrick Bellasi