| Message ID | 20200505140136.263461-1-arnd@arndb.de |
|---|---|
| State | Accepted |
| Delegated to: | Miquel Raynal |
| Headers | show |
| Series | mtd: lpddr: fix excessive stack usage with clang | expand |
On Tue, May 05, 2020 at 04:01:16PM +0200, Arnd Bergmann wrote: > Building lpddr2_nvm with clang can result in a giant stack usage > in one function: > > drivers/mtd/lpddr/lpddr2_nvm.c:399:12: error: stack frame size of 1144 bytes in function 'lpddr2_nvm_probe' [-Werror,-Wframe-larger-than=] > > The problem is that clang decides to build a copy of the mtd_info > structure on the stack and then do a memcpy() into the actual version. It > shouldn't really do it that way, but it's not strictly a bug either. > > As a workaround, use a static const version of the structure to assign > most of the members upfront and then only set the few members that > require runtime knowledge at probe time. > > Fixes: 96ba9dd65788 ("mtd: lpddr: add driver for LPDDR2-NVM PCM memories") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> > --- > drivers/mtd/lpddr/lpddr2_nvm.c | 35 ++++++++++++++++++---------------- > 1 file changed, 19 insertions(+), 16 deletions(-) > > diff --git a/drivers/mtd/lpddr/lpddr2_nvm.c b/drivers/mtd/lpddr/lpddr2_nvm.c > index 0f1547f09d08..72f5c7b30079 100644 > --- a/drivers/mtd/lpddr/lpddr2_nvm.c > +++ b/drivers/mtd/lpddr/lpddr2_nvm.c > @@ -393,6 +393,17 @@ static int lpddr2_nvm_lock(struct mtd_info *mtd, loff_t start_add, > return lpddr2_nvm_do_block_op(mtd, start_add, len, LPDDR2_NVM_LOCK); > } > > +static const struct mtd_info lpddr2_nvm_mtd_info = { > + .type = MTD_RAM, > + .writesize = 1, > + .flags = (MTD_CAP_NVRAM | MTD_POWERUP_LOCK), > + ._read = lpddr2_nvm_read, > + ._write = lpddr2_nvm_write, > + ._erase = lpddr2_nvm_erase, > + ._unlock = lpddr2_nvm_unlock, > + ._lock = lpddr2_nvm_lock, > +}; > + > /* > * lpddr2_nvm driver probe method > */ > @@ -433,6 +444,7 @@ static int lpddr2_nvm_probe(struct platform_device *pdev) > .pfow_base = OW_BASE_ADDRESS, > .fldrv_priv = pcm_data, > }; > + > if (IS_ERR(map->virt)) > return PTR_ERR(map->virt); > > @@ -444,22 +456,13 @@ static int lpddr2_nvm_probe(struct platform_device *pdev) > return PTR_ERR(pcm_data->ctl_regs); > > /* Populate mtd_info data structure */ > - *mtd = (struct mtd_info) { > - .dev = { .parent = &pdev->dev }, > - .name = pdev->dev.init_name, > - .type = MTD_RAM, > - .priv = map, > - .size = resource_size(add_range), > - .erasesize = ERASE_BLOCKSIZE * pcm_data->bus_width, > - .writesize = 1, > - .writebufsize = WRITE_BUFFSIZE * pcm_data->bus_width, > - .flags = (MTD_CAP_NVRAM | MTD_POWERUP_LOCK), > - ._read = lpddr2_nvm_read, > - ._write = lpddr2_nvm_write, > - ._erase = lpddr2_nvm_erase, > - ._unlock = lpddr2_nvm_unlock, > - ._lock = lpddr2_nvm_lock, > - }; > + *mtd = lpddr2_nvm_mtd_info; > + mtd->dev.parent = &pdev->dev; > + mtd->name = pdev->dev.init_name; > + mtd->priv = map; > + mtd->size = resource_size(add_range); > + mtd->erasesize = ERASE_BLOCKSIZE * pcm_data->bus_width; > + mtd->writebufsize = WRITE_BUFFSIZE * pcm_data->bus_width; > > /* Verify the presence of the device looking for PFOW string */ > if (!lpddr2_nvm_pfow_present(map)) { > -- > 2.26.0 >
Nathan Chancellor <natechancellor@gmail.com> wrote on Tue, 5 May 2020 19:38:28 -0700: > On Tue, May 05, 2020 at 04:01:16PM +0200, Arnd Bergmann wrote: > > Building lpddr2_nvm with clang can result in a giant stack usage > > in one function: > > > > drivers/mtd/lpddr/lpddr2_nvm.c:399:12: error: stack frame size of 1144 bytes in function 'lpddr2_nvm_probe' [-Werror,-Wframe-larger-than=] > > > > The problem is that clang decides to build a copy of the mtd_info > > structure on the stack and then do a memcpy() into the actual version. It > > shouldn't really do it that way, but it's not strictly a bug either. > > > > As a workaround, use a static const version of the structure to assign > > most of the members upfront and then only set the few members that > > require runtime knowledge at probe time. > > > > Fixes: 96ba9dd65788 ("mtd: lpddr: add driver for LPDDR2-NVM PCM memories") > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> Acked-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > --- > > drivers/mtd/lpddr/lpddr2_nvm.c | 35 ++++++++++++++++++---------------- > > 1 file changed, 19 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/mtd/lpddr/lpddr2_nvm.c b/drivers/mtd/lpddr/lpddr2_nvm.c > > index 0f1547f09d08..72f5c7b30079 100644 > > --- a/drivers/mtd/lpddr/lpddr2_nvm.c > > +++ b/drivers/mtd/lpddr/lpddr2_nvm.c > > @@ -393,6 +393,17 @@ static int lpddr2_nvm_lock(struct mtd_info *mtd, loff_t start_add, > > return lpddr2_nvm_do_block_op(mtd, start_add, len, LPDDR2_NVM_LOCK); > > } > > > > +static const struct mtd_info lpddr2_nvm_mtd_info = { > > + .type = MTD_RAM, > > + .writesize = 1, > > + .flags = (MTD_CAP_NVRAM | MTD_POWERUP_LOCK), > > + ._read = lpddr2_nvm_read, > > + ._write = lpddr2_nvm_write, > > + ._erase = lpddr2_nvm_erase, > > + ._unlock = lpddr2_nvm_unlock, > > + ._lock = lpddr2_nvm_lock, > > +}; > > + > > /* > > * lpddr2_nvm driver probe method > > */ > > @@ -433,6 +444,7 @@ static int lpddr2_nvm_probe(struct platform_device *pdev) > > .pfow_base = OW_BASE_ADDRESS, > > .fldrv_priv = pcm_data, > > }; > > + > > if (IS_ERR(map->virt)) > > return PTR_ERR(map->virt); > > > > @@ -444,22 +456,13 @@ static int lpddr2_nvm_probe(struct platform_device *pdev) > > return PTR_ERR(pcm_data->ctl_regs); > > > > /* Populate mtd_info data structure */ > > - *mtd = (struct mtd_info) { > > - .dev = { .parent = &pdev->dev }, > > - .name = pdev->dev.init_name, > > - .type = MTD_RAM, > > - .priv = map, > > - .size = resource_size(add_range), > > - .erasesize = ERASE_BLOCKSIZE * pcm_data->bus_width, > > - .writesize = 1, > > - .writebufsize = WRITE_BUFFSIZE * pcm_data->bus_width, > > - .flags = (MTD_CAP_NVRAM | MTD_POWERUP_LOCK), > > - ._read = lpddr2_nvm_read, > > - ._write = lpddr2_nvm_write, > > - ._erase = lpddr2_nvm_erase, > > - ._unlock = lpddr2_nvm_unlock, > > - ._lock = lpddr2_nvm_lock, > > - }; > > + *mtd = lpddr2_nvm_mtd_info; > > + mtd->dev.parent = &pdev->dev; > > + mtd->name = pdev->dev.init_name; > > + mtd->priv = map; > > + mtd->size = resource_size(add_range); > > + mtd->erasesize = ERASE_BLOCKSIZE * pcm_data->bus_width; > > + mtd->writebufsize = WRITE_BUFFSIZE * pcm_data->bus_width; > > > > /* Verify the presence of the device looking for PFOW string */ > > if (!lpddr2_nvm_pfow_present(map)) { > > -- > > 2.26.0 > >
On Tue, 2020-05-05 at 14:01:16 UTC, Arnd Bergmann wrote: > Building lpddr2_nvm with clang can result in a giant stack usage > in one function: > > drivers/mtd/lpddr/lpddr2_nvm.c:399:12: error: stack frame size of 1144 bytes in function 'lpddr2_nvm_probe' [-Werror,-Wframe-larger-than=] > > The problem is that clang decides to build a copy of the mtd_info > structure on the stack and then do a memcpy() into the actual version. It > shouldn't really do it that way, but it's not strictly a bug either. > > As a workaround, use a static const version of the structure to assign > most of the members upfront and then only set the few members that > require runtime knowledge at probe time. > > Fixes: 96ba9dd65788 ("mtd: lpddr: add driver for LPDDR2-NVM PCM memories") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> > Acked-by: Miquel Raynal <miquel.raynal@bootlin.com> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks. Miquel
diff --git a/drivers/mtd/lpddr/lpddr2_nvm.c b/drivers/mtd/lpddr/lpddr2_nvm.c index 0f1547f09d08..72f5c7b30079 100644 --- a/drivers/mtd/lpddr/lpddr2_nvm.c +++ b/drivers/mtd/lpddr/lpddr2_nvm.c @@ -393,6 +393,17 @@ static int lpddr2_nvm_lock(struct mtd_info *mtd, loff_t start_add, return lpddr2_nvm_do_block_op(mtd, start_add, len, LPDDR2_NVM_LOCK); } +static const struct mtd_info lpddr2_nvm_mtd_info = { + .type = MTD_RAM, + .writesize = 1, + .flags = (MTD_CAP_NVRAM | MTD_POWERUP_LOCK), + ._read = lpddr2_nvm_read, + ._write = lpddr2_nvm_write, + ._erase = lpddr2_nvm_erase, + ._unlock = lpddr2_nvm_unlock, + ._lock = lpddr2_nvm_lock, +}; + /* * lpddr2_nvm driver probe method */ @@ -433,6 +444,7 @@ static int lpddr2_nvm_probe(struct platform_device *pdev) .pfow_base = OW_BASE_ADDRESS, .fldrv_priv = pcm_data, }; + if (IS_ERR(map->virt)) return PTR_ERR(map->virt); @@ -444,22 +456,13 @@ static int lpddr2_nvm_probe(struct platform_device *pdev) return PTR_ERR(pcm_data->ctl_regs); /* Populate mtd_info data structure */ - *mtd = (struct mtd_info) { - .dev = { .parent = &pdev->dev }, - .name = pdev->dev.init_name, - .type = MTD_RAM, - .priv = map, - .size = resource_size(add_range), - .erasesize = ERASE_BLOCKSIZE * pcm_data->bus_width, - .writesize = 1, - .writebufsize = WRITE_BUFFSIZE * pcm_data->bus_width, - .flags = (MTD_CAP_NVRAM | MTD_POWERUP_LOCK), - ._read = lpddr2_nvm_read, - ._write = lpddr2_nvm_write, - ._erase = lpddr2_nvm_erase, - ._unlock = lpddr2_nvm_unlock, - ._lock = lpddr2_nvm_lock, - }; + *mtd = lpddr2_nvm_mtd_info; + mtd->dev.parent = &pdev->dev; + mtd->name = pdev->dev.init_name; + mtd->priv = map; + mtd->size = resource_size(add_range); + mtd->erasesize = ERASE_BLOCKSIZE * pcm_data->bus_width; + mtd->writebufsize = WRITE_BUFFSIZE * pcm_data->bus_width; /* Verify the presence of the device looking for PFOW string */ if (!lpddr2_nvm_pfow_present(map)) {
Building lpddr2_nvm with clang can result in a giant stack usage in one function: drivers/mtd/lpddr/lpddr2_nvm.c:399:12: error: stack frame size of 1144 bytes in function 'lpddr2_nvm_probe' [-Werror,-Wframe-larger-than=] The problem is that clang decides to build a copy of the mtd_info structure on the stack and then do a memcpy() into the actual version. It shouldn't really do it that way, but it's not strictly a bug either. As a workaround, use a static const version of the structure to assign most of the members upfront and then only set the few members that require runtime knowledge at probe time. Fixes: 96ba9dd65788 ("mtd: lpddr: add driver for LPDDR2-NVM PCM memories") Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/mtd/lpddr/lpddr2_nvm.c | 35 ++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-)