[PATCH v3 1/1] mtd: cfi_cmdset_0001: Factor out do_write_buffer_locked() to reduce stack frame

[Andy Shevchenko]

On Mon, Apr 27, 2026 at 05:38:31PM +0200, Miquel Raynal wrote:
> On 14/04/2026 at 15:38:38 +03, Andy Shevchenko <andriy.shevchenko at linux.intel.com> wrote:
> > On Thu, Apr 09, 2026 at 12:28:46PM +0100, David Laight wrote:
> >> On Thu, 9 Apr 2026 09:58:28 +0200
> >> Lukas Wunner <lukas at wunner.de> wrote:
> >> > On Thu, Apr 09, 2026 at 08:26:11AM +0100, David Laight wrote:
> >> > > On Wed,  8 Apr 2026 23:11:48 +0200 Andy Shevchenko <andriy.shevchenko at linux.intel.com> wrote:  
> >> > > > Compiler is not happy about used stack frame:
> >> > > > 
> >> > > > drivers/mtd/chips/cfi_cmdset_0001.c: In function 'do_write_buffer':
> >> > > > drivers/mtd/chips/cfi_cmdset_0001.c:1887:1: error: the frame size of 1296 bytes is larger than 1280 bytes [-Werror=frame-larger-than=]
> >> > > > 
> >> > > > Fix this by factoring out do_write_buffer_locked().  
> >> > > 
> >> > > Does this just split the large stack frame between two nested functions?
> >> > > I'd also expect the compiler to inline do_write_buffer_locked() so it
> >> > > makes little difference.
> >> > > OTOH I can't immediately see where the large stack frame comes from.  
> >> > 
> >> > The error occurs for an allmodconfig build on arm, which implies
> >> > CONFIG_KASAN_STACK=y and thus increases stack usage vis-à-vis a
> >> > "regular" build.
> >> > 
> >> > Stack usage is high here because of the three "map_word" types,
> >> > which can each be up to 256 unsigned longs (32 * 8), see the
> >> > definitions of MAX_MAP_LONGS, MAX_MAP_BANKWIDTH, map_word in
> >> > include/linux/mtd/map.h.
> >> 
> >> Ugg - that code is horrid.
> >> Returning structures by value isn't really a good idea.
> 
> Looks like the primary reason for the stack over usage, no? Isn't
> playing with inline and refactoring just a tiny fix that prevents
> problem by just a couple of bytes?
> 
> I haven't looked too carefully, but could we (Andy?) have a fix that
> reduces the number of map_word (as suggested, IIUC) and/or avoid passing
> them by value?

I am not an expert for this particular change, I am afraid. Consider my patch
as a bug report that needs to be addressed, Currently it might break some
builds when `make W=1` is passed.

> I can also take this cleanup if enclosed in a bigger
> series, I don't mind because it may make the code easier to read as
> well, but I feel like this is not a proper fix. If it is, please explain
> to me again :-)

-- 
With Best Regards,
Andy Shevchenko