| Message ID | 20210920122017.205975-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dmaengine: remove debugfs #ifdef | expand |
Hi Arnd, Thank you for the patch. On Mon, Sep 20, 2021 at 02:20:07PM +0200, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > The ptdma driver has added debugfs support, but this fails to build > when debugfs is disabled: > > drivers/dma/ptdma/ptdma-debugfs.c: In function 'ptdma_debugfs_setup': > drivers/dma/ptdma/ptdma-debugfs.c:93:54: error: 'struct dma_device' has no member named 'dbg_dev_root' > 93 | debugfs_create_file("info", 0400, pt->dma_dev.dbg_dev_root, pt, > | ^ > drivers/dma/ptdma/ptdma-debugfs.c:96:55: error: 'struct dma_device' has no member named 'dbg_dev_root' > 96 | debugfs_create_file("stats", 0400, pt->dma_dev.dbg_dev_root, pt, > | ^ > drivers/dma/ptdma/ptdma-debugfs.c:102:52: error: 'struct dma_device' has no member named 'dbg_dev_root' > 102 | debugfs_create_dir("q", pt->dma_dev.dbg_dev_root); > | ^ > > Remove the #ifdef in the header, as this only saves a few bytes, > but would require ugly #ifdefs in each driver using it. > Simplify the other user while we're at it. > > Fixes: e2fb2e2a33fa ("dmaengine: ptdma: Add debugfs entries for PTDMA") > Fixes: 26cf132de6f7 ("dmaengine: Create debug directories for DMA devices") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/dma/xilinx/xilinx_dpdma.c | 15 +-------------- > include/linux/dmaengine.h | 2 -- > 2 files changed, 1 insertion(+), 16 deletions(-) > > diff --git a/drivers/dma/xilinx/xilinx_dpdma.c b/drivers/dma/xilinx/xilinx_dpdma.c > index b280a53e8570..ce5c66e6897d 100644 > --- a/drivers/dma/xilinx/xilinx_dpdma.c > +++ b/drivers/dma/xilinx/xilinx_dpdma.c > @@ -271,9 +271,6 @@ struct xilinx_dpdma_device { > /* ----------------------------------------------------------------------------- > * DebugFS > */ > - > -#ifdef CONFIG_DEBUG_FS > - It's only a few bytes of data in struct dma_device, but a bit more in .text here. Is the simplification really required in this driver ? > #define XILINX_DPDMA_DEBUGFS_READ_MAX_SIZE 32 > #define XILINX_DPDMA_DEBUGFS_UINT16_MAX_STR "65535" > > @@ -299,7 +296,7 @@ struct xilinx_dpdma_debugfs_request { > > static void xilinx_dpdma_debugfs_desc_done_irq(struct xilinx_dpdma_chan *chan) > { > - if (chan->id == dpdma_debugfs.chan_id) > + if (IS_ENABLED(CONFIG_DEBUG_FS) && chan->id == dpdma_debugfs.chan_id) > dpdma_debugfs.xilinx_dpdma_irq_done_count++; > } > > @@ -462,16 +459,6 @@ static void xilinx_dpdma_debugfs_init(struct xilinx_dpdma_device *xdev) > dev_err(xdev->dev, "Failed to create debugfs testcase file\n"); > } > > -#else > -static void xilinx_dpdma_debugfs_init(struct xilinx_dpdma_device *xdev) > -{ > -} > - > -static void xilinx_dpdma_debugfs_desc_done_irq(struct xilinx_dpdma_chan *chan) > -{ > -} > -#endif /* CONFIG_DEBUG_FS */ > - > /* ----------------------------------------------------------------------------- > * I/O Accessors > */ > diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h > index e5c2c9e71bf1..9000f3ffce8b 100644 > --- a/include/linux/dmaengine.h > +++ b/include/linux/dmaengine.h > @@ -944,10 +944,8 @@ struct dma_device { > void (*device_issue_pending)(struct dma_chan *chan); > void (*device_release)(struct dma_device *dev); > /* debugfs support */ > -#ifdef CONFIG_DEBUG_FS > void (*dbg_summary_show)(struct seq_file *s, struct dma_device *dev); > struct dentry *dbg_dev_root; > -#endif > }; > > static inline int dmaengine_slave_config(struct dma_chan *chan,
On Mon, Sep 20, 2021 at 2:47 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > It's only a few bytes of data in struct dma_device, but a bit more in > .text here. Is the simplification really required in this driver ? The intention was to not change the resulting object code in this driver, and I still don't see where it would grow after dead-code-elimination removes all the unused static functions. What am I missing? Arnd
Hi Arnd, On Mon, Sep 20, 2021 at 02:50:52PM +0200, Arnd Bergmann wrote: > On Mon, Sep 20, 2021 at 2:47 PM Laurent Pinchart wrote: > > > > It's only a few bytes of data in struct dma_device, but a bit more in > > .text here. Is the simplification really required in this driver ? > > The intention was to not change the resulting object code in this driver, > and I still don't see where it would grow after dead-code-elimination removes > all the unused static functions. What am I missing? Indeed, gcc does a fairly good job there. The .text section doesn't grow. Interestingly, there's an increase in size in the .data and .rodata sections in the xilinx-dpdma module: - 8 .rodata.str1.8 0000029f 0000000000000000 0000000000000000 00003660 2**3 + 8 .rodata.str1.8 000002a7 0000000000000000 0000000000000000 00003660 2**3 - 10 .rodata 00001080 0000000000000000 0000000000000000 00003960 2**5 + 10 .rodata 000010e0 0000000000000000 0000000000000000 00003960 2**5 - 15 .data 00001050 0000000000000000 0000000000000000 00004e40 2**5 + 15 .data 00001090 0000000000000000 0000000000000000 00004ea0 2**5 I'm not entirely sure where it comes from, it may be related to instrumentation caused by debugging options. For your patch, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
On 20-09-21, 14:20, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > The ptdma driver has added debugfs support, but this fails to build > when debugfs is disabled: > > drivers/dma/ptdma/ptdma-debugfs.c: In function 'ptdma_debugfs_setup': > drivers/dma/ptdma/ptdma-debugfs.c:93:54: error: 'struct dma_device' has no member named 'dbg_dev_root' > 93 | debugfs_create_file("info", 0400, pt->dma_dev.dbg_dev_root, pt, > | ^ > drivers/dma/ptdma/ptdma-debugfs.c:96:55: error: 'struct dma_device' has no member named 'dbg_dev_root' > 96 | debugfs_create_file("stats", 0400, pt->dma_dev.dbg_dev_root, pt, > | ^ > drivers/dma/ptdma/ptdma-debugfs.c:102:52: error: 'struct dma_device' has no member named 'dbg_dev_root' > 102 | debugfs_create_dir("q", pt->dma_dev.dbg_dev_root); > | ^ > > Remove the #ifdef in the header, as this only saves a few bytes, > but would require ugly #ifdefs in each driver using it. > Simplify the other user while we're at it. Applied, thanks
diff --git a/drivers/dma/xilinx/xilinx_dpdma.c b/drivers/dma/xilinx/xilinx_dpdma.c index b280a53e8570..ce5c66e6897d 100644 --- a/drivers/dma/xilinx/xilinx_dpdma.c +++ b/drivers/dma/xilinx/xilinx_dpdma.c @@ -271,9 +271,6 @@ struct xilinx_dpdma_device { /* ----------------------------------------------------------------------------- * DebugFS */ - -#ifdef CONFIG_DEBUG_FS - #define XILINX_DPDMA_DEBUGFS_READ_MAX_SIZE 32 #define XILINX_DPDMA_DEBUGFS_UINT16_MAX_STR "65535" @@ -299,7 +296,7 @@ struct xilinx_dpdma_debugfs_request { static void xilinx_dpdma_debugfs_desc_done_irq(struct xilinx_dpdma_chan *chan) { - if (chan->id == dpdma_debugfs.chan_id) + if (IS_ENABLED(CONFIG_DEBUG_FS) && chan->id == dpdma_debugfs.chan_id) dpdma_debugfs.xilinx_dpdma_irq_done_count++; } @@ -462,16 +459,6 @@ static void xilinx_dpdma_debugfs_init(struct xilinx_dpdma_device *xdev) dev_err(xdev->dev, "Failed to create debugfs testcase file\n"); } -#else -static void xilinx_dpdma_debugfs_init(struct xilinx_dpdma_device *xdev) -{ -} - -static void xilinx_dpdma_debugfs_desc_done_irq(struct xilinx_dpdma_chan *chan) -{ -} -#endif /* CONFIG_DEBUG_FS */ - /* ----------------------------------------------------------------------------- * I/O Accessors */ diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h index e5c2c9e71bf1..9000f3ffce8b 100644 --- a/include/linux/dmaengine.h +++ b/include/linux/dmaengine.h @@ -944,10 +944,8 @@ struct dma_device { void (*device_issue_pending)(struct dma_chan *chan); void (*device_release)(struct dma_device *dev); /* debugfs support */ -#ifdef CONFIG_DEBUG_FS void (*dbg_summary_show)(struct seq_file *s, struct dma_device *dev); struct dentry *dbg_dev_root; -#endif }; static inline int dmaengine_slave_config(struct dma_chan *chan,