[PATCH 0/2] Sysfs updates

[PATCH 0/2] Sysfs updates

[David Sterba]

Export the potential debugging data in the per-filesystem directories we
already have, so we can drop debugfs. The new directories depend on
CONFIG_BTRFS_DEBUG so they're not affecting normal builds.

David Sterba (2):
  btrfs: sysfs: add debugging exports
  btrfs: delete debugfs code

 fs/btrfs/sysfs.c | 68 +++++++++++++++++++++++-------------------------
 fs/btrfs/sysfs.h |  5 ----
 2 files changed, 32 insertions(+), 41 deletions(-)

-- 
2.22.0

[PATCH 1/2] btrfs: sysfs: add debugging exports

[David Sterba]

Add 'debug' directories to global sysfs and per-filesystem. This will
replace the debugfs directory. The sysfs location is simpler and builds
on top of the existing file hierarchy so there will hopefully be no more
questions about the sample debugfs file.

The directory is called 'debug' and only present under
CONFIG_BTRFS_DEBUG so this will not affect productions builds.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/sysfs.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index e6493b068294..6eef46556d75 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -247,6 +247,25 @@ static const struct attribute_group btrfs_static_feature_attr_group = {
 	.attrs = btrfs_supported_static_feature_attrs,
 };
 
+#ifdef CONFIG_BTRFS_DEBUG
+
+/*
+ * Runtime debugging exported via sysfs
+ *
+ * /sys/fs/btrfs/debug - applies to module or all filesystems
+ * /sys/fs/btrfs/UUID  - applies only to the given filesystem
+ */
+static struct attribute *btrfs_debug_feature_attrs[] = {
+	NULL
+};
+
+static const struct attribute_group btrfs_debug_feature_attr_group = {
+	.name = "debug",
+	.attrs = btrfs_debug_feature_attrs,
+};
+
+#endif
+
 static ssize_t btrfs_show_u64(u64 *value_ptr, spinlock_t *lock, char *buf)
 {
 	u64 val;
@@ -857,6 +876,13 @@ int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info)
 	if (error)
 		goto failure;
 
+#ifdef CONFIG_BTRFS_DEBUG
+	error = sysfs_create_group(fsid_kobj,
+				   &btrfs_debug_feature_attr_group);
+	if (error)
+		goto failure;
+#endif
+
 	error = addrm_unknown_feature_attrs(fs_info, true);
 	if (error)
 		goto failure;
@@ -954,6 +980,12 @@ int __init btrfs_init_sysfs(void)
 	if (ret)
 		goto out_remove_group;
 
+#ifdef CONFIG_BTRFS_DEBUG
+	ret = sysfs_create_group(&btrfs_kset->kobj, &btrfs_debug_feature_attr_group);
+	if (ret)
+		goto out2;
+#endif
+
 	return 0;
 
 out_remove_group:
-- 
2.22.0

[PATCH 2/2] btrfs: delete debugfs code

[David Sterba]

Replaced by the sysfs exports that provide a more fine grained interface
for filesystem debugging.

Signed-off-by: David Sterba <dsterba@suse.com>
---
 fs/btrfs/sysfs.c | 36 ------------------------------------
 fs/btrfs/sysfs.h |  5 -----
 2 files changed, 41 deletions(-)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 6eef46556d75..db9fa801ec2d 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -9,7 +9,6 @@
 #include <linux/completion.h>
 #include <linux/kobject.h>
 #include <linux/bug.h>
-#include <linux/debugfs.h>
 
 #include "ctree.h"
 #include "disk-io.h"
