dmaengine: dw-edma: reduce stack usage after debugfs rework
Subsystems (2)
PATCH 1/1 dmaengine: dw-edma: reduce stack usage after debugfs rework
From: Arnd Bergmann <[email protected]>
After the dw_edma_debugfs_entry arrays are no longer compile-time constant, they take up space on the stack, which exceeds the warning limit after inlining:
drivers/dma/dw-edma/dw-edma-v0-debugfs.c:280:6: error: stack frame size (1784) exceeds limit (1400) in 'dw_edma_v0_debugfs_on' [-Werror,-Wframe-larger-than] void dw_edma_v0_debugfs_on(struct dw_edma *dw)
Work around this by marking the functions with the largest arrays as 'noinline_for_stack' to make them not use up space on the same stack together.
--- drivers/dma/dw-edma/dw-edma-v0-debugfs.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)
drivers/dma/dw-edma/dw-edma-v0-debugfs.c +8 -5
@@ -116,7 +116,8 @@ static void dw_edma_debugfs_create_x32(struct dw_edma *dw, } } -static void dw_edma_debugfs_regs_ch (struct dw_edma *dw,enum dw_edma_dir dir, +static noinline_for_stack void +dw_edma_debugfs_regs_ch (struct dw_edma *dw,enum dw_edma_dir dir,u16 ch,struct dentry *dent) {struct dw_edma_debugfs_entry debugfs_regs[] = {
@@ -136,9 +137,10 @@ static void dw_edma_debugfs_regs_ch(struct dw_edma *dw, enum dw_edma_dir dir,dw_edma_debugfs_create_x32 (dw, debugfs_regs, nr_entries, dent); } -static void dw_edma_debugfs_regs_wr (struct dw_edma *dw,struct dentry *dent) +static noinline_for_stack void +dw_edma_debugfs_regs_wr (struct dw_edma *dw,struct dentry *dent) { -static const struct dw_edma_debugfs_entry debugfs_regs[] = { +const struct dw_edma_debugfs_entry debugfs_regs[] = {/* eDMA global registers */ WR_REGISTER (dw, engine_en),WR_REGISTER (dw, doorbell),
@@ -159,7 +161,7 @@ static void dw_edma_debugfs_regs_wr(struct dw_edma *dw, struct dentry *dent)WR_REGISTER (dw, ch67_imwr_data), WR_REGISTER(dw, linked_list_err_en), }; -static const struct dw_edma_debugfs_entry debugfs_unroll_regs[] = { +const struct dw_edma_debugfs_entry debugfs_unroll_regs[] = {/* eDMA channel context grouping */ WR_REGISTER_UNROLL (dw, engine_chgroup), WR_REGISTER_UNROLL(dw, engine_hshake_cnt.lsb),
@@ -197,7 +199,8 @@ static void dw_edma_debugfs_regs_wr(struct dw_edma *dw, struct dentry *dent) } } -static void dw_edma_debugfs_regs_rd (struct dw_edma *dw,struct dentry *dent) +static noinline void +dw_edma_debugfs_regs_rd (struct dw_edma *dw,struct dentry *dent) {const struct dw_edma_debugfs_entry debugfs_regs[] = {/* eDMA global registers */
Signature · 1 line
2.39.0
Hi Arnd
Quoted 6 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
After the
dw_edma_debugfs_entryarrays are no longer compile-time constant, they take up space on the stack, which exceeds the warning limit after inlining:
Quoted 7 lines
drivers/dma/dw-edma/dw-edma-v0-debugfs.c:280:6: error: stack frame size (1784) exceeds limit (1400) in 'dw_edma_v0_debugfs_on' [-Werror,-Wframe-larger-than] voiddw_edma_v0_debugfs_on(struct dw_edma *dw)Work around this by marking the functions with the largest arrays as '
noinline_for_stack' to make them not use up space on the same stack together.
You mean the same stack !frame!, right?
Quoted 12 lines
Fixes 5c0373eafd83 ("dmaengine: dw-edma: Move eDMA data pointer to debugfs node descriptor")Signed-off-by Arnd Bergmann <[email protected]>--- drivers/dma/dw-edma/dw-edma-v0-debugfs.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)drivers/dma/dw-edma/dw-edma-v0-debugfs.c +0
-0
drivers/dma/dw-edma/dw-edma-v0-debugfs.c static void dw_edma_debugfs_create_x32(struct dw_edma *dw, · line 122
Quoted 8 lines
drivers/dma/dw-edma/dw-edma-v0-debugfs.c +2
-1@@ -116,7 +116,8 @@ static void dw_edma_debugfs_create_x32(struct dw_edma *dw, } } -static void dw_edma_debugfs_regs_ch (struct dw_edma *dw,enum dw_edma_dir dir, +static noinline_for_stack void +dw_edma_debugfs_regs_ch (struct dw_edma *dw,enum dw_edma_dir dir,u16 ch,struct dentry *dent)
This doesn't seem like required. Does it? This method is called from two functions: dw_edma_debugfs_regs_wr() and dw_edma_debugfs_regs_rd() and if I am not mistaken will unlikely be inlined. Even if compiler will inline it the stack memory consumption won't change much since the functions aren't called from each other.
Quoted 17 lines
drivers/dma/dw-edma/dw-edma-v0-debugfs.c +0
-0{drivers/dma/dw-edma/dw-edma-v0-debugfs.c +3
-2struct dw_edma_debugfs_entry debugfs_regs[] = {@@ -136,9 +137,10 @@ static void dw_edma_debugfs_regs_ch(struct dw_edma *dw, enum dw_edma_dir dir,dw_edma_debugfs_create_x32 (dw, debugfs_regs, nr_entries, dent); } -static void dw_edma_debugfs_regs_wr (struct dw_edma *dw,struct dentry *dent) +static noinline_for_stack void +dw_edma_debugfs_regs_wr (struct dw_edma *dw,struct dentry *dent) { -static const struct dw_edma_debugfs_entry debugfs_regs[] = { +const struct dw_edma_debugfs_entry debugfs_regs[] = {/* eDMA global registers */ WR_REGISTER (dw, engine_en),drivers/dma/dw-edma/dw-edma-v0-debugfs.c +0
-0WR_REGISTER(dw, doorbell),
drivers/dma/dw-edma/dw-edma-v0-debugfs.c static void dw_edma_debugfs_regs_wr(struct dw_edma *dw, struct dentry *dent) · line 164
Quoted 5 lines
drivers/dma/dw-edma/dw-edma-v0-debugfs.c +1
-1@@ -159,7 +161,7 @@ static void dw_edma_debugfs_regs_wr(struct dw_edma *dw, struct dentry *dent)WR_REGISTER (dw, ch67_imwr_data), WR_REGISTER(dw, linked_list_err_en), }; - static const struct dw_edma_debugfs_entry debugfs_unroll_regs[] = { + const struct dw_edma_debugfs_entry debugfs_unroll_regs[] = {
I am confused how come these locals've turned to be static const? Neither my patches nor the Bjorn repo provide such changes:
https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/tree/drivers/dma/dw-edma/dw-edma-v0-debugfs.c?h=next#n139
they were supposed to be just const.
What repo is this patch based on?
drivers/dma/dw-edma/dw-edma-v0-debugfs.c static void dw_edma_debugfs_regs_wr(struct dw_edma *dw, struct dentry *dent) · line 203
Quoted 10 lines
drivers/dma/dw-edma/dw-edma-v0-debugfs.c +1
-1/* eDMA channel context grouping */ WR_REGISTER_UNROLL (dw, engine_chgroup), WR_REGISTER_UNROLL(dw, engine_hshake_cnt.lsb),@@ -197,7 +199,8 @@ static void dw_edma_debugfs_regs_wr(struct dw_edma *dw, struct dentry *dent) } } -static void dw_edma_debugfs_regs_rd (struct dw_edma *dw,struct dentry *dent) +static noinline void
noinline_for_stack, right?
-Serge(y)
Quoted 7 lines
drivers/dma/dw-edma/dw-edma-v0-debugfs.c +1
-0+dw_edma_debugfs_regs_rd(struct dw_edma *dw, struct dentry *dent) { const struct dw_edma_debugfs_entry debugfs_regs[] = {/* eDMA global registers */
Signature · 1 line
2.39.0
Quoted 13 lines · Serge Semin
Hi Arnd
Quoted 9 lines · Arnd Bergmann
From: Arnd Bergmann <[email protected]>
After the
dw_edma_debugfs_entryarrays are no longer compile-time constant, they take up space on the stack, which exceeds the warning limit after inlining:
drivers/dma/dw-edma/dw-edma-v0-debugfs.c:280:6: error: stack frame size (1784) exceeds limit (1400) in 'dw_edma_v0_debugfs_on' [-Werror,-Wframe-larger-than] voiddw_edma_v0_debugfs_on(struct dw_edma *dw)
Quoted 5 lines
Quoted 3 lines
Work around this by marking the functions with the largest arrays as '
noinline_for_stack' to make them not use up space on the same stack together.You mean the same stack !frame!, right?
Right.
Quoted 10 lines
drivers/dma/dw-edma/dw-edma-v0-debugfs.c static void dw_edma_debugfs_create_x32(struct dw_edma *dw, · line 121
Quoted 4 lines
drivers/dma/dw-edma/dw-edma-v0-debugfs.c +2
-1-static void dw_edma_debugfs_regs_ch (struct dw_edma *dw,enum dw_edma_dir dir, +static noinline_for_stack void +dw_edma_debugfs_regs_ch (struct dw_edma *dw,enum dw_edma_dir dir,u16 ch,struct dentry *dent)This doesn't seem like required. Does it? This method is called from two functions:
dw_edma_debugfs_regs_wr()anddw_edma_debugfs_regs_rd()and if I am not mistaken will unlikely be inlined. Even if compiler will inline it the stack memory consumption won't change much since the functions aren't called from each other.
I did this mainly for consistency, marking all the functions that have an array of dw_edma_debugfs_entry. I have retested without this change now and adapted the changelog accordingly.
Quoted 12 lines
drivers/dma/dw-edma/dw-edma-v0-debugfs.c static void dw_edma_debugfs_regs_wr(struct dw_edma *dw, struct dentry *dent) · line 164
Quoted 6 lines
drivers/dma/dw-edma/dw-edma-v0-debugfs.c +1
-1@@ -159,7 +161,7 @@ static void dw_edma_debugfs_regs_wr(struct dw_edma *dw, struct dentry *dent)WR_REGISTER (dw, ch67_imwr_data), WR_REGISTER(dw, linked_list_err_en), }; - static const struct dw_edma_debugfs_entry debugfs_unroll_regs[] = { + const struct dw_edma_debugfs_entry debugfs_unroll_regs[] = {I am confused how come these locals've turned to be static const? Neither my patches nor the Bjorn repo provide such changes:
https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/tree/drivers/dma/dw-edma/dw-edma-v0-debugfs.c?h=next#n139
they were supposed to be just const.
Quoted 1 line
What repo is this patch based on?
My mistake, that was a revert from an earlier broken attempt to fix the issue. It's fixed now after reverting the other patch and rebasing.
Quoted 8 lines
Quoted 7 lines
drivers/dma/dw-edma/dw-edma-v0-debugfs.c +0
-0/* eDMA channel context grouping */ WR_REGISTER_UNROLL (dw, engine_chgroup), WR_REGISTER_UNROLL(dw, engine_hshake_cnt.lsb),@@ -197,7 +199,8 @@ static void dw_edma_debugfs_regs_wr(struct dw_edma *dw, struct dentry *dent) } }
Quoted 4 lines
drivers/dma/dw-edma/dw-edma-v0-debugfs.c static void dw_edma_debugfs_regs_wr(struct dw_edma *dw, struct dentry *dent) · line 202
Quoted 2 lines
drivers/dma/dw-edma/dw-edma-v0-debugfs.c +1
-1-static void dw_edma_debugfs_regs_rd (struct dw_edma *dw,struct dentry *dent) +static noinline void
noinline_for_stack, right?
Fixed.
Arnd