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

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: David Laight <david.laight.linux@gmail.com>,
	Lukas Wunner <lukas@wunner.de>,
	linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH v3 1/1] mtd: cfi_cmdset_0001: Factor out do_write_buffer_locked() to reduce stack frame
Date: Mon, 27 Apr 2026 18:47:58 +0300	[thread overview]
Message-ID: <ae-FLk1N3wJs8sTW@ashevche-desk.local> (raw)
In-Reply-To: <87ik9cfm6g.fsf@bootlin.com>

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@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@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@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

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/