@@ -826,12 +825,6 @@ int btrfs_sysfs_add_device_link(struct btrfs_fs_devices *fs_devices,
 /* /sys/fs/btrfs/ entry */
 static struct kset *btrfs_kset;
 
-/* /sys/kernel/debug/btrfs */
-static struct dentry *btrfs_debugfs_root_dentry;
-
-/* Debugging tunables and exported data */
-u64 btrfs_debugfs_test;
-
 /*
  * Can be called by the device discovery thread.
  * And parent can be specified for seed device
@@ -937,28 +930,6 @@ void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
 	ret = sysfs_create_group(fsid_kobj, &btrfs_feature_attr_group);
 }
 
-static int btrfs_init_debugfs(void)
-{
-#ifdef CONFIG_DEBUG_FS
-	btrfs_debugfs_root_dentry = debugfs_create_dir("btrfs", NULL);
-	if (!btrfs_debugfs_root_dentry)
-		return -ENOMEM;
-
-	/*
-	 * Example code, how to export data through debugfs.
-	 *
-	 * file:        /sys/kernel/debug/btrfs/test
-	 * contents of: btrfs_debugfs_test
-	 */
-#ifdef CONFIG_BTRFS_DEBUG
-	debugfs_create_u64("test", S_IRUGO | S_IWUSR, btrfs_debugfs_root_dentry,
-			&btrfs_debugfs_test);
-#endif
-
-#endif
-	return 0;
-}
-
 int __init btrfs_init_sysfs(void)
 {
 	int ret;
@@ -967,10 +938,6 @@ int __init btrfs_init_sysfs(void)
 	if (!btrfs_kset)
 		return -ENOMEM;
 
-	ret = btrfs_init_debugfs();
-	if (ret)
-		goto out1;
-
 	init_feature_attrs();
 	ret = sysfs_create_group(&btrfs_kset->kobj, &btrfs_feature_attr_group);
 	if (ret)
@@ -991,8 +958,6 @@ int __init btrfs_init_sysfs(void)
 out_remove_group:
 	sysfs_remove_group(&btrfs_kset->kobj, &btrfs_feature_attr_group);
 out2:
-	debugfs_remove_recursive(btrfs_debugfs_root_dentry);
-out1:
 	kset_unregister(btrfs_kset);
 
 	return ret;
@@ -1004,6 +969,5 @@ void __cold btrfs_exit_sysfs(void)
 			    &btrfs_static_feature_attr_group);
 	sysfs_remove_group(&btrfs_kset->kobj, &btrfs_feature_attr_group);
 	kset_unregister(btrfs_kset);
-	debugfs_remove_recursive(btrfs_debugfs_root_dentry);
 }
 
diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
index 40716b357c1d..4bb4fe96d4bd 100644
--- a/fs/btrfs/sysfs.h
+++ b/fs/btrfs/sysfs.h
@@ -3,11 +3,6 @@
 #ifndef BTRFS_SYSFS_H
 #define BTRFS_SYSFS_H
 
