Messages in this thread | | | From | Geert Uytterhoeven <> | Date | Tue, 21 Jan 2020 08:51:49 +0100 | Subject | Re: drivers/acpi/processor_thermal.c:66:1: warning: the frame size of 2160 bytes is larger than 2048 bytes |
| |
Hi Rafael,
On Tue, Jan 21, 2020 at 1:40 AM Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > On Monday, January 20, 2020 11:16:12 AM CET Rafael J. Wysocki wrote: > > On Fri, Jan 17, 2020 at 4:51 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > On Fri, Nov 15, 2019 at 6:23 AM kbuild test robot <lkp@intel.com> wrote: > > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master > > > > head: 96b95eff4a591dbac582c2590d067e356a18aacb > > > > commit: 3000ce3c52f8b8db093e4dc649cd172390f71137 cpufreq: Use per-policy frequency QoS > > > > date: 4 weeks ago > > > > config: ia64-randconfig-a001-20191115 (attached as .config) > > > > compiler: ia64-linux-gcc (GCC) 7.4.0 > > > > reproduce: > > > > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > > > > chmod +x ~/bin/make.cross > > > > git checkout 3000ce3c52f8b8db093e4dc649cd172390f71137 > > > > # save the attached .config to linux build tree > > > > GCC_VERSION=7.4.0 make.cross ARCH=ia64 > > > > > > > > If you fix the issue, kindly add following tag > > > > Reported-by: kbuild test robot <lkp@intel.com> > > > > > > Seeing similar warnings on arm64, so this triggered my attention. > > > > > > > -- > > > > drivers/cpufreq/cpufreq.c: In function 'refresh_frequency_limits.part.33': > > > > >> drivers/cpufreq/cpufreq.c:1116:1: warning: the frame size of 2160 bytes is larger than 2048 bytes [-Wframe-larger-than=] > > > > > > | struct cpufreq_policy new_policy; > > > > > > That's a large struct on the stack... > > > > > > | if (!policy_is_inactive(policy)) { > > > | new_policy = *policy; > > > > > > Let's make a copy? > > > How well does this work, given struct cpufreq_policy contains a > > > work_struct, list_head, kobject, completion, semaphore, spinlock_t, > > > wait_queue_head_t, and two notifier_blocks, which are all objects you > > > cannot just copy and reuse? > > > > > > | pr_debug("updating policy for CPU %u\n", policy->cpu); > > > | > > > | cpufreq_set_policy(policy, &new_policy); > > > > > > If cpufreq_set_policy() uses only a few fields from new_policy, > > > > That's really the case. > > > > > it might be a good idea to extract those into its own structure. > > > > Or organize the code differently. > > > > This is old code that hasn't been change, but I'll look at it since it > > is problematic. > > So what about the patch below (untested)? > > It should be mostly self-explanatory, and the point is basically that > cpufreq_set_policy() gets the limits values from freq QoS, so it only > needs to get the new governor/policy value from the caller and more > data need to be passed to the driver's ->verify() callback.
Thanks, LGTM! No Rb, as I'm not that familiar with the code, and only browsed through the core parts.
I can confirm it gets rids of the -Wframe-larger-than warnings on arm64, though.
Gr{oetje,eeting}s,
Geert
-- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
| |