[PATCH] hfs: export dbg_flags in debugfs

[PATCH] hfs: export dbg_flags in debugfs

[Yangtao Li]

hfs currently has some function tracking points,
which are helpful for problem analysis, but rely on
modifying the DBG_MASK macro.

Modifying the macro requires recompiling the kernel,
and the control of the log is more troublesome.

Let's export this debug facility to debugfs so that
it can be easily controlled through the node.

node:
	/sys/kernel/debug/hfs/dbg_flags

for_each_bit:

	DBG_BNODE_REFS  0x00000001
	DBG_BNODE_MOD   0x00000002
	DBG_CAT_MOD     0x00000004
	DBG_INODE       0x00000008
	DBG_SUPER       0x00000010
	DBG_EXTENT      0x00000020
	DBG_BITMAP      0x00000040

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 fs/hfs/Makefile |  4 ++--
 fs/hfs/debug.c  | 30 ++++++++++++++++++++++++++++++
 fs/hfs/hfs_fs.h | 19 ++++++++++++-------
 fs/hfs/super.c  |  8 ++++++--
 4 files changed, 50 insertions(+), 11 deletions(-)
 create mode 100644 fs/hfs/debug.c

diff --git a/fs/hfs/Makefile b/fs/hfs/Makefile
index b65459bf3dc4..a6b8091449d7 100644
--- a/fs/hfs/Makefile
+++ b/fs/hfs/Makefile
@@ -7,5 +7,5 @@ obj-$(CONFIG_HFS_FS) += hfs.o
 
 hfs-objs := bitmap.o bfind.o bnode.o brec.o btree.o \
 	    catalog.o dir.o extent.o inode.o attr.o mdb.o \
-            part_tbl.o string.o super.o sysdep.o trans.o
-
+	    part_tbl.o string.o super.o sysdep.o trans.o \
+	    debug.o
diff --git a/fs/hfs/debug.c b/fs/hfs/debug.c
new file mode 100644
index 000000000000..4e98f7a3bc74
--- /dev/null
+++ b/fs/hfs/debug.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * hfs debug support
+ *
+ * Copyright (c) 2025 Yangtao Li <frank.li@vivo.com>
+ */
+
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+
+#include "hfs_fs.h"
+
+#if IS_ENABLED(CONFIG_DEBUG_FS)
+static struct dentry *hfs_debugfs_root;
+u8 dbg_flags;
+
+void __init hfs_debug_init(void)
+{
+	hfs_debugfs_root = debugfs_create_dir("hfs", NULL);
+	debugfs_create_u8("dbg_flags", 0600, hfs_debugfs_root, &dbg_flags);
+}
+
+void hfs_debug_exit(void)
+{
+	debugfs_remove_recursive(hfs_debugfs_root);
+}
+#else
+void __init hfs_debug_init(void) {}
+void hfs_debug_exit(void) {}
+#endif
diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h
index a0c7cb0f79fc..bfcf1441e26b 100644
--- a/fs/hfs/hfs_fs.h
+++ b/fs/hfs/hfs_fs.h
@@ -27,6 +27,7 @@
 
 #include "hfs.h"
 
+#if IS_ENABLED(CONFIG_DEBUG_FS)
 #define DBG_BNODE_REFS	0x00000001
 #define DBG_BNODE_MOD	0x00000002
 #define DBG_CAT_MOD	0x00000004
@@ -35,23 +36,23 @@
 #define DBG_EXTENT	0x00000020
 #define DBG_BITMAP	0x00000040
 
