| Message ID | 20250219-fix-power-allocator-calc-v1-1-48b860291919@chromium.org (mailing list archive) |
|---|---|
| State | Queued |
| Delegated to: | Rafael Wysocki |
| Headers | show |
| Series | thermal: gov_power_allocator: Fix incorrect calculation in divvy_up_power | expand |
On Wed, Feb 19, 2025 at 8:07 AM Yu-Che Cheng <giver@chromium.org> wrote: > > divvy_up_power should use weighted_req_power instead of req_power to > calculate the granted_power. Otherwise, the granted_power may be > unexpected as the denominator total_req_power is weighted sum. Yes, this is what's happening, to my eyes. > This is a mistake during the previous refactor. > > Replace the req_power with weighted_req_power in divvy_up_power > calculation. > > Fixes: 912e97c67cc3 ("thermal: gov_power_allocator: Move memory allocation out of throttle()") > Signed-off-by: Yu-Che Cheng <giver@chromium.org> > --- > drivers/thermal/gov_power_allocator.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c > index 3b644de3292e..3b626db55b2b 100644 > --- a/drivers/thermal/gov_power_allocator.c > +++ b/drivers/thermal/gov_power_allocator.c > @@ -370,7 +370,7 @@ static void divvy_up_power(struct power_actor *power, int num_actors, > > for (i = 0; i < num_actors; i++) { > struct power_actor *pa = &power[i]; > - u64 req_range = (u64)pa->req_power * power_range; > + u64 req_range = (u64)pa->weighted_req_power * power_range; > > pa->granted_power = DIV_ROUND_CLOSEST_ULL(req_range, > total_req_power); > > --- And the fix looks good to me. Lukasz, any concerns?
On 2/20/25 19:45, Rafael J. Wysocki wrote: > On Wed, Feb 19, 2025 at 8:07 AM Yu-Che Cheng <giver@chromium.org> wrote: >> >> divvy_up_power should use weighted_req_power instead of req_power to >> calculate the granted_power. Otherwise, the granted_power may be >> unexpected as the denominator total_req_power is weighted sum. > > Yes, this is what's happening, to my eyes. > >> This is a mistake during the previous refactor. >> >> Replace the req_power with weighted_req_power in divvy_up_power >> calculation. >> >> Fixes: 912e97c67cc3 ("thermal: gov_power_allocator: Move memory allocation out of throttle()") >> Signed-off-by: Yu-Che Cheng <giver@chromium.org> >> --- >> drivers/thermal/gov_power_allocator.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c >> index 3b644de3292e..3b626db55b2b 100644 >> --- a/drivers/thermal/gov_power_allocator.c >> +++ b/drivers/thermal/gov_power_allocator.c >> @@ -370,7 +370,7 @@ static void divvy_up_power(struct power_actor *power, int num_actors, >> >> for (i = 0; i < num_actors; i++) { >> struct power_actor *pa = &power[i]; >> - u64 req_range = (u64)pa->req_power * power_range; >> + u64 req_range = (u64)pa->weighted_req_power * power_range; >> >> pa->granted_power = DIV_ROUND_CLOSEST_ULL(req_range, >> total_req_power); >> >> --- > > And the fix looks good to me. > > Lukasz, any concerns? Good catch! It went through since the test didn't set different weights. Thanks for the fix! Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
On Fri, Feb 21, 2025 at 11:35 AM Lukasz Luba <lukasz.luba@arm.com> wrote: > > > > On 2/20/25 19:45, Rafael J. Wysocki wrote: > > On Wed, Feb 19, 2025 at 8:07 AM Yu-Che Cheng <giver@chromium.org> wrote: > >> > >> divvy_up_power should use weighted_req_power instead of req_power to > >> calculate the granted_power. Otherwise, the granted_power may be > >> unexpected as the denominator total_req_power is weighted sum. > > > > Yes, this is what's happening, to my eyes. > > > >> This is a mistake during the previous refactor. > >> > >> Replace the req_power with weighted_req_power in divvy_up_power > >> calculation. > >> > >> Fixes: 912e97c67cc3 ("thermal: gov_power_allocator: Move memory allocation out of throttle()") > >> Signed-off-by: Yu-Che Cheng <giver@chromium.org> > >> --- > >> drivers/thermal/gov_power_allocator.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c > >> index 3b644de3292e..3b626db55b2b 100644 > >> --- a/drivers/thermal/gov_power_allocator.c > >> +++ b/drivers/thermal/gov_power_allocator.c > >> @@ -370,7 +370,7 @@ static void divvy_up_power(struct power_actor *power, int num_actors, > >> > >> for (i = 0; i < num_actors; i++) { > >> struct power_actor *pa = &power[i]; > >> - u64 req_range = (u64)pa->req_power * power_range; > >> + u64 req_range = (u64)pa->weighted_req_power * power_range; > >> > >> pa->granted_power = DIV_ROUND_CLOSEST_ULL(req_range, > >> total_req_power); > >> > >> --- > > > > And the fix looks good to me. > > > > Lukasz, any concerns? > > Good catch! It went through since the test didn't set different weights. > > Thanks for the fix! > > Reviewed-by: Lukasz Luba <lukasz.luba@arm.com> Applied as 6.14-rc material, thanks!
diff --git a/drivers/thermal/gov_power_allocator.c b/drivers/thermal/gov_power_allocator.c index 3b644de3292e..3b626db55b2b 100644 --- a/drivers/thermal/gov_power_allocator.c +++ b/drivers/thermal/gov_power_allocator.c @@ -370,7 +370,7 @@ static void divvy_up_power(struct power_actor *power, int num_actors, for (i = 0; i < num_actors; i++) { struct power_actor *pa = &power[i]; - u64 req_range = (u64)pa->req_power * power_range; + u64 req_range = (u64)pa->weighted_req_power * power_range; pa->granted_power = DIV_ROUND_CLOSEST_ULL(req_range, total_req_power);
divvy_up_power should use weighted_req_power instead of req_power to calculate the granted_power. Otherwise, the granted_power may be unexpected as the denominator total_req_power is weighted sum. This is a mistake during the previous refactor. Replace the req_power with weighted_req_power in divvy_up_power calculation. Fixes: 912e97c67cc3 ("thermal: gov_power_allocator: Move memory allocation out of throttle()") Signed-off-by: Yu-Che Cheng <giver@chromium.org> --- drivers/thermal/gov_power_allocator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- base-commit: 2408a807bfc3f738850ef5ad5e3fd59d66168996 change-id: 20250218-fix-power-allocator-calc-f00b4ab650ba Best regards,