Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 23 Jan 2020 11:02:51 +0530
From:   Viresh Kumar <viresh.kumar@linaro.org>
To:     "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc:     Linux PM <linux-pm@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        kbuild test robot <lkp@intel.com>
Subject: Re: [PATCH] cpufreq: Avoid creating excessively large stack frames
Message-ID: <20200123053251.3j7muofewhfidplw@vireshk-i7>
References: <2523300.pfpGjzY74h@kreacher>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <2523300.pfpGjzY74h@kreacher>
User-Agent: NeoMutt/20180716-391-311a52
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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