diff mbox series

dmaengine: dw-edma: fix unnecessary stack usage

Message ID 20190617131733.2429469-1-arnd@arndb.de
State Superseded
Headers show
Series dmaengine: dw-edma: fix unnecessary stack usage | expand

Commit Message

Arnd Bergmann June 17, 2019, 1:16 p.m. UTC
Putting large constant data on the stack causes unnecessary overhead
and stack usage:

drivers/dma/dw-edma/dw-edma-v0-debugfs.c:285:6: error: stack frame size of 1376 bytes in function 'dw_edma_v0_debugfs_on' [-Werror,-Wframe-larger-than=]

Mark the variable 'static const' in order for the compiler to move it
into the .rodata section where it does no such harm.

Fixes: 305aebeff879 ("dmaengine: Add Synopsys eDMA IP version 0 debugfs support")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 drivers/dma/dw-edma/dw-edma-v0-debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.20.0

Comments

Gustavo Pimentel June 21, 2019, 8:21 a.m. UTC | #1
Hi,

On Mon, Jun 17, 2019 at 14:16:45, Arnd Bergmann <arnd@arndb.de> wrote:

> Putting large constant data on the stack causes unnecessary overhead

> and stack usage:

> 

> drivers/dma/dw-edma/dw-edma-v0-debugfs.c:285:6: error: stack frame size of 1376 bytes in function 'dw_edma_v0_debugfs_on' [-Werror,-Wframe-larger-than=]


I had that warning at the beginning of the development, that's why I 
divided the debugfs entries into several subfunctions. Perhaps my 
configuration has configured a bigger stack frame size than your 
configuration.

> 

> Mark the variable 'static const' in order for the compiler to move it

> into the .rodata section where it does no such harm.


Ok, sounds good.
Thanks.

> 

> Fixes: 305aebeff879 ("dmaengine: Add Synopsys eDMA IP version 0 debugfs support")

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---

>  drivers/dma/dw-edma/dw-edma-v0-debugfs.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c

> index affa1ccaf7a0..42739508c0d8 100644

> --- a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c

> +++ b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c

> @@ -48,7 +48,7 @@ static struct {

>  } lim[2][EDMA_V0_MAX_NR_CH];

>  

>  struct debugfs_entries {

> -	char					name[24];

> +	const char				*name;

>  	dma_addr_t				*reg;

>  };

>  

> -- 

> 2.20.0
Arnd Bergmann June 21, 2019, 9:05 a.m. UTC | #2
On Fri, Jun 21, 2019 at 10:21 AM Gustavo Pimentel
<Gustavo.Pimentel@synopsys.com> wrote:

> On Mon, Jun 17, 2019 at 14:16:45, Arnd Bergmann <arnd@arndb.de> wrote:

>

> > Putting large constant data on the stack causes unnecessary overhead

> > and stack usage:

> >

> > drivers/dma/dw-edma/dw-edma-v0-debugfs.c:285:6: error: stack frame size of 1376 bytes in function 'dw_edma_v0_debugfs_on' [-Werror,-Wframe-larger-than=]

>

> I had that warning at the beginning of the development, that's why I

> divided the debugfs entries into several subfunctions. Perhaps my

> configuration has configured a bigger stack frame size than your

> configuration.


I suspect the problem is CONFIG_GCC_PLUGIN_STRUCTLEAK_VERBOSE,
which is a farily new configuration option that has the unintended side-effect
of sometimes preventing the compiler combining local variables from functions
inlined into the caller. Adding 'noinline_for_stack' to the sub-functions would
have also prevented this, but I think just not having the strings on the
stack is better anyway.

        Arnd
diff mbox series

Patch

diff --git a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
index affa1ccaf7a0..42739508c0d8 100644
--- a/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
+++ b/drivers/dma/dw-edma/dw-edma-v0-debugfs.c
@@ -48,7 +48,7 @@  static struct {
 } lim[2][EDMA_V0_MAX_NR_CH];
 
 struct debugfs_entries {
-	char					name[24];
+	const char				*name;
 	dma_addr_t				*reg;
 };