Messages in this thread | | | Date | Thu, 15 Dec 2022 20:49:03 +0100 | Subject | Re: [PATCH] drm/amd/pm: avoid large variable on kernel stack | From | Marion & Christophe JAILLET <> |
| |
Le 15/12/2022 à 20:46, Christophe JAILLET a écrit : > Le 15/12/2022 à 17:36, Arnd Bergmann a écrit : >> From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> >> >> The activity_monitor_external[] array is too big to fit on the >> kernel stack, resulting in this warning with clang: >> >> drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu13/smu_v13_0_7_ppt.c:1438:12: error: stack frame size (1040) exceeds limit (1024) in 'smu_v13_0_7_get_power_profile_mode' [-Werror,-Wframe-larger-than] >> >> Use dynamic allocation instead. It should also be possible to >> have single element here instead of the array, but this seems >> easier. >> >> Fixes: 334682ae8151 ("drm/amd/pm: enable workload type change on >> smu_v13_0_7") >> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> >> --- >> .../drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 21 ++++++++++++++----- >> 1 file changed, 16 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c >> b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c >> index c270f94a1b86..7eba854e09ec 100644 >> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c >> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c >> @@ -1439,7 +1439,7 @@ static int smu_v13_0_7_get_power_limit(struct >> smu_context *smu, >> static int smu_v13_0_7_get_power_profile_mode(struct smu_context >> *smu, char *buf) >> { >> - DpmActivityMonitorCoeffIntExternal_t >> activity_monitor_external[PP_SMC_POWER_PROFILE_COUNT]; >> + DpmActivityMonitorCoeffIntExternal_t *activity_monitor_external; >> uint32_t i, j, size = 0; >> int16_t workload_type = 0; >> int result = 0; >> @@ -1447,6 +1447,12 @@ static int >> smu_v13_0_7_get_power_profile_mode(struct smu_context *smu, char *buf >> if (!buf) >> return -EINVAL; >> + activity_monitor_external = >> kcalloc(sizeof(activity_monitor_external), > > Hi, > > Before, 'activity_monitor_external' was not initialized. > Maybe kcalloc() is enough?
Read kmalloc_array()
> > sizeof(*activity_monitor_external)? > ~~~~ > >> + PP_SMC_POWER_PROFILE_COUNT, >> + GFP_KERNEL); >> + if (!activity_monitor_external) >> + return -ENOMEM; >> + >> size += sysfs_emit_at(buf, size, " "); >> for (i = 0; i <= PP_SMC_POWER_PROFILE_WINDOW3D; i++) > > Unrelated, but wouldn't it be more straightforward with "< > PP_SMC_POWER_PROFILE_COUNT"? > >> size += sysfs_emit_at(buf, size, "%-14s%s", >> amdgpu_pp_profile_name[i], >> @@ -1459,15 +1465,17 @@ static int >> smu_v13_0_7_get_power_profile_mode(struct smu_context *smu, char *buf >> workload_type = smu_cmn_to_asic_specific_index(smu, >> CMN2ASIC_MAPPING_WORKLOAD, >> i); >> - if (workload_type < 0) >> - return -EINVAL; >> + if (workload_type < 0) { >> + result = -EINVAL; >> + goto out; >> + } >> result = smu_cmn_update_table(smu, >> SMU_TABLE_ACTIVITY_MONITOR_COEFF, workload_type, >> (void *)(&activity_monitor_external[i]), false); >> if (result) { >> dev_err(smu->adev->dev, "[%s] Failed to get activity >> monitor!", __func__); >> - return result; >> + goto out; >> } >> } >> @@ -1495,7 +1503,10 @@ do >> { \ >> PRINT_DPM_MONITOR(Fclk_BoosterFreq); >> #undef PRINT_DPM_MONITOR >> - return size; >> + result = size; >> +out: >> + kfree(activity_monitor_external); >> + return result; >> } >> static int smu_v13_0_7_set_power_profile_mode(struct smu_context >> *smu, long *input, uint32_t size) > >
| |