John Hubbard <jhubb...@nvidia.com> writes: > On 10/30/19 7:39 PM, Michael Ellerman wrote: >> Hi John, >> >> Sorry I didn't reply to this sooner, too many patches :/ >> >> John Hubbard <jhubb...@nvidia.com> writes: >>> The following build warning occurred on powerpc 64-bit builds: >>> >>> drivers/cpufreq/powernv-cpufreq.c: In function 'init_chip_info': >>> drivers/cpufreq/powernv-cpufreq.c:1070:1: warning: the frame size of 1040 >>> bytes is larger than 1024 bytes [-Wframe-larger-than=] >> >> Oddly I don't see that warning in my builds, eg with GCC9: >> >> https://travis-ci.org/linuxppc/linux/jobs/604870722 > > This is with a cross-compiler based on gcc 8.1.0, which I got from: > https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/8.1.0/ > > I'll put that in the v3 commit description. > >> >>> This is due to putting 1024 bytes on the stack: >>> >>> unsigned int chip[256]; >>> >>> ...and while looking at this, it also has a bug: it fails with a stack >>> overrun, if CONFIG_NR_CPUS > 256. >> >> It _probably_ doesn't, because it only increments the index when the >> chip_id of the CPU changes, ie. it doesn't create a chip for every CPU. >> But I agree it's flaky the way it's written. > > I'll soften up the wording accordingly. > >> >>> Fix both problems by dynamically allocating based on CONFIG_NR_CPUS. >> >> Shouldn't it use num_possible_cpus() ? >> >> Given the for loop is over possible CPUs that seems like the upper >> bound. In practice it should be lower because some CPUs will share a >> chip. >> > > OK, I see, that's more consistent with the code, I'll change to that.
Thanks. cheers
