Messages in this thread | | | Date | Thu, 23 Jan 2020 11:02:51 +0530 | From | Viresh Kumar <> | Subject | Re: [PATCH] cpufreq: Avoid creating excessively large stack frames |
| |
On 23-01-20, 00:16, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > In the process of modifying a cpufreq policy, the cpufreq core makes > a copy of it including all of the internals which is stored on the > CPU stack. Because struct cpufreq_policy is relatively large, this > may cause the size of the stack frame to exceed the 2 KB limit and > so the GCC complains when -Wframe-larger-than= is used. > > In fact, it is not necessary to copy the entire policy structure > in order to modify it, however. > > First, because cpufreq_set_policy() obtains the min and max policy > limits from frequency QoS now, it is not necessary to pass the limits > to it from the callers. The only things that need to be passed to it > from there are the new governor pointer or (if there is a built-in > governor in the driver) the "policy" value representing the governor > choice. They both can be passed as individual arguments, though, so > make cpufreq_set_policy() take them this way and rework its callers > accordingly. This avoids making copies of cpufreq policies in the > callers of cpufreq_set_policy(). > > Second, cpufreq_set_policy() still needs to pass the new policy > data to the ->verify() callback of the cpufreq driver whose task > is to sanitize the min and max policy limits. It still does not > need to make a full copy of struct cpufreq_policy for this purpose, > but it needs to pass a few items from it to the driver in case they > are needed (different drivers have different needs in that respect > and all of them have to be covered). For this reason, introduce > struct cpufreq_policy_data to hold copies of the members of > struct cpufreq_policy used by the existing ->verify() driver > callbacks and pass a pointer to a temporary structure of that > type to ->verify() (instead of passing a pointer to full struct > cpufreq_policy to it). > > While at it, notice that intel_pstate doesn't really need to verify > the "policy" value in struct cpufreq_policy, so drop that check from > it to avoid copying "policy" into struct cpufreq_policy_data (which > allows it to be slightly smaller). > > Also while at it fix up white space in a couple of places and make > cpufreq_set_policy() static (as it can be so). > > Fixes: 3000ce3c52f8 ("cpufreq: Use per-policy frequency QoS") > Link: https://lore.kernel.org/linux-pm/CAMuHMdX6-jb1W8uC2_237m8ctCpsnGp=JCxqt8pCWVqNXHmkVg@mail.gmail.com > Reported-by: kbuild test robot <lkp@intel.com> > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> > Cc: 5.4+ <stable@vger.kernel.org> # 5.4+ > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/cpufreq/cppc_cpufreq.c | 2 > drivers/cpufreq/cpufreq-nforce2.c | 2 > drivers/cpufreq/cpufreq.c | 147 +++++++++++++++++-------------------- > drivers/cpufreq/freq_table.c | 4 - > drivers/cpufreq/gx-suspmod.c | 2 > drivers/cpufreq/intel_pstate.c | 38 ++++----- > drivers/cpufreq/longrun.c | 2 > drivers/cpufreq/pcc-cpufreq.c | 2 > drivers/cpufreq/sh-cpufreq.c | 2 > drivers/cpufreq/unicore2-cpufreq.c | 2 > include/linux/cpufreq.h | 32 +++++--- > 11 files changed, 119 insertions(+), 116 deletions(-)
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
-- viresh
| |