diff mbox series

[RFC,1/3] block: move main block debugfs initialization to its own file

Message ID 20200402000002.7442-2-mcgrof@kernel.org
State New, archived
Headers show
Series block: address blktrace use-after-free | expand

Commit Message

Luis Chamberlain April 2, 2020, midnight UTC
Single and multiqeueue block devices share some debugfs code. By
moving this into its own file it makes it easier to expand and audit
this shared code.

This patch contains no functional changes.

Cc: Bart Van Assche <bvanassche@acm.org>
Cc: Omar Sandoval <osandov@fb.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Nicolai Stange <nstange@suse.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Michal Hocko <mhocko@kernel.org>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 block/Makefile      |  1 +
 block/blk-core.c    |  9 +--------
 block/blk-debugfs.c | 15 +++++++++++++++
 block/blk.h         |  7 +++++++
 4 files changed, 24 insertions(+), 8 deletions(-)
 create mode 100644 block/blk-debugfs.c

Comments

Bart Van Assche April 5, 2020, 3:12 a.m. UTC | #1
On 2020-04-01 17:00, Luis Chamberlain wrote:
> Single and multiqeueue block devices share some debugfs code. By
             ^^^^^^^^^^^
             multiqueue?
> moving this into its own file it makes it easier to expand and audit
> this shared code.

[ ... ]

> diff --git a/block/blk-debugfs.c b/block/blk-debugfs.c
> new file mode 100644
> index 000000000000..634dea4b1507
> --- /dev/null
> +++ b/block/blk-debugfs.c
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Shared debugfs mq / non-mq functionality
> + */

The legacy block layer is gone, so not sure why the above comment refers
to non-mq?

> diff --git a/block/blk.h b/block/blk.h
> index 0a94ec68af32..86a66b614f08 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -487,5 +487,12 @@ struct request_queue *__blk_alloc_queue(int node_id);
>  int __bio_add_pc_page(struct request_queue *q, struct bio *bio,
>  		struct page *page, unsigned int len, unsigned int offset,
>  		bool *same_page);
> +#ifdef CONFIG_DEBUG_FS
> +void blk_debugfs_register(void);
> +#else
> +static inline void blk_debugfs_register(void)
> +{
> +}
> +#endif /* CONFIG_DEBUG_FS */

Do we really need a new header file that only declares a single
function? How about adding the above into block/blk-mq-debugfs.h?

Thanks,

Bart.
Luis Chamberlain April 6, 2020, 2:23 p.m. UTC | #2
On Sat, Apr 04, 2020 at 08:12:53PM -0700, Bart Van Assche wrote:
> On 2020-04-01 17:00, Luis Chamberlain wrote:
> > Single and multiqeueue block devices share some debugfs code. By
>              ^^^^^^^^^^^
>              multiqueue?
> > moving this into its own file it makes it easier to expand and audit
> > this shared code.
> 
> [ ... ]
> 
> > diff --git a/block/blk-debugfs.c b/block/blk-debugfs.c
> > new file mode 100644
> > index 000000000000..634dea4b1507
> > --- /dev/null
> > +++ b/block/blk-debugfs.c
> > @@ -0,0 +1,15 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/*
> > + * Shared debugfs mq / non-mq functionality
> > + */
> 
> The legacy block layer is gone, so not sure why the above comment refers
> to non-mq?

Will adjust the language, thanks.

> 
> > diff --git a/block/blk.h b/block/blk.h
> > index 0a94ec68af32..86a66b614f08 100644
> > --- a/block/blk.h
> > +++ b/block/blk.h
> > @@ -487,5 +487,12 @@ struct request_queue *__blk_alloc_queue(int node_id);
> >  int __bio_add_pc_page(struct request_queue *q, struct bio *bio,
> >  		struct page *page, unsigned int len, unsigned int offset,
> >  		bool *same_page);
> > +#ifdef CONFIG_DEBUG_FS
> > +void blk_debugfs_register(void);
> > +#else
> > +static inline void blk_debugfs_register(void)
> > +{
> > +}
> > +#endif /* CONFIG_DEBUG_FS */
> 
> Do we really need a new header file that only declares a single
> function? How about adding the above into block/blk-mq-debugfs.h?

Moving forward rq->debugfs_dir will created when CONFIG_DEBUG_FS is
enabled to enable blktrace to use it. This creation won't depend on
CONFIG_BLK_DEBUG_FS, so we can definitely sprinkly the #ifdef
CONFIG_DEBUG_FS stuff in block/blk-mq-debugfs.h but it just didn't
seem the best place. Let me know.

  Luis
diff mbox series

Patch

diff --git a/block/Makefile b/block/Makefile
index 206b96e9387f..1d3ab20505d8 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -10,6 +10,7 @@  obj-$(CONFIG_BLOCK) := bio.o elevator.o blk-core.o blk-sysfs.o \
 			blk-mq-sysfs.o blk-mq-cpumap.o blk-mq-sched.o ioctl.o \
 			genhd.o ioprio.o badblocks.o partitions/ blk-rq-qos.o
 
+obj-$(CONFIG_DEBUG_FS)		+= blk-debugfs.o
 obj-$(CONFIG_BOUNCE)		+= bounce.o
 obj-$(CONFIG_BLK_SCSI_REQUEST)	+= scsi_ioctl.o
 obj-$(CONFIG_BLK_DEV_BSG)	+= bsg.o
diff --git a/block/blk-core.c b/block/blk-core.c
index 7e4a1da0715e..5aaae7a1b338 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -48,10 +48,6 @@ 
 #include "blk-pm.h"
 #include "blk-rq-qos.h"
 
-#ifdef CONFIG_DEBUG_FS
-struct dentry *blk_debugfs_root;
-#endif
-
 EXPORT_TRACEPOINT_SYMBOL_GPL(block_bio_remap);
 EXPORT_TRACEPOINT_SYMBOL_GPL(block_rq_remap);
 EXPORT_TRACEPOINT_SYMBOL_GPL(block_bio_complete);
@@ -1796,10 +1792,7 @@  int __init blk_dev_init(void)
 
 	blk_requestq_cachep = kmem_cache_create("request_queue",
 			sizeof(struct request_queue), 0, SLAB_PANIC, NULL);
-
-#ifdef CONFIG_DEBUG_FS
-	blk_debugfs_root = debugfs_create_dir("block", NULL);
-#endif
+	blk_debugfs_register();
 
 	return 0;
 }
diff --git a/block/blk-debugfs.c b/block/blk-debugfs.c
new file mode 100644
index 000000000000..634dea4b1507
--- /dev/null
+++ b/block/blk-debugfs.c
@@ -0,0 +1,15 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Shared debugfs mq / non-mq functionality
+ */
+#include <linux/kernel.h>
+#include <linux/blkdev.h>
+#include <linux/debugfs.h>
+
+struct dentry *blk_debugfs_root;
+
+void blk_debugfs_register(void)
+{
+	blk_debugfs_root = debugfs_create_dir("block", NULL);
+}
diff --git a/block/blk.h b/block/blk.h
index 0a94ec68af32..86a66b614f08 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -487,5 +487,12 @@  struct request_queue *__blk_alloc_queue(int node_id);
 int __bio_add_pc_page(struct request_queue *q, struct bio *bio,
 		struct page *page, unsigned int len, unsigned int offset,
 		bool *same_page);
+#ifdef CONFIG_DEBUG_FS
+void blk_debugfs_register(void);
+#else
+static inline void blk_debugfs_register(void)
+{
+}
+#endif /* CONFIG_DEBUG_FS */
 
 #endif /* BLK_INTERNAL_H */