-/*
- * Data exported through sysfs
- */
-extern u64 btrfs_debugfs_test;
-
 enum btrfs_feature_set {
 	FEAT_COMPAT,
 	FEAT_COMPAT_RO,
-- 
2.22.0

Re: [PATCH 0/2] Sysfs updates

[David Sterba]

On Tue, Jul 30, 2019 at 07:10:07PM +0200, David Sterba wrote:
> Export the potential debugging data in the per-filesystem directories we
> already have, so we can drop debugfs. The new directories depend on
> CONFIG_BTRFS_DEBUG so they're not affecting normal builds.
> 
> David Sterba (2):
>   btrfs: sysfs: add debugging exports
>   btrfs: delete debugfs code

Pushed to misc-next.

Re: [PATCH 0/2] Sysfs updates

[Anand Jain]

On 7/31/19 1:10 AM, David Sterba wrote:
> Export the potential debugging data in the per-filesystem directories we
> already have, so we can drop debugfs. The new directories depend on
> CONFIG_BTRFS_DEBUG so they're not affecting normal builds.
> 
> David Sterba (2):
>    btrfs: sysfs: add debugging exports
>    btrfs: delete debugfs code
> 
>   fs/btrfs/sysfs.c | 68 +++++++++++++++++++++++-------------------------
>   fs/btrfs/sysfs.h |  5 ----
>   2 files changed, 32 insertions(+), 41 deletions(-)
> 

For 2/2:
  Reviewed-by: Anand Jain <anand.jain@oracle.com>

For 1/2:
  IMO it would be better to delay this until we really have a debug hook
  exposed at the sysfs.

Thanks, Anand

Re: [PATCH 0/2] Sysfs updates

[David Sterba]

On Tue, Aug 06, 2019 at 11:17:09PM +0800, Anand Jain wrote:
> On 7/31/19 1:10 AM, David Sterba wrote:
> > Export the potential debugging data in the per-filesystem directories we
> > already have, so we can drop debugfs. The new directories depend on
> > CONFIG_BTRFS_DEBUG so they're not affecting normal builds.
> > 
> > David Sterba (2):
> >    btrfs: sysfs: add debugging exports
> >    btrfs: delete debugfs code
> > 
> >   fs/btrfs/sysfs.c | 68 +++++++++++++++++++++++-------------------------
> >   fs/btrfs/sysfs.h |  5 ----
> >   2 files changed, 32 insertions(+), 41 deletions(-)
> > 
> 
> For 2/2:
>   Reviewed-by: Anand Jain <anand.jain@oracle.com>
> 
> For 1/2:
>   IMO it would be better to delay this until we really have a debug hook
>   exposed at the sysfs.

Sorry, I don't understand what you mean.

Re: [PATCH 0/2] Sysfs updates

[Anand Jain]

On 7/8/19 12:46 AM, David Sterba wrote:
> On Tue, Aug 06, 2019 at 11:17:09PM +0800, Anand Jain wrote:
>> On 7/31/19 1:10 AM, David Sterba wrote:
>>> Export the potential debugging data in the per-filesystem directories we
>>> already have, so we can drop debugfs. The new directories depend on
>>> CONFIG_BTRFS_DEBUG so they're not affecting normal builds.
>>>
>>> David Sterba (2):
>>>     btrfs: sysfs: add debugging exports
>>>     btrfs: delete debugfs code
>>>
>>>    fs/btrfs/sysfs.c | 68 +++++++++++++++++++++++-------------------------
>>>    fs/btrfs/sysfs.h |  5 ----
>>>    2 files changed, 32 insertions(+), 41 deletions(-)
>>>
>>
>> For 2/2:
>>    Reviewed-by: Anand Jain <anand.jain@oracle.com>
>>
>> For 1/2:
>>    IMO it would be better to delay this until we really have a debug hook
>>    exposed at the sysfs.
> 
> Sorry, I don't understand what you mean.
> 

  I notice that /sysfs/fs/btrfs/<debug>|<fsid/debug> is dummy as of now,
  IMO its better to add this (1/2) patch along with the some actual trace
  which is needed. Potentially either dtrace/bfp probes will be better
  runtime debugging approach.

Thanks, Anand

Re: [PATCH 0/2] Sysfs updates

[David Sterba]

On Wed, Aug 07, 2019 at 04:29:45PM +0800, Anand Jain wrote:
> On 7/8/19 12:46 AM, David Sterba wrote:
> > On Tue, Aug 06, 2019 at 11:17:09PM +0800, Anand Jain wrote:
> >> On 7/31/19 1:10 AM, David Sterba wrote:
> >>> Export the potential debugging data in the per-filesystem directories we
> >>> already have, so we can drop debugfs. The new directories depend on
> >>> CONFIG_BTRFS_DEBUG so they're not affecting normal builds.
> >>>
> >>> David Sterba (2):
> >>>     btrfs: sysfs: add debugging exports
> >>>     btrfs: delete debugfs code
> >>>
> >>>    fs/btrfs/sysfs.c | 68 +++++++++++++++++++++++-------------------------
> >>>    fs/btrfs/sysfs.h |  5 ----
> >>>    2 files changed, 32 insertions(+), 41 deletions(-)
> >>>
> >>
> >> For 2/2:
> >>    Reviewed-by: Anand Jain <anand.jain@oracle.com>
> >>
> >> For 1/2:
> >>    IMO it would be better to delay this until we really have a debug hook
> >>    exposed at the sysfs.
> > 
> > Sorry, I don't understand what you mean.
> > 
> 
>   I notice that /sysfs/fs/btrfs/<debug>|<fsid/debug> is dummy as of now,
>   IMO its better to add this (1/2) patch along with the some actual trace
>   which is needed.

Yes it is empty for now, ready for use, like was the previous debugfs.

I don't want to delay removing the debugfs code further and not
providing a replacement does not sound as a good option, the patch would
be lost in the mailinglist.

>   Potentially either dtrace/bfp probes will be better
>   runtime debugging approach.

That's orthogonal, the sysfs is for exporting data or triggering some
action by writing to the files. The file based approach is simple to use
without external tools like bpf or even 3rd party patchsets like dtrace.