powercap/drivers/dtpm: Fix __udivdi3 and __aeabi_uldivmod unresolved symbols
diff mbox series

Message ID 20201230153744.15612-1-daniel.lezcano@linaro.org
State Accepted
Commit 8f50db4b5c79af2ba54f5fbe8a5173fd7f37a493
Headers show
Series
  • powercap/drivers/dtpm: Fix __udivdi3 and __aeabi_uldivmod unresolved symbols
Related show

Commit Message

Daniel Lezcano Dec. 30, 2020, 3:37 p.m. UTC
32 bits architectures do not support u64 division, so the macro
DIV_ROUND_CLOSEST is not adequate as the compiler will replace the
call to an unexisting function for the platform, leading to an
unresolved symbols.

Fix this by using the compatible macros:

DIV64_U64_ROUND_CLOSEST and DIV_ROUND_CLOSEST_ULL.

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
 drivers/powercap/dtpm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Rafael J. Wysocki Dec. 30, 2020, 6:09 p.m. UTC | #1
On Wed, Dec 30, 2020 at 4:38 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> 32 bits architectures do not support u64 division, so the macro
> DIV_ROUND_CLOSEST is not adequate as the compiler will replace the
> call to an unexisting function for the platform, leading to an
> unresolved symbols.
>
> Fix this by using the compatible macros:
>
> DIV64_U64_ROUND_CLOSEST and DIV_ROUND_CLOSEST_ULL.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Applied and pushed into the linux-next branch, thanks!

> ---
>  drivers/powercap/dtpm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/powercap/dtpm.c b/drivers/powercap/dtpm.c
> index 5b6857e9b064..0abcc439d728 100644
> --- a/drivers/powercap/dtpm.c
> +++ b/drivers/powercap/dtpm.c
> @@ -99,8 +99,8 @@ static void __dtpm_rebalance_weight(struct dtpm *dtpm)
>                 pr_debug("Setting weight '%d' for '%s'\n",
>                          child->weight, child->zone.name);
>
> -               child->weight = DIV_ROUND_CLOSEST(child->power_max * 1024,
> -                                                 dtpm->power_max);
> +               child->weight = DIV64_U64_ROUND_CLOSEST(
> +                       child->power_max * 1024, dtpm->power_max);
>
>                 __dtpm_rebalance_weight(child);
>         }
> @@ -272,7 +272,7 @@ static int __set_power_limit_uw(struct dtpm *dtpm, int cid, u64 power_limit)
>                         } else if (power_limit == dtpm->power_min) {
>                                 power = child->power_min;
>                         } else {
> -                               power = DIV_ROUND_CLOSEST(
> +                               power = DIV_ROUND_CLOSEST_ULL(
>                                         power_limit * child->weight, 1024);
>                         }
>
> --
> 2.17.1
>
Geert Uytterhoeven Jan. 4, 2021, 8:18 a.m. UTC | #2
Hi Daniel,

On Wed, Dec 30, 2020 at 4:39 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
> 32 bits architectures do not support u64 division, so the macro
> DIV_ROUND_CLOSEST is not adequate as the compiler will replace the
> call to an unexisting function for the platform, leading to an
> unresolved symbols.
>
> Fix this by using the compatible macros:
>
> DIV64_U64_ROUND_CLOSEST and DIV_ROUND_CLOSEST_ULL.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>

Thanks for your patch!

> --- a/drivers/powercap/dtpm.c
> +++ b/drivers/powercap/dtpm.c
> @@ -99,8 +99,8 @@ static void __dtpm_rebalance_weight(struct dtpm *dtpm)
>                 pr_debug("Setting weight '%d' for '%s'\n",
>                          child->weight, child->zone.name);
>
> -               child->weight = DIV_ROUND_CLOSEST(child->power_max * 1024,
> -                                                 dtpm->power_max);
> +               child->weight = DIV64_U64_ROUND_CLOSEST(
> +                       child->power_max * 1024, dtpm->power_max);

Note that 64-by-64 divisions are expensive on 32-bit platforms.

Does dtpm.power_max need to be u64?
The (lack of) documentation for the dtpm structure does not say what is
being stored there.

Gr{oetje,eeting}s,

                        Geert
Daniel Lezcano Jan. 4, 2021, 11:20 a.m. UTC | #3
Hi Geert,


On 04/01/2021 09:18, Geert Uytterhoeven wrote:
> Hi Daniel,
> 
> On Wed, Dec 30, 2020 at 4:39 PM Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
>> 32 bits architectures do not support u64 division, so the macro
>> DIV_ROUND_CLOSEST is not adequate as the compiler will replace the
>> call to an unexisting function for the platform, leading to an
>> unresolved symbols.
>>
>> Fix this by using the compatible macros:
>>
>> DIV64_U64_ROUND_CLOSEST and DIV_ROUND_CLOSEST_ULL.
>>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> 
> Thanks for your patch!
> 
>> --- a/drivers/powercap/dtpm.c
>> +++ b/drivers/powercap/dtpm.c
>> @@ -99,8 +99,8 @@ static void __dtpm_rebalance_weight(struct dtpm *dtpm)
>>                 pr_debug("Setting weight '%d' for '%s'\n",
>>                          child->weight, child->zone.name);
>>
>> -               child->weight = DIV_ROUND_CLOSEST(child->power_max * 1024,
>> -                                                 dtpm->power_max);
>> +               child->weight = DIV64_U64_ROUND_CLOSEST(
>> +                       child->power_max * 1024, dtpm->power_max);
> 
> Note that 64-by-64 divisions are expensive on 32-bit platforms.
> 
> Does dtpm.power_max need to be u64?

The dtpm is based on the powercap framework which deals with microwatts
and the functions are expecting u64 values.

The division here happens when there is an update of the dtpm tree which
occurs rarely (at boot time or hotplug).

As the power model is in the vast majority on 64b platforms, the effort
to optimize to u32 sounds not worth, especially that the 32b platforms
supporting the energy model are now obsolete.

> The (lack of) documentation for the dtpm structure does not say what is
> being stored there.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
>

Patch
diff mbox series

diff --git a/drivers/powercap/dtpm.c b/drivers/powercap/dtpm.c
index 5b6857e9b064..0abcc439d728 100644
--- a/drivers/powercap/dtpm.c
+++ b/drivers/powercap/dtpm.c
@@ -99,8 +99,8 @@  static void __dtpm_rebalance_weight(struct dtpm *dtpm)
 		pr_debug("Setting weight '%d' for '%s'\n",
 			 child->weight, child->zone.name);
 
-		child->weight = DIV_ROUND_CLOSEST(child->power_max * 1024,
-						  dtpm->power_max);
+		child->weight = DIV64_U64_ROUND_CLOSEST(
+			child->power_max * 1024, dtpm->power_max);
 
 		__dtpm_rebalance_weight(child);
 	}
@@ -272,7 +272,7 @@  static int __set_power_limit_uw(struct dtpm *dtpm, int cid, u64 power_limit)
 			} else if (power_limit == dtpm->power_min) {
 				power = child->power_min;
 			} else {
-				power = DIV_ROUND_CLOSEST(
+				power = DIV_ROUND_CLOSEST_ULL(
 					power_limit * child->weight, 1024);
 			}