-//#define DBG_MASK	(DBG_EXTENT|DBG_INODE|DBG_BNODE_MOD|DBG_CAT_MOD|DBG_BITMAP)
-//#define DBG_MASK	(DBG_BNODE_MOD|DBG_CAT_MOD|DBG_INODE)
-//#define DBG_MASK	(DBG_CAT_MOD|DBG_BNODE_REFS|DBG_INODE|DBG_EXTENT)
-#define DBG_MASK	(0)
+extern u8 dbg_flags;
 
 #define hfs_dbg(flg, fmt, ...)					\
 do {								\
-	if (DBG_##flg & DBG_MASK)				\
+	if (DBG_##flg & dbg_flags)				\
 		printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);	\
 } while (0)
 
 #define hfs_dbg_cont(flg, fmt, ...)				\
 do {								\
-	if (DBG_##flg & DBG_MASK)				\
+	if (DBG_##flg & dbg_flags)				\
 		pr_cont(fmt, ##__VA_ARGS__);			\
 } while (0)
-
+#else
+#define hfs_dbg(flg, fmt, ...) do {} while (0)
+#define hfs_dbg_cont(flg, fmt, ...) do {} while (0)
+#endif
 
 /*
  * struct hfs_inode_info
@@ -184,6 +185,10 @@ extern int hfs_cat_move(u32, struct inode *, const struct qstr *,
 			struct inode *, const struct qstr *);
 extern void hfs_cat_build_key(struct super_block *, btree_key *, u32, const struct qstr *);
 
+/* debug.c */
+extern void __init hfs_debug_init(void);
+extern void hfs_debug_exit(void);
+
 /* dir.c */
 extern const struct file_operations hfs_dir_operations;
 extern const struct inode_operations hfs_dir_inode_operations;
diff --git a/fs/hfs/super.c b/fs/hfs/super.c
index fe09c2093a93..8403f3bc89b1 100644
--- a/fs/hfs/super.c
+++ b/fs/hfs/super.c
@@ -452,16 +452,20 @@ static int __init init_hfs_fs(void)
 		SLAB_HWCACHE_ALIGN|SLAB_ACCOUNT, hfs_init_once);
 	if (!hfs_inode_cachep)
 		return -ENOMEM;
+	hfs_debug_init();
 	err = register_filesystem(&hfs_fs_type);
-	if (err)
+	if (err) {
+		hfs_debug_exit();
 		kmem_cache_destroy(hfs_inode_cachep);
-	return err;
+	}
+	return 0;
 }
 
 static void __exit exit_hfs_fs(void)
 {
 	unregister_filesystem(&hfs_fs_type);
 
+	hfs_debug_exit();
 	/*
 	 * Make sure all delayed rcu free inodes are flushed before we
 	 * destroy cache.
-- 
2.48.1

Re: [PATCH] hfs: export dbg_flags in debugfs

[kernel test robot]

Hi Yangtao,

kernel test robot noticed the following build warnings:

[auto build test WARNING on brauner-vfs/vfs.all]
[also build test WARNING on linus/master v6.15-rc5 next-20250508]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yangtao-Li/hfs-export-dbg_flags-in-debugfs/20250507-224016
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
patch link:    https://lore.kernel.org/r/20250507145550.425303-1-frank.li%40vivo.com
patch subject: [PATCH] hfs: export dbg_flags in debugfs
config: m68k-defconfig (https://download.01.org/0day-ci/archive/20250508/202505082102.YBG1Di7L-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250508/202505082102.YBG1Di7L-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202505082102.YBG1Di7L-lkp@intel.com/

All warnings (new ones prefixed by >>):

   fs/hfs/bnode.c: In function 'hfs_bnode_dump':
>> fs/hfs/bnode.c:176:29: warning: variable 'tmp' set but not used [-Wunused-but-set-variable]
     176 |                         int tmp;
         |                             ^~~
   At top level:
   cc1: note: unrecognized command-line option '-Wno-unterminated-string-initialization' may have been intended to silence earlier diagnostics


vim +/tmp +176 fs/hfs/bnode.c

^1da177e4c3f415 Linus Torvalds 2005-04-16  147  
^1da177e4c3f415 Linus Torvalds 2005-04-16  148  void hfs_bnode_dump(struct hfs_bnode *node)
^1da177e4c3f415 Linus Torvalds 2005-04-16  149  {
^1da177e4c3f415 Linus Torvalds 2005-04-16  150  	struct hfs_bnode_desc desc;
^1da177e4c3f415 Linus Torvalds 2005-04-16  151  	__be32 cnid;
^1da177e4c3f415 Linus Torvalds 2005-04-16  152  	int i, off, key_off;
^1da177e4c3f415 Linus Torvalds 2005-04-16  153  
c2b3e1f76e5c902 Joe Perches    2013-04-30  154  	hfs_dbg(BNODE_MOD, "bnode: %d\n", node->this);
^1da177e4c3f415 Linus Torvalds 2005-04-16  155  	hfs_bnode_read(node, &desc, 0, sizeof(desc));
c2b3e1f76e5c902 Joe Perches    2013-04-30  156  	hfs_dbg(BNODE_MOD, "%d, %d, %d, %d, %d\n",
^1da177e4c3f415 Linus Torvalds 2005-04-16  157  		be32_to_cpu(desc.next), be32_to_cpu(desc.prev),
^1da177e4c3f415 Linus Torvalds 2005-04-16  158  		desc.type, desc.height, be16_to_cpu(desc.num_recs));
^1da177e4c3f415 Linus Torvalds 2005-04-16  159  
^1da177e4c3f415 Linus Torvalds 2005-04-16  160  	off = node->tree->node_size - 2;
^1da177e4c3f415 Linus Torvalds 2005-04-16  161  	for (i = be16_to_cpu(desc.num_recs); i >= 0; off -= 2, i--) {
^1da177e4c3f415 Linus Torvalds 2005-04-16  162  		key_off = hfs_bnode_read_u16(node, off);
c2b3e1f76e5c902 Joe Perches    2013-04-30  163  		hfs_dbg_cont(BNODE_MOD, " %d", key_off);
^1da177e4c3f415 Linus Torvalds 2005-04-16  164  		if (i && node->type == HFS_NODE_INDEX) {
^1da177e4c3f415 Linus Torvalds 2005-04-16  165  			int tmp;
^1da177e4c3f415 Linus Torvalds 2005-04-16  166  
^1da177e4c3f415 Linus Torvalds 2005-04-16  167  			if (node->tree->attributes & HFS_TREE_VARIDXKEYS)
^1da177e4c3f415 Linus Torvalds 2005-04-16  168  				tmp = (hfs_bnode_read_u8(node, key_off) | 1) + 1;
^1da177e4c3f415 Linus Torvalds 2005-04-16  169  			else
^1da177e4c3f415 Linus Torvalds 2005-04-16  170  				tmp = node->tree->max_key_len + 1;
c2b3e1f76e5c902 Joe Perches    2013-04-30  171  			hfs_dbg_cont(BNODE_MOD, " (%d,%d",
c2b3e1f76e5c902 Joe Perches    2013-04-30  172  				     tmp, hfs_bnode_read_u8(node, key_off));
^1da177e4c3f415 Linus Torvalds 2005-04-16  173  			hfs_bnode_read(node, &cnid, key_off + tmp, 4);
c2b3e1f76e5c902 Joe Perches    2013-04-30  174  			hfs_dbg_cont(BNODE_MOD, ",%d)", be32_to_cpu(cnid));
^1da177e4c3f415 Linus Torvalds 2005-04-16  175  		} else if (i && node->type == HFS_NODE_LEAF) {
^1da177e4c3f415 Linus Torvalds 2005-04-16 @176  			int tmp;
^1da177e4c3f415 Linus Torvalds 2005-04-16  177  
^1da177e4c3f415 Linus Torvalds 2005-04-16  178  			tmp = hfs_bnode_read_u8(node, key_off);
c2b3e1f76e5c902 Joe Perches    2013-04-30  179  			hfs_dbg_cont(BNODE_MOD, " (%d)", tmp);
^1da177e4c3f415 Linus Torvalds 2005-04-16  180  		}
^1da177e4c3f415 Linus Torvalds 2005-04-16  181  	}
c2b3e1f76e5c902 Joe Perches    2013-04-30  182  	hfs_dbg_cont(BNODE_MOD, "\n");
^1da177e4c3f415 Linus Torvalds 2005-04-16  183  }
^1da177e4c3f415 Linus Torvalds 2005-04-16  184  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] hfs: export dbg_flags in debugfs

[Viacheslav Dubeyko]

Hi Yangtao,

On Wed, 2025-05-07 at 08:55 -0600, Yangtao Li wrote:
> hfs currently has some function tracking points,
> which are helpful for problem analysis, but rely on
> modifying the DBG_MASK macro.
> 
> Modifying the macro requires recompiling the kernel,
> and the control of the log is more troublesome.
> 
> Let's export this debug facility to debugfs so that
> it can be easily controlled through the node.
> 
> node:
> 	/sys/kernel/debug/hfs/dbg_flags
> 
> for_each_bit:
> 
> 	DBG_BNODE_REFS  0x00000001
> 	DBG_BNODE_MOD   0x00000002
> 	DBG_CAT_MOD     0x00000004
> 	DBG_INODE       0x00000008
> 	DBG_SUPER       0x00000010
> 	DBG_EXTENT      0x00000020
> 	DBG_BITMAP      0x00000040
> 

Frankly speaking, if we would like to rework the debugging framework in
HFS/HFS+, then I prefer to switch on pr_debug() and to use dynamic
debug framework of Linux kernel [1]. It will provide the more flexible
solution.

Thanks,
Slava.

[1] https://www.kernel.org/doc/html/v4.14/admin-guide/dynamic-debug-howto.html

回复: [PATCH] hfs: export dbg_flags in debugfs

[李扬韬]

Hi Slava,

> Frankly speaking, if we would like to rework the debugging framework in HFS/HFS+, then I prefer to switch on pr_debug() and to use dynamic debug framework of Linux kernel [1]. It will provide the more flexible solution.

I'll try it.

By the way, I plan to export disk, mem and some statistics related information to debugfs, just like f2fs does. Any suggestions?

Thanks,
.

Re: 回复: [PATCH] hfs: export dbg_flags in debugfs

[Viacheslav Dubeyko]

On Sun, 2025-05-11 at 10:45 +0000, 李扬韬 wrote:
> Hi Slava,
> 
> > Frankly speaking, if we would like to rework the debugging
> > framework in HFS/HFS+, then I prefer to switch on pr_debug() and to
> > use dynamic debug framework of Linux kernel [1]. It will provide
> > the more flexible solution.
> 
> I'll try it.

Sounds good! As far as I can see, it should be pretty simple fix. And I
think we we need to unify the code for HFS and HFS+. I am considering
to introduce HFS folder in include/linux where we can gather small
duplicated code patterns of HFS/HFS+. However, for more complex
duplicated code patterns, maybe, we need to consider of introduction an
fs/hfs_common module or shared HFS subsystem.

> 
> By the way, I plan to export disk, mem and some statistics related
> information to debugfs, just like f2fs does. Any suggestions?
> 

I think there is no troubles with it. But could you please share which
particular stats would you like to keep in debugfs? Which in-core and
on-disk objects fields will be stored into debugfs?

Thanks,
Slava.