[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAMuHMdVDmdhEQKkkw9Ap5nSwMNH-jGb1TcMRgoQjPk44RwdY5w@mail.gmail.com>
Date: Tue, 21 Jan 2020 08:51:49 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc: Viresh Kumar <viresh.kumar@...aro.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Linux PM list <linux-pm@...r.kernel.org>
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@...ysocki.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@...ux-m68k.org> wrote:
> > > On Fri, Nov 15, 2019 at 6:23 AM kbuild test robot <lkp@...el.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@...el.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@...ux-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
Powered by blists - more mailing lists