| Message ID | 20230130212656.876311-2-bvanassche@acm.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for limits below the page size | expand |
On Mon, Jan 30, 2023 at 01:26:50PM -0800, Bart Van Assche wrote: > Move the code for creating the block layer debugfs root directory into > blk-mq-debugfs.c. This patch prepares for adding more debugfs > initialization code by introducing the function blk_mq_debugfs_init(). > > Cc: Christoph Hellwig <hch@lst.de> > Cc: Ming Lei <ming.lei@redhat.com> > Cc: Keith Busch <kbusch@kernel.org> > Signed-off-by: Bart Van Assche <bvanassche@acm.org> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> Luis
On Wed, Feb 01, 2023 at 12:58:00PM -0800, Luis Chamberlain wrote: > On Mon, Jan 30, 2023 at 01:26:50PM -0800, Bart Van Assche wrote: > > Move the code for creating the block layer debugfs root directory into > > blk-mq-debugfs.c. This patch prepares for adding more debugfs > > initialization code by introducing the function blk_mq_debugfs_init(). > > > > Cc: Christoph Hellwig <hch@lst.de> > > Cc: Ming Lei <ming.lei@redhat.com> > > Cc: Keith Busch <kbusch@kernel.org> > > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > > Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> Sorry but actually a neuron triggered after this to remind me of commit 85e0cbbb8a ("block: create the request_queue debugfs_dir on registration") and so using the terminology on that commit, wouldn't this not create now the root block debugfs dir for request-based block drivers? Luis
On 2/1/23 13:23, Luis Chamberlain wrote: > On Wed, Feb 01, 2023 at 12:58:00PM -0800, Luis Chamberlain wrote: >> On Mon, Jan 30, 2023 at 01:26:50PM -0800, Bart Van Assche wrote: >>> Move the code for creating the block layer debugfs root directory into >>> blk-mq-debugfs.c. This patch prepares for adding more debugfs >>> initialization code by introducing the function blk_mq_debugfs_init(). >>> >>> Cc: Christoph Hellwig <hch@lst.de> >>> Cc: Ming Lei <ming.lei@redhat.com> >>> Cc: Keith Busch <kbusch@kernel.org> >>> Signed-off-by: Bart Van Assche <bvanassche@acm.org> >> >> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> > > Sorry but actually a neuron triggered after this to remind me of commit > 85e0cbbb8a ("block: create the request_queue debugfs_dir on > registration") and so using the terminology on that commit, wouldn't > this not create now the root block debugfs dir for request-based block > drivers? Hi Luis, This patch should not change any behavior with CONFIG_DEBUG_FS=y. As one can see in include/linux/debugfs.h, debugfs_create_dir() does not create a directory with CONFIG_DEBUG_FS=n: static inline struct dentry *debugfs_create_dir(const char *name, struct dentry *parent) { return ERR_PTR(-ENODEV); } I think the only behavior change introduced by this patch is that blk_debugfs_root remains NULL with CONFIG_DEBUG_FS=n instead of being set to ERR_PTR(-ENODEV). Thanks, Bart.
On Wed, Feb 01, 2023 at 02:01:18PM -0800, Bart Van Assche wrote: > On 2/1/23 13:23, Luis Chamberlain wrote: > > On Wed, Feb 01, 2023 at 12:58:00PM -0800, Luis Chamberlain wrote: > > > On Mon, Jan 30, 2023 at 01:26:50PM -0800, Bart Van Assche wrote: > > > > Move the code for creating the block layer debugfs root directory into > > > > blk-mq-debugfs.c. This patch prepares for adding more debugfs > > > > initialization code by introducing the function blk_mq_debugfs_init(). > > > > > > > > Cc: Christoph Hellwig <hch@lst.de> > > > > Cc: Ming Lei <ming.lei@redhat.com> > > > > Cc: Keith Busch <kbusch@kernel.org> > > > > Signed-off-by: Bart Van Assche <bvanassche@acm.org> > > > > > > Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> > > > > Sorry but actually a neuron triggered after this to remind me of commit > > 85e0cbbb8a ("block: create the request_queue debugfs_dir on > > registration") and so using the terminology on that commit, wouldn't > > this not create now the root block debugfs dir for request-based block > > drivers? > > Hi Luis, > > This patch should not change any behavior with CONFIG_DEBUG_FS=y. Ignore CONFIG_DEBUG_FS=y. This is about blktrace which needs it for both types of drivers. > As one can see in include/linux/debugfs.h, debugfs_create_dir() does not > create a directory with CONFIG_DEBUG_FS=n: > > static inline struct dentry *debugfs_create_dir(const char *name, > struct dentry *parent) > { > return ERR_PTR(-ENODEV); > } > > I think the only behavior change introduced by this patch is that > blk_debugfs_root remains NULL with CONFIG_DEBUG_FS=n instead of being set to > ERR_PTR(-ENODEV). My point was commit 85e0cbbb8a made blk_debugfs_root non-null always now when debugfs is enabled for both request-based block drivers and for make_request block drivers (multiqueue). My reading is that with your patch blk_debugfs_root will not be created for request-based block drivers. Luis
On 2/1/23 15:59, Luis Chamberlain wrote: > My point was commit 85e0cbbb8a made blk_debugfs_root non-null always now > when debugfs is enabled for both request-based block drivers and for > make_request block drivers (multiqueue). My reading is that with your > patch blk_debugfs_root will not be created for request-based block > drivers. Hi Luis, The empty version of blk_mq_debugfs_init() in my patch is only selected if CONFIG_BLK_DEBUG_FS=n. I think that my patch preserves the behavior that /sys/kernel/debug/block/ is created independent of the type of request queues that is created. Am I perhaps misunderstanding your feedback? Thanks, Bart.
On Wed, Feb 01, 2023 at 05:06:22PM -0800, Bart Van Assche wrote: > On 2/1/23 15:59, Luis Chamberlain wrote: > > My point was commit 85e0cbbb8a made blk_debugfs_root non-null always now > > when debugfs is enabled for both request-based block drivers and for > > make_request block drivers (multiqueue). My reading is that with your > > patch blk_debugfs_root will not be created for request-based block > > drivers. > > Hi Luis, > > The empty version of blk_mq_debugfs_init() in my patch is only selected if > CONFIG_BLK_DEBUG_FS=n. I think that my patch preserves the behavior that > /sys/kernel/debug/block/ is created independent of the type of request > queues that is created. Am I perhaps misunderstanding your feedback? Yes, my point is prior to this patch the debugfs directory is created even if you have CONFIG_BLK_DEBUG_FS=n and I've mentioned the commit which describes why, but the skinnys is blktrace. Luis
On 2/6/23 08:03, Luis Chamberlain wrote: > Yes, my point is prior to this patch the debugfs directory is created > even if you have CONFIG_BLK_DEBUG_FS=n and I've mentioned the commit > which describes why, but the skinnys is blktrace. I overlooked that the blk-mq-debugfs code is guarded by CONFIG_BLK_DEBUG_FS instead of CONFIG_DEBUG_FS. I will fix the issue that you reported. Thanks, Bart.
diff --git a/block/blk-core.c b/block/blk-core.c index ccf9a7683a3c..0dacc2df9588 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -45,6 +45,7 @@ #include <trace/events/block.h> #include "blk.h" +#include "blk-mq-debugfs.h" #include "blk-mq-sched.h" #include "blk-pm.h" #include "blk-cgroup.h" @@ -1202,7 +1203,7 @@ int __init blk_dev_init(void) blk_requestq_cachep = kmem_cache_create("request_queue", sizeof(struct request_queue), 0, SLAB_PANIC, NULL); - blk_debugfs_root = debugfs_create_dir("block", NULL); + blk_mq_debugfs_init(); return 0; } diff --git a/block/blk-mq-debugfs.c b/block/blk-mq-debugfs.c index bd942341b638..60d1de0ce624 100644 --- a/block/blk-mq-debugfs.c +++ b/block/blk-mq-debugfs.c @@ -874,3 +874,8 @@ void blk_mq_debugfs_unregister_sched_hctx(struct blk_mq_hw_ctx *hctx) debugfs_remove_recursive(hctx->sched_debugfs_dir); hctx->sched_debugfs_dir = NULL; } + +void blk_mq_debugfs_init(void) +{ + blk_debugfs_root = debugfs_create_dir("block", NULL); +} diff --git a/block/blk-mq-debugfs.h b/block/blk-mq-debugfs.h index 9c7d4b6117d4..7942119051f5 100644 --- a/block/blk-mq-debugfs.h +++ b/block/blk-mq-debugfs.h @@ -17,6 +17,8 @@ struct blk_mq_debugfs_attr { const struct seq_operations *seq_ops; }; +void blk_mq_debugfs_init(void); + int __blk_mq_debugfs_rq_show(struct seq_file *m, struct request *rq); int blk_mq_debugfs_rq_show(struct seq_file *m, void *v); @@ -36,6 +38,10 @@ void blk_mq_debugfs_unregister_sched_hctx(struct blk_mq_hw_ctx *hctx); void blk_mq_debugfs_register_rqos(struct rq_qos *rqos); void blk_mq_debugfs_unregister_rqos(struct rq_qos *rqos); #else +static inline void blk_mq_debugfs_init(void) +{ +} + static inline void blk_mq_debugfs_register(struct request_queue *q) { }
Move the code for creating the block layer debugfs root directory into blk-mq-debugfs.c. This patch prepares for adding more debugfs initialization code by introducing the function blk_mq_debugfs_init(). Cc: Christoph Hellwig <hch@lst.de> Cc: Ming Lei <ming.lei@redhat.com> Cc: Keith Busch <kbusch@kernel.org> Signed-off-by: Bart Van Assche <bvanassche@acm.org> --- block/blk-core.c | 3 ++- block/blk-mq-debugfs.c | 5 +++++ block/blk-mq-debugfs.h | 6 ++++++ 3 files changed, 13 insertions(+), 1 deletion(-)