[PATCH] mtd: cfi_cmdset_0001: shrink do_write_buffer() stack frame
David Laight
david.laight.linux at gmail.com
Mon Aug 24 00:56:32 PDT 2026
On Mon, 24 Aug 2026 03:47:03 +0200
Karl Mehltretter <kmehltretter at gmail.com> wrote:
> arm32 allmodconfig fails to build with gcc:
>
> drivers/mtd/chips/cfi_cmdset_0001.c:1883:1: error: the frame size of 1296
> bytes is larger than 1280 bytes [-Werror=frame-larger-than=]
>
> With MTD_MAP_BANK_WIDTH_32 a map_word is 32 bytes, and with KASAN_STACK
> every temporary gets its own redzoned slot. do_write_buffer() builds a
> map_word for each of the twelve commands it issues, and those temporaries
> dominate its frame.
Looks to me like pretty much all of mtd/map.h should be real functions
and not #defines or inlines.
The bloat from all those expansions is going to be significant.
They only time they get optimised to anything small is when only
CONFIG_MTD_MAP_BANK_WIDTH_1 is set.
The execution time of all those that do real io (especially reads)
will be dominated by the io access itself.
David
>
> Issue the commands through a small noinline helper that takes the
> command as a plain value and builds the map_word in its own frame. The
> sequence of reads and writes to the chip is unchanged.
>
> The helper must not be inlined, and reusing one map_word local is not
> enough: clang cannot assume the callee does not observe the local, so it
> still returns each CMD() into a per-call-site temporary, and the frame
> grows (1760 -> 1824 bytes in cfi_intelext_writev(), into which clang
> inlines do_write_buffer()).
>
> Frame sizes with MTD_MAP_BANK_WIDTH_32 and KASAN_STACK:
>
> gcc 15.2 do_write_buffer() 1296 -> 520
> clang 21 cfi_intelext_writev() 1760 -> 896
>
> The helper itself takes 168 (gcc) / 160 (clang) bytes.
>
> Reported-by: Andy Shevchenko <andriy.shevchenko at linux.intel.com>
> Closes: https://lore.kernel.org/all/20260408211407.2295175-1-andriy.shevchenko@linux.intel.com/
> Suggested-by: Miquel Raynal <miquel.raynal at bootlin.com>
> Link: https://lore.kernel.org/all/87ik9cfm6g.fsf@bootlin.com/
> Assisted-by: LLM
> Signed-off-by: Karl Mehltretter <kmehltretter at gmail.com>
> ---
> drivers/mtd/chips/cfi_cmdset_0001.c | 42 ++++++++++++++++++-----------
> 1 file changed, 26 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c b/drivers/mtd/chips/cfi_cmdset_0001.c
> index b73596a8e021..a7c77c4e76ae 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0001.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0001.c
> @@ -1716,12 +1716,24 @@ static int cfi_intelext_write_words (struct mtd_info *mtd, loff_t to , size_t le
> }
>
>
> +/*
> + * Keep noinline: inlined, the map_word temporaries put do_write_buffer() over
> + * the frame-size limit with MTD_MAP_BANK_WIDTH_32 and KASAN_STACK.
> + */
> +static noinline void __xipram cfi_write_cmd(struct map_info *map,
> + unsigned long cmd, unsigned long adr)
> +{
> + struct cfi_private *cfi = map->fldrv_priv;
> +
> + map_write(map, CMD(cmd), adr);
> +}
> +
> static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
> unsigned long adr, const struct kvec **pvec,
> unsigned long *pvec_seek, int len)
> {
> struct cfi_private *cfi = map->fldrv_priv;
> - map_word status, write_cmd, datum;
> + map_word status, datum;
> unsigned long cmd_adr;
> int ret, wbufsize, word_gap, words;
> const struct kvec *vec;
> @@ -1740,9 +1752,6 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
> if (is_LH28F640BF(cfi))
> cmd_adr = adr;
>
> - /* Let's determine this according to the interleave only once */
> - write_cmd = (cfi->cfiq->P_ID != P_ID_INTEL_PERFORMANCE) ? CMD(0xe8) : CMD(0xe9);
> -
> mutex_lock(&chip->mutex);
> ret = get_chip(map, chip, cmd_adr, FL_WRITING);
> if (ret) {
> @@ -1759,7 +1768,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
> So we must check here and reset those bits if they're set. Otherwise
> we're just pissing in the wind */
> if (chip->state != FL_STATUS) {
> - map_write(map, CMD(0x70), cmd_adr);
> + cfi_write_cmd(map, 0x70, cmd_adr);
> chip->state = FL_STATUS;
> }
> status = map_read(map, cmd_adr);
> @@ -1767,21 +1776,22 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
> xip_enable(map, chip, cmd_adr);
> printk(KERN_WARNING "SR.4 or SR.5 bits set in buffer write (status %lx). Clearing.\n", status.x[0]);
> xip_disable(map, chip, cmd_adr);
> - map_write(map, CMD(0x50), cmd_adr);
> - map_write(map, CMD(0x70), cmd_adr);
> + cfi_write_cmd(map, 0x50, cmd_adr);
> + cfi_write_cmd(map, 0x70, cmd_adr);
> }
>
> chip->state = FL_WRITING_TO_BUFFER;
> - map_write(map, write_cmd, cmd_adr);
> + cfi_write_cmd(map, (cfi->cfiq->P_ID != P_ID_INTEL_PERFORMANCE) ? 0xe8 : 0xe9,
> + cmd_adr);
> ret = WAIT_TIMEOUT(map, chip, cmd_adr, 0, 0);
> if (ret) {
> /* Argh. Not ready for write to buffer */
> map_word Xstatus = map_read(map, cmd_adr);
> - map_write(map, CMD(0x70), cmd_adr);
> + cfi_write_cmd(map, 0x70, cmd_adr);
> chip->state = FL_STATUS;
> status = map_read(map, cmd_adr);
> - map_write(map, CMD(0x50), cmd_adr);
> - map_write(map, CMD(0x70), cmd_adr);
> + cfi_write_cmd(map, 0x50, cmd_adr);
> + cfi_write_cmd(map, 0x70, cmd_adr);
> xip_enable(map, chip, cmd_adr);
> printk(KERN_ERR "%s: Chip not ready for buffer write. Xstatus = %lx, status = %lx\n",
> map->name, Xstatus.x[0], status.x[0]);
> @@ -1800,7 +1810,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
> }
>
> /* Write length of data to come */
> - map_write(map, CMD(words), cmd_adr );
> + cfi_write_cmd(map, words, cmd_adr);
>
> /* Write data */
> vec = *pvec;
> @@ -1837,7 +1847,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
> *pvec_seek = vec_seek;
>
> /* GO GO GO */
> - map_write(map, CMD(0xd0), cmd_adr);
> + cfi_write_cmd(map, 0xd0, cmd_adr);
> chip->state = FL_WRITING;
>
> ret = INVAL_CACHE_AND_WAIT(map, chip, cmd_adr,
> @@ -1845,7 +1855,7 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
> chip->buffer_write_time,
> chip->buffer_write_time_max);
> if (ret) {
> - map_write(map, CMD(0x70), cmd_adr);
> + cfi_write_cmd(map, 0x70, cmd_adr);
> chip->state = FL_STATUS;
> xip_enable(map, chip, cmd_adr);
> printk(KERN_ERR "%s: buffer write error (status timeout)\n", map->name);
> @@ -1858,8 +1868,8 @@ static int __xipram do_write_buffer(struct map_info *map, struct flchip *chip,
> unsigned long chipstatus = MERGESTATUS(status);
>
> /* reset status */
> - map_write(map, CMD(0x50), cmd_adr);
> - map_write(map, CMD(0x70), cmd_adr);
> + cfi_write_cmd(map, 0x50, cmd_adr);
> + cfi_write_cmd(map, 0x70, cmd_adr);
> xip_enable(map, chip, cmd_adr);
>
> if (chipstatus & 0x02) {
More information about the linux-mtd
mailing list