[PATCH 00/15] btrfs: sysfs, cleanups

[PATCH 00/15] btrfs: sysfs, cleanups

[Anand Jain]

Mostly cleanups patches.

Patches 1-7 are renames, code moves patches and there are no
functional changes.

Patch 8 drops unused argument in the function btrfs_sysfs_add_fsid().
Patch 9 merges two small functions which is an extension of the other.

Patches 10,11 and 13 removes unnecessary features in the functions, 
originally it was planned to provide sysfs attributes for the scanned
and unmounted devices, as in the un-merged patch in the mailing list [1]
   [1] [PATCH] btrfs: Introduce device pool sysfs attributes

Patch 12 merges functions.

Patches 14,15 are code optimize patches.

Anand Jain (15):
  btrfs: sysfs, rename device_link add,remove functions
  btrfs: sysfs, rename btrfs_sysfs_add_device()
  btrfs: sysfs, rename btrfs_device member device_dir_kobj
  btrfs: sysfs, move declared struct near its use
  btrfs: sysfs, move /sys/fs/btrfs/UUID related functions together
  btrfs: sysfs, move add remove _mounted function together
  btrfs: sysfs, delete code in a comment
  btrfs: sysfs, btrfs_sysfs_add_fsid() drop unused argument parent
  btrfs: sysfs, merge btrfs_sysfs_add devices_dir and fsid
  btrfs: volume, btrfs_free_stale_devices() cleanup unreachable code
  btrfs: sysfs, migrate fs_decvices::fsid_kobject to struct
    btrfs_fs_info
  btrfs: sysfs, unexport btrfs_sysfs_add_mounted()
  btrfs: sysfs, cleanup btrfs_sysfs_remove_fsid()
  btrfs: sysfs, merge btrfs_sysfs_remove_fsid() helper function
  btrfs: sysfs, unexport btrfs_sysfs_remove_mounted()

 fs/btrfs/ctree.h       |   2 +
 fs/btrfs/dev-replace.c |   4 +-
 fs/btrfs/disk-io.c     |  25 +---
 fs/btrfs/sysfs.c       | 258 ++++++++++++++++++-----------------------
 fs/btrfs/sysfs.h       |  12 +-
 fs/btrfs/volumes.c     |  10 +-
 fs/btrfs/volumes.h     |   3 +-
 7 files changed, 134 insertions(+), 180 deletions(-)

-- 
2.23.0

[PATCH 01/15] btrfs: sysfs, rename device_link add,remove functions

[Anand Jain]

In preparation to add btrfs_device::dev_state attribute in
  /sys/fs/btrfs/UUID/devices/

Rename btrfs_sysfs_add_device_link() and btrfs_sysfs_rm_device_link() to
btrfs_sysfs_add_device_info() and btrfs_sysfs_remove_device_info() as
these functions is going to create more attributes rather than just the
link to the disk. No functional changes.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/dev-replace.c |  4 ++--
 fs/btrfs/sysfs.c       | 10 +++++-----
 fs/btrfs/sysfs.h       |  4 ++--
 fs/btrfs/volumes.c     |  8 ++++----
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c
index f639dde2a679..8184ddad28cf 100644
--- a/fs/btrfs/dev-replace.c
+++ b/fs/btrfs/dev-replace.c
@@ -472,7 +472,7 @@ static int btrfs_dev_replace_start(struct btrfs_fs_info *fs_info,
 	atomic64_set(&dev_replace->num_uncorrectable_read_errors, 0);
 	up_write(&dev_replace->rwsem);
 
-	ret = btrfs_sysfs_add_device_link(tgt_device->fs_devices, tgt_device);
+	ret = btrfs_sysfs_add_device_info(tgt_device->fs_devices, tgt_device);
 	if (ret)
 		btrfs_err(fs_info, "kobj add dev failed %d", ret);
 
@@ -706,7 +706,7 @@ static int btrfs_dev_replace_finishing(struct btrfs_fs_info *fs_info,
 	mutex_unlock(&fs_info->fs_devices->device_list_mutex);
 
 	/* replace the sysfs entry */
-	btrfs_sysfs_rm_device_link(fs_info->fs_devices, src_device);
+	btrfs_sysfs_remove_device_info(fs_info->fs_devices, src_device);
 	btrfs_rm_dev_replace_free_srcdev(src_device);
 
 	/* write back the superblocks */
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 343d535a8081..76ee266f0b8d 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -784,7 +784,7 @@ void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
 	addrm_unknown_feature_attrs(fs_info, false);
 	sysfs_remove_group(&fs_info->fs_devices->fsid_kobj, &btrfs_feature_attr_group);
 	sysfs_remove_files(&fs_info->fs_devices->fsid_kobj, btrfs_attrs);
-	btrfs_sysfs_rm_device_link(fs_info->fs_devices, NULL);
+	btrfs_sysfs_remove_device_info(fs_info->fs_devices, NULL);
 }
 
 static const char * const btrfs_feature_set_names[FEAT_MAX] = {
@@ -973,7 +973,7 @@ int btrfs_sysfs_add_space_info_type(struct btrfs_fs_info *fs_info,
 
 /* when one_device is NULL, it removes all device links */
 
-int btrfs_sysfs_rm_device_link(struct btrfs_fs_devices *fs_devices,
+int btrfs_sysfs_remove_device_info(struct btrfs_fs_devices *fs_devices,
 		struct btrfs_device *one_device)
 {
 	struct hd_struct *disk;
@@ -1019,7 +1019,7 @@ int btrfs_sysfs_add_device(struct btrfs_fs_devices *fs_devs)
 	return 0;
 }
 
-int btrfs_sysfs_add_device_link(struct btrfs_fs_devices *fs_devices,
+int btrfs_sysfs_add_device_info(struct btrfs_fs_devices *fs_devices,
 				struct btrfs_device *one_device)
 {
 	int error = 0;
@@ -1105,13 +1105,13 @@ int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info)
 
 	btrfs_set_fs_info_ptr(fs_info);
 
-	error = btrfs_sysfs_add_device_link(fs_devs, NULL);
+	error = btrfs_sysfs_add_device_info(fs_devs, NULL);
 	if (error)
 		return error;
 
 	error = sysfs_create_files(fsid_kobj, btrfs_attrs);
 	if (error) {
-		btrfs_sysfs_rm_device_link(fs_devs, NULL);
+		btrfs_sysfs_remove_device_info(fs_devs, NULL);
 		return error;
 	}
 
diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
index e10c3adfc30f..4b624e7d97c1 100644
--- a/fs/btrfs/sysfs.h
+++ b/fs/btrfs/sysfs.h
@@ -14,9 +14,9 @@ enum btrfs_feature_set {
 
 char *btrfs_printable_features(enum btrfs_feature_set set, u64 flags);
 const char * const btrfs_feature_set_name(enum btrfs_feature_set set);
-int btrfs_sysfs_add_device_link(struct btrfs_fs_devices *fs_devices,
+int btrfs_sysfs_add_device_info(struct btrfs_fs_devices *fs_devices,
 		struct btrfs_device *one_device);
-int btrfs_sysfs_rm_device_link(struct btrfs_fs_devices *fs_devices,
+int btrfs_sysfs_remove_device_info(struct btrfs_fs_devices *fs_devices,
                 struct btrfs_device *one_device);
 int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs,
 				struct kobject *parent);
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index d8e5560db285..40d77b5846dc 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2012,7 +2012,7 @@ int btrfs_rm_device(struct btrfs_fs_info *fs_info, const char *device_path,
 	if (device->bdev) {
 		cur_devices->open_devices--;
 		/* remove sysfs entry */
-		btrfs_sysfs_rm_device_link(fs_devices, device);
+		btrfs_sysfs_remove_device_info(fs_devices, device);
 	}
 
 	num_devices = btrfs_super_num_devices(fs_info->super_copy) - 1;
@@ -2133,7 +2133,7 @@ void btrfs_destroy_dev_replace_tgtdev(struct btrfs_device *tgtdev)
 	WARN_ON(!tgtdev);
 	mutex_lock(&fs_devices->device_list_mutex);
 
-	btrfs_sysfs_rm_device_link(fs_devices, tgtdev);
+	btrfs_sysfs_remove_device_info(fs_devices, tgtdev);
 
 	if (tgtdev->bdev)
 		fs_devices->open_devices--;
@@ -2481,7 +2481,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 				    orig_super_num_devices + 1);
 
 	/* add sysfs device entry */
-	btrfs_sysfs_add_device_link(fs_devices, device);
+	btrfs_sysfs_add_device_info(fs_devices, device);
 
 	/*
 	 * we've got more storage, clear any full flags on the space
@@ -2549,7 +2549,7 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 	return ret;
 
 error_sysfs:
-	btrfs_sysfs_rm_device_link(fs_devices, device);
+	btrfs_sysfs_remove_device_info(fs_devices, device);
 	mutex_lock(&fs_info->fs_devices->device_list_mutex);
 	mutex_lock(&fs_info->chunk_mutex);
 	list_del_rcu(&device->dev_list);
-- 
2.23.0

[PATCH 02/15] btrfs: sysfs, rename btrfs_sysfs_add_device()

[Anand Jain]

btrfs_sysfs_add_device() creates the directory /sys/fs/btrfs/UUID/devices
but its function name is misleading. Rename it to
btrfs_sysfs_add_devices_dir() instead. No functional changes.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/disk-io.c | 2 +-
 fs/btrfs/sysfs.c   | 2 +-
 fs/btrfs/sysfs.h   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index e0edfdc9c82b..72191c213ac4 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3089,7 +3089,7 @@ int __cold open_ctree(struct super_block *sb,
 		goto fail_block_groups;
 	}
 
-	ret = btrfs_sysfs_add_device(fs_devices);
+	ret = btrfs_sysfs_add_devices_dir(fs_devices);
 	if (ret) {
 		btrfs_err(fs_info, "failed to init sysfs device interface: %d",
 				ret);
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 76ee266f0b8d..399b9c46790c 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -1007,7 +1007,7 @@ int btrfs_sysfs_remove_device_info(struct btrfs_fs_devices *fs_devices,
 	return 0;
 }
 
-int btrfs_sysfs_add_device(struct btrfs_fs_devices *fs_devs)
+int btrfs_sysfs_add_devices_dir(struct btrfs_fs_devices *fs_devs)
 {
 	if (!fs_devs->device_dir_kobj)
 		fs_devs->device_dir_kobj = kobject_create_and_add("devices",
diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
index 4b624e7d97c1..3dac8e163056 100644
--- a/fs/btrfs/sysfs.h
+++ b/fs/btrfs/sysfs.h
@@ -20,7 +20,7 @@ int btrfs_sysfs_remove_device_info(struct btrfs_fs_devices *fs_devices,
                 struct btrfs_device *one_device);
 int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs,
 				struct kobject *parent);
-int btrfs_sysfs_add_device(struct btrfs_fs_devices *fs_devs);
+int btrfs_sysfs_add_devices_dir(struct btrfs_fs_devices *fs_devs);
 void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs);
 void btrfs_sysfs_update_sprout_fsid(struct btrfs_fs_devices *fs_devices,
 				    const u8 *fsid);
-- 
2.23.0

[PATCH 03/15] btrfs: sysfs, rename btrfs_device member device_dir_kobj

[Anand Jain]

The struct member btrfs_device::device_dir_kobj holds the kobj of the
sysfs directory /sys/fs/btrfs/UUID/devices, so rename its holder from
device_dir_kobj to devices_dir_kobj. No functional changes.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/sysfs.c   | 28 ++++++++++++++--------------
 fs/btrfs/volumes.h |  2 +-
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 399b9c46790c..1d58187a6b33 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -744,10 +744,10 @@ static int addrm_unknown_feature_attrs(struct btrfs_fs_info *fs_info, bool add)
 
 static void __btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs)
 {
-	if (fs_devs->device_dir_kobj) {
-		kobject_del(fs_devs->device_dir_kobj);
-		kobject_put(fs_devs->device_dir_kobj);
-		fs_devs->device_dir_kobj = NULL;
+	if (fs_devs->devices_dir_kobj) {
+		kobject_del(fs_devs->devices_dir_kobj);
+		kobject_put(fs_devs->devices_dir_kobj);
+		fs_devs->devices_dir_kobj = NULL;
 	}
 
 	if (fs_devs->fsid_kobj.state_initialized) {
@@ -979,15 +979,15 @@ int btrfs_sysfs_remove_device_info(struct btrfs_fs_devices *fs_devices,
 	struct hd_struct *disk;
 	struct kobject *disk_kobj;
 
-	if (!fs_devices->device_dir_kobj)
+	if (!fs_devices->devices_dir_kobj)
 		return -EINVAL;
 
 	if (one_device && one_device->bdev) {
 		disk = one_device->bdev->bd_part;
 		disk_kobj = &part_to_dev(disk)->kobj;
 
-		sysfs_remove_link(fs_devices->device_dir_kobj,
-						disk_kobj->name);
+		sysfs_remove_link(fs_devices->devices_dir_kobj,
+				  disk_kobj->name);
 	}
 
 	if (one_device)
@@ -1000,8 +1000,8 @@ int btrfs_sysfs_remove_device_info(struct btrfs_fs_devices *fs_devices,
 		disk = one_device->bdev->bd_part;
 		disk_kobj = &part_to_dev(disk)->kobj;
 
-		sysfs_remove_link(fs_devices->device_dir_kobj,
-						disk_kobj->name);
+		sysfs_remove_link(fs_devices->devices_dir_kobj,
+				  disk_kobj->name);
 	}
 
 	return 0;
@@ -1009,11 +1009,11 @@ int btrfs_sysfs_remove_device_info(struct btrfs_fs_devices *fs_devices,
 
 int btrfs_sysfs_add_devices_dir(struct btrfs_fs_devices *fs_devs)
 {
-	if (!fs_devs->device_dir_kobj)
-		fs_devs->device_dir_kobj = kobject_create_and_add("devices",
-						&fs_devs->fsid_kobj);
+	if (!fs_devs->devices_dir_kobj)
+		fs_devs->devices_dir_kobj = kobject_create_and_add("devices",
+							&fs_devs->fsid_kobj);
 
-	if (!fs_devs->device_dir_kobj)
+	if (!fs_devs->devices_dir_kobj)
 		return -ENOMEM;
 
 	return 0;
@@ -1038,7 +1038,7 @@ int btrfs_sysfs_add_device_info(struct btrfs_fs_devices *fs_devices,
 		disk = dev->bdev->bd_part;
 		disk_kobj = &part_to_dev(disk)->kobj;
 
-		error = sysfs_create_link(fs_devices->device_dir_kobj,
+		error = sysfs_create_link(fs_devices->devices_dir_kobj,
 					  disk_kobj, disk_kobj->name);
 		if (error)
 			break;
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index fc1b564b9cfe..012e75f29fe0 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -255,7 +255,7 @@ struct btrfs_fs_devices {
 	struct btrfs_fs_info *fs_info;
 	/* sysfs kobjects */
 	struct kobject fsid_kobj;
-	struct kobject *device_dir_kobj;
+	struct kobject *devices_dir_kobj;
 	struct completion kobj_unregister;
 };
 
-- 
2.23.0

[PATCH 04/15] btrfs: sysfs, move declared struct near its use

[Anand Jain]

Move struct btrfs_feature_attr and struct raid_kobject near functions
and defines where its used. No functional change.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/sysfs.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 1d58187a6b33..d54777d5c78e 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -19,18 +19,6 @@
 #include "space-info.h"
 #include "block-group.h"
 
-struct btrfs_feature_attr {
-	struct kobj_attribute kobj_attr;
-	enum btrfs_feature_set feature_set;
-	u64 feature_bit;
-};
-
-/* For raid type sysfs entries */
-struct raid_kobject {
-	u64 flags;
-	struct kobject kobj;
-};
-
 #define __INIT_KOBJ_ATTR(_name, _mode, _show, _store)			\
 {									\
 	.attr	= { .name = __stringify(_name), .mode = _mode },	\
@@ -49,6 +37,12 @@ struct raid_kobject {
 #define BTRFS_ATTR_PTR(_prefix, _name)					\
 	(&btrfs_attr_##_prefix##_##_name.attr)
 
+struct btrfs_feature_attr {
+	struct kobj_attribute kobj_attr;
+	enum btrfs_feature_set feature_set;
+	u64 feature_bit;
+};
+
 #define BTRFS_FEAT_ATTR(_name, _feature_set, _feature_prefix, _feature_bit)  \
 static struct btrfs_feature_attr btrfs_attr_features_##_name = {	     \
 	.kobj_attr = __INIT_KOBJ_ATTR(_name, S_IRUGO,			     \
@@ -394,6 +388,12 @@ static ssize_t global_rsv_reserved_show(struct kobject *kobj,
 }
 BTRFS_ATTR(allocation, global_rsv_reserved, global_rsv_reserved_show);
 
+/* For raid type sysfs entries */
+struct raid_kobject {
+	u64 flags;
+	struct kobject kobj;
+};
+
 #define to_space_info(_kobj) container_of(_kobj, struct btrfs_space_info, kobj)
 #define to_raid_kobj(_kobj) container_of(_kobj, struct raid_kobject, kobj)
 
-- 
2.23.0

Re: [PATCH 04/15] btrfs: sysfs, move declared struct near its use

[David Sterba]

On Mon, Nov 18, 2019 at 04:46:45PM +0800, Anand Jain wrote:
> Move struct btrfs_feature_attr and struct raid_kobject near functions
> and defines where its used. No functional change.

No, please keep the definitions at the beginning of the file.

[PATCH 05/15] btrfs: sysfs, move /sys/fs/btrfs/UUID related functions together

[Anand Jain]

No functional changes. Move functions to bring btrfs_sysfs_remove_fsid()
and btrfs_sysfs_add_fsid() and its related functions together.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/sysfs.c | 60 ++++++++++++++++++++++++------------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index d54777d5c78e..370f30561001 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -742,36 +742,6 @@ static int addrm_unknown_feature_attrs(struct btrfs_fs_info *fs_info, bool add)
 	return 0;
 }
 
-static void __btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs)
-{
-	if (fs_devs->devices_dir_kobj) {
-		kobject_del(fs_devs->devices_dir_kobj);
-		kobject_put(fs_devs->devices_dir_kobj);
-		fs_devs->devices_dir_kobj = NULL;
-	}
-
-	if (fs_devs->fsid_kobj.state_initialized) {
-		kobject_del(&fs_devs->fsid_kobj);
-		kobject_put(&fs_devs->fsid_kobj);
-		wait_for_completion(&fs_devs->kobj_unregister);
-	}
-}
-
-/* when fs_devs is NULL it will remove all fsid kobject */
-void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs)
-{
-	struct list_head *fs_uuids = btrfs_get_fs_uuids();
-
-	if (fs_devs) {
-		__btrfs_sysfs_remove_fsid(fs_devs);
-		return;
-	}
-
-	list_for_each_entry(fs_devs, fs_uuids, fs_list) {
-		__btrfs_sysfs_remove_fsid(fs_devs);
-	}
-}
-
 void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
 {
 	btrfs_reset_fs_info_ptr(fs_info);
@@ -1076,6 +1046,36 @@ void btrfs_sysfs_update_sprout_fsid(struct btrfs_fs_devices *fs_devices,
 /* /sys/fs/btrfs/ entry */
 static struct kset *btrfs_kset;
 
+static void __btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs)
+{
+	if (fs_devs->devices_dir_kobj) {
+		kobject_del(fs_devs->devices_dir_kobj);
+		kobject_put(fs_devs->devices_dir_kobj);
+		fs_devs->devices_dir_kobj = NULL;
+	}
+
+	if (fs_devs->fsid_kobj.state_initialized) {
+		kobject_del(&fs_devs->fsid_kobj);
+		kobject_put(&fs_devs->fsid_kobj);
+		wait_for_completion(&fs_devs->kobj_unregister);
+	}
+}
+
+/* when fs_devs is NULL it will remove all fsid kobject */
+void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs)
+{
+	struct list_head *fs_uuids = btrfs_get_fs_uuids();
+
+	if (fs_devs) {
+		__btrfs_sysfs_remove_fsid(fs_devs);
+		return;
+	}
+
+	list_for_each_entry(fs_devs, fs_uuids, fs_list) {
+		__btrfs_sysfs_remove_fsid(fs_devs);
+	}
+}
+
 /*
  * Can be called by the device discovery thread.
  * And parent can be specified for seed device
-- 
2.23.0

Re: [PATCH 05/15] btrfs: sysfs, move /sys/fs/btrfs/UUID related functions together

[Nikolay Borisov]

On 18.11.19 г. 10:46 ч., Anand Jain wrote:
> No functional changes. Move functions to bring btrfs_sysfs_remove_fsid()
> and btrfs_sysfs_add_fsid() and its related functions together.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>


This seems like pointless code motion.

Re: [PATCH 05/15] btrfs: sysfs, move /sys/fs/btrfs/UUID related functions together

[David Sterba]

On Tue, Nov 19, 2019 at 11:24:37AM +0200, Nikolay Borisov wrote:
> 
> 
> On 18.11.19 г. 10:46 ч., Anand Jain wrote:
> > No functional changes. Move functions to bring btrfs_sysfs_remove_fsid()
> > and btrfs_sysfs_add_fsid() and its related functions together.
> > 
> > Signed-off-by: Anand Jain <anand.jain@oracle.com>
> 
> 
> This seems like pointless code motion.

Yeah, unless there's some other reason to move the code, just plain
moves are not desired.

Re: [PATCH 05/15] btrfs: sysfs, move /sys/fs/btrfs/UUID related functions together

[Anand Jain]

On 11/19/19 6:58 PM, David Sterba wrote:
> On Tue, Nov 19, 2019 at 11:24:37AM +0200, Nikolay Borisov wrote:
>>
>>
>> On 18.11.19 г. 10:46 ч., Anand Jain wrote:
>>> No functional changes. Move functions to bring btrfs_sysfs_remove_fsid()
>>> and btrfs_sysfs_add_fsid() and its related functions together.
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>
>>
>> This seems like pointless code motion.
> 
> Yeah, unless there's some other reason to move the code, just plain
> moves are not desired.
> 

  The reason was - btrfs_sysfs_add_fsid() and btrfs_sysfs_remove_fsid()
  are related. Easy to read and verify to have placed them one below
  other.

  Ok not a big deal. I am ok either ways.

Re: [PATCH 05/15] btrfs: sysfs, move /sys/fs/btrfs/UUID related functions together

[David Sterba]

On Wed, Nov 20, 2019 at 01:56:04PM +0800, Anand Jain wrote:
> On 11/19/19 6:58 PM, David Sterba wrote:
> > On Tue, Nov 19, 2019 at 11:24:37AM +0200, Nikolay Borisov wrote:
> >> On 18.11.19 г. 10:46 ч., Anand Jain wrote:
> >>> No functional changes. Move functions to bring btrfs_sysfs_remove_fsid()
> >>> and btrfs_sysfs_add_fsid() and its related functions together.
> >>>
> >>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> >> This seems like pointless code motion.
> > 
> > Yeah, unless there's some other reason to move the code, just plain
> > moves are not desired.
> 
>   The reason was - btrfs_sysfs_add_fsid() and btrfs_sysfs_remove_fsid()
>   are related. Easy to read and verify to have placed them one below
>   other.

I see that add and remove functions are grouped, so this would move
someting else away:

btrfs_sysfs_remove_fsid + __btrfs_sysfs_remove_fsid

btrfs_sysfs_add_fsid + btrfs_sysfs_add_mounted

and device related functions are also grouped by the action type, so we
can keep it like that.

Re: [PATCH 05/15] btrfs: sysfs, move /sys/fs/btrfs/UUID related functions together

[Anand Jain]

On 11/23/19 1:48 AM, David Sterba wrote:
> On Wed, Nov 20, 2019 at 01:56:04PM +0800, Anand Jain wrote:
>> On 11/19/19 6:58 PM, David Sterba wrote:
>>> On Tue, Nov 19, 2019 at 11:24:37AM +0200, Nikolay Borisov wrote:
>>>> On 18.11.19 г. 10:46 ч., Anand Jain wrote:
>>>>> No functional changes. Move functions to bring btrfs_sysfs_remove_fsid()
>>>>> and btrfs_sysfs_add_fsid() and its related functions together.
>>>>>
>>>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>>> This seems like pointless code motion.
>>>
>>> Yeah, unless there's some other reason to move the code, just plain
>>> moves are not desired.
>>
>>    The reason was - btrfs_sysfs_add_fsid() and btrfs_sysfs_remove_fsid()
>>    are related. Easy to read and verify to have placed them one below
>>    other.
> 
> I see that add and remove functions are grouped, so this would move
> someting else away:
> 
> btrfs_sysfs_remove_fsid + __btrfs_sysfs_remove_fsid
> 
> btrfs_sysfs_add_fsid + btrfs_sysfs_add_mounted

Ok.

> and device related functions are also grouped by the action type, so we
> can keep it like that.

Ok. Grouped by action type.

Thanks, Anand

[PATCH 06/15] btrfs: sysfs, move add remove _mounted function together

[Anand Jain]

The functions btrfs_sysfs_add_mounted() and btrfs_sysfs_remove_mounted()
which add and remove files and directory under /sys/fs/btrfs/UUID keep
them together to improve readability. No functional changes.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/sysfs.c | 132 +++++++++++++++++++++++------------------------
 1 file changed, 66 insertions(+), 66 deletions(-)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 370f30561001..724af1322dce 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -742,21 +742,6 @@ static int addrm_unknown_feature_attrs(struct btrfs_fs_info *fs_info, bool add)
 	return 0;
 }
 
-void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
-{
-	btrfs_reset_fs_info_ptr(fs_info);
-
-	if (fs_info->space_info_kobj) {
-		sysfs_remove_files(fs_info->space_info_kobj, allocation_attrs);
-		kobject_del(fs_info->space_info_kobj);
-		kobject_put(fs_info->space_info_kobj);
-	}
-	addrm_unknown_feature_attrs(fs_info, false);
-	sysfs_remove_group(&fs_info->fs_devices->fsid_kobj, &btrfs_feature_attr_group);
-	sysfs_remove_files(&fs_info->fs_devices->fsid_kobj, btrfs_attrs);
-	btrfs_sysfs_remove_device_info(fs_info->fs_devices, NULL);
-}
-
 static const char * const btrfs_feature_set_names[FEAT_MAX] = {
 	[FEAT_COMPAT]	 = "compat",
 	[FEAT_COMPAT_RO] = "compat_ro",
@@ -1028,6 +1013,72 @@ void btrfs_kobject_uevent(struct block_device *bdev, enum kobject_action action)
 			&disk_to_dev(bdev->bd_disk)->kobj);
 }
 
+void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
+{
+	btrfs_reset_fs_info_ptr(fs_info);
+
+	if (fs_info->space_info_kobj) {
+		sysfs_remove_files(fs_info->space_info_kobj, allocation_attrs);
+		kobject_del(fs_info->space_info_kobj);
+		kobject_put(fs_info->space_info_kobj);
+	}
+	addrm_unknown_feature_attrs(fs_info, false);
+	sysfs_remove_group(&fs_info->fs_devices->fsid_kobj, &btrfs_feature_attr_group);
+	sysfs_remove_files(&fs_info->fs_devices->fsid_kobj, btrfs_attrs);
+	btrfs_sysfs_remove_device_info(fs_info->fs_devices, NULL);
+}
+
+int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info)
+{
+	int error;
+	struct btrfs_fs_devices *fs_devs = fs_info->fs_devices;
+	struct kobject *fsid_kobj = &fs_devs->fsid_kobj;
+
+	btrfs_set_fs_info_ptr(fs_info);
+
+	error = btrfs_sysfs_add_device_info(fs_devs, NULL);
+	if (error)
+		return error;
+
+	error = sysfs_create_files(fsid_kobj, btrfs_attrs);
+	if (error) {
+		btrfs_sysfs_remove_device_info(fs_devs, NULL);
+		return error;
+	}
+
+	error = sysfs_create_group(fsid_kobj,
+				   &btrfs_feature_attr_group);
+	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;
+
+	fs_info->space_info_kobj = kobject_create_and_add("allocation",
+						  fsid_kobj);
+	if (!fs_info->space_info_kobj) {
+		error = -ENOMEM;
+		goto failure;
+	}
+
+	error = sysfs_create_files(fs_info->space_info_kobj, allocation_attrs);
+	if (error)
+		goto failure;
+
+	return 0;
+failure:
+	btrfs_sysfs_remove_mounted(fs_info);
+	return error;
+}
+
 void btrfs_sysfs_update_sprout_fsid(struct btrfs_fs_devices *fs_devices,
 				    const u8 *fsid)
 {
@@ -1097,57 +1148,6 @@ int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs,
 	return 0;
 }
 
-int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info)
-{
-	int error;
-	struct btrfs_fs_devices *fs_devs = fs_info->fs_devices;
-	struct kobject *fsid_kobj = &fs_devs->fsid_kobj;
-
-	btrfs_set_fs_info_ptr(fs_info);
-
-	error = btrfs_sysfs_add_device_info(fs_devs, NULL);
-	if (error)
-		return error;
-
-	error = sysfs_create_files(fsid_kobj, btrfs_attrs);
-	if (error) {
-		btrfs_sysfs_remove_device_info(fs_devs, NULL);
-		return error;
-	}
-
-	error = sysfs_create_group(fsid_kobj,
-				   &btrfs_feature_attr_group);
-	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;
-
-	fs_info->space_info_kobj = kobject_create_and_add("allocation",
-						  fsid_kobj);
-	if (!fs_info->space_info_kobj) {
-		error = -ENOMEM;
-		goto failure;
-	}
-
-	error = sysfs_create_files(fs_info->space_info_kobj, allocation_attrs);
-	if (error)
-		goto failure;
-
-	return 0;
-failure:
-	btrfs_sysfs_remove_mounted(fs_info);
-	return error;
-}
-
 
 /*
  * Change per-fs features in /sys/fs/btrfs/UUID/features to match current
-- 
2.23.0

[PATCH 07/15] btrfs: sysfs, delete code in a comment

[Anand Jain]

commit 3ebb2ada (btrfs: sysfs: export supported checksums), added some unwanted
commented-code, delete it.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/sysfs.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 724af1322dce..9494ccace624 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -255,16 +255,6 @@ BTRFS_FEAT_ATTR_INCOMPAT(metadata_uuid, METADATA_UUID);
 BTRFS_FEAT_ATTR_COMPAT_RO(free_space_tree, FREE_SPACE_TREE);
 BTRFS_FEAT_ATTR_INCOMPAT(raid1c34, RAID1C34);
 
-/*
-static struct btrfs_feature_attr btrfs_attr_features_checksums_name = {
-	.kobj_attr = __INIT_KOBJ_ATTR(supported_checksums, S_IRUGO,
-				      btrfs_supported_checksums_show,
-				      NULL),
-	.feature_set	= FEAT_INCOMPAT,
-	.feature_bit	= 0,
-};
-*/
-
 static struct attribute *btrfs_supported_feature_attrs[] = {
 	BTRFS_FEAT_ATTR_PTR(mixed_backref),
 	BTRFS_FEAT_ATTR_PTR(default_subvol),
-- 
2.23.0

Re: [PATCH 07/15] btrfs: sysfs, delete code in a comment

[David Sterba]

On Mon, Nov 18, 2019 at 04:46:48PM +0800, Anand Jain wrote:
> commit 3ebb2ada (btrfs: sysfs: export supported checksums), added some unwanted
> commented-code, delete it.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  fs/btrfs/sysfs.c | 10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> index 724af1322dce..9494ccace624 100644
> --- a/fs/btrfs/sysfs.c
> +++ b/fs/btrfs/sysfs.c
> @@ -255,16 +255,6 @@ BTRFS_FEAT_ATTR_INCOMPAT(metadata_uuid, METADATA_UUID);
>  BTRFS_FEAT_ATTR_COMPAT_RO(free_space_tree, FREE_SPACE_TREE);
>  BTRFS_FEAT_ATTR_INCOMPAT(raid1c34, RAID1C34);
>  
> -/*
> -static struct btrfs_feature_attr btrfs_attr_features_checksums_name = {
> -	.kobj_attr = __INIT_KOBJ_ATTR(supported_checksums, S_IRUGO,
> -				      btrfs_supported_checksums_show,
> -				      NULL),
> -	.feature_set	= FEAT_INCOMPAT,
> -	.feature_bit	= 0,
> -};
> -*/

Uh, I'll fold that to the patch that introduced it, thanks for spotting
it.

[PATCH 08/15] btrfs: sysfs, btrfs_sysfs_add_fsid() drop unused argument parent

[Anand Jain]

Commit 24bd69cb (Btrfs: sysfs: add support to add parent for fsid)
added parent argument in preparation to show the seed fsid under the
sprout fsid as in the patch [1] in the mailing list.

 [1] Btrfs: sysfs: support seed devices in the sysfs layout

But later this idea was superseded by another idea to rename the fsid
as in the commit f93c39970b1d (btrfs: factor out sysfs code for updating
sprout fsid).

So we don't need parent argument anymore.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/disk-io.c |  2 +-
 fs/btrfs/sysfs.c   | 11 +++++------
 fs/btrfs/sysfs.h   |  3 +--
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 72191c213ac4..3f6a26d9afcd 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3082,7 +3082,7 @@ int __cold open_ctree(struct super_block *sb,
 
 	btrfs_free_extra_devids(fs_devices, 1);
 
-	ret = btrfs_sysfs_add_fsid(fs_devices, NULL);
+	ret = btrfs_sysfs_add_fsid(fs_devices);
 	if (ret) {
 		btrfs_err(fs_info, "failed to init sysfs fsid interface: %d",
 				ret);
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 9494ccace624..42aeb2375f2a 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -1118,18 +1118,17 @@ void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs)
 }
 
 /*
- * Can be called by the device discovery thread.
- * And parent can be specified for seed device
+ * Creates:
+ * 	/sys/fs/btrfs/UUID
  */
-int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs,
-				struct kobject *parent)
+int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs)
 {
 	int error;
 
 	init_completion(&fs_devs->kobj_unregister);
 	fs_devs->fsid_kobj.kset = btrfs_kset;
-	error = kobject_init_and_add(&fs_devs->fsid_kobj,
-				&btrfs_ktype, parent, "%pU", fs_devs->fsid);
+	error = kobject_init_and_add(&fs_devs->fsid_kobj, &btrfs_ktype, NULL,
+				     "%pU", fs_devs->fsid);
 	if (error) {
 		kobject_put(&fs_devs->fsid_kobj);
 		return error;
diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
index 3dac8e163056..6addc35deb67 100644
--- a/fs/btrfs/sysfs.h
+++ b/fs/btrfs/sysfs.h
@@ -18,8 +18,7 @@ int btrfs_sysfs_add_device_info(struct btrfs_fs_devices *fs_devices,
 		struct btrfs_device *one_device);
 int btrfs_sysfs_remove_device_info(struct btrfs_fs_devices *fs_devices,
                 struct btrfs_device *one_device);
-int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs,
-				struct kobject *parent);
+int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs);
 int btrfs_sysfs_add_devices_dir(struct btrfs_fs_devices *fs_devs);
 void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs);
 void btrfs_sysfs_update_sprout_fsid(struct btrfs_fs_devices *fs_devices,
-- 
2.23.0

[PATCH 09/15] btrfs: sysfs, merge btrfs_sysfs_add device_dir and fsid

[Anand Jain]

Merge btrfs_sysfs_add_fsid() and btrfs_sysfs_add_devices_dir() functions
these two are small and they are called one after the other.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/disk-io.c |  7 -------
 fs/btrfs/sysfs.c   | 22 +++++++++-------------
 fs/btrfs/sysfs.h   |  1 -
 3 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 3f6a26d9afcd..ab888d89d844 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3089,13 +3089,6 @@ int __cold open_ctree(struct super_block *sb,
 		goto fail_block_groups;
 	}
 
-	ret = btrfs_sysfs_add_devices_dir(fs_devices);
-	if (ret) {
-		btrfs_err(fs_info, "failed to init sysfs device interface: %d",
-				ret);
-		goto fail_fsdev_sysfs;
-	}
-
 	ret = btrfs_sysfs_add_mounted(fs_info);
 	if (ret) {
 		btrfs_err(fs_info, "failed to init sysfs interface: %d", ret);
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 42aeb2375f2a..b42245104bc9 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -952,18 +952,6 @@ int btrfs_sysfs_remove_device_info(struct btrfs_fs_devices *fs_devices,
 	return 0;
 }
 
-int btrfs_sysfs_add_devices_dir(struct btrfs_fs_devices *fs_devs)
-{
-	if (!fs_devs->devices_dir_kobj)
-		fs_devs->devices_dir_kobj = kobject_create_and_add("devices",
-							&fs_devs->fsid_kobj);
-
-	if (!fs_devs->devices_dir_kobj)
-		return -ENOMEM;
-
-	return 0;
-}
-
 int btrfs_sysfs_add_device_info(struct btrfs_fs_devices *fs_devices,
 				struct btrfs_device *one_device)
 {
@@ -1134,10 +1122,18 @@ int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs)
 		return error;
 	}
 
+	fs_devs->devices_dir_kobj = kobject_create_and_add("devices",
+							&fs_devs->fsid_kobj);
+	if (!fs_devs->devices_dir_kobj) {
+		btrfs_err(fs_devs->fs_info,
+			  "failed to init sysfs device interface");
+		kobject_put(&fs_devs->fsid_kobj);
+		return -ENOMEM;
+	}
+
 	return 0;
 }
 
-
 /*
  * Change per-fs features in /sys/fs/btrfs/UUID/features to match current
  * values in superblock. Call after any changes to incompat/compat_ro flags
diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
index 6addc35deb67..e7ae91f68e36 100644
--- a/fs/btrfs/sysfs.h
+++ b/fs/btrfs/sysfs.h
@@ -19,7 +19,6 @@ int btrfs_sysfs_add_device_info(struct btrfs_fs_devices *fs_devices,
 int btrfs_sysfs_remove_device_info(struct btrfs_fs_devices *fs_devices,
                 struct btrfs_device *one_device);
 int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs);
-int btrfs_sysfs_add_devices_dir(struct btrfs_fs_devices *fs_devs);
 void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs);
 void btrfs_sysfs_update_sprout_fsid(struct btrfs_fs_devices *fs_devices,
 				    const u8 *fsid);
-- 
2.23.0

[PATCH 10/15] btrfs: volume, btrfs_free_stale_devices() cleanup unreachable code

[Anand Jain]

fs_devices::num_devices can be zero only in unmounted context, and we
don't have any fsid specific kobjects in the unmoutend context. So no
need to call btrfs_sysfs_remove_fsid() in btrfs_free_stale_devices().

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
We implemented this in preparation to provide the fsid/device attributes
for the devices in unmoutned context as was proposed here [1].

[1] [PATCH] btrfs: Introduce device pool sysfs attributes

 fs/btrfs/volumes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 40d77b5846dc..fe135b5a8a03 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -585,7 +585,7 @@ static int btrfs_free_stale_devices(const char *path,
 		mutex_unlock(&fs_devices->device_list_mutex);
 
 		if (fs_devices->num_devices == 0) {
-			btrfs_sysfs_remove_fsid(fs_devices);
+			/* If its here fsid is unmounted */
 			list_del(&fs_devices->fs_list);
 			free_fs_devices(fs_devices);
 		}
-- 
2.23.0

[PATCH 11/15] btrfs: sysfs, migrate fs_decvices::fsid_kobject to struct btrfs_fs_info

[Anand Jain]

In an idea that at some point we would have to create sysfs kobjects to
show the scanned devices, so we tried to maintain the fsid_kobject in
the struct btrfs_fs_devices instead of in the struct btrfs_fs_info.
Its been without it for a long time and if it has to be done at some point
it can be done using ioctl as well. For now cleanup sysfs and migrate the
top most kobject in the mounted context to btrfs_fs_info.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
PS: There are patches in the ML [1], which shows scanned devices
in the sysfs, if at all need there is a choice of doing the same using
ioctl as well. So that we dont' have to stress already overloaded sysfs.
  [1] [PATCH] btrfs: Introduce device pool sysfs attributes

 fs/btrfs/ctree.h   |  2 +
 fs/btrfs/disk-io.c |  8 ++--
 fs/btrfs/sysfs.c   | 94 ++++++++++++++++++++++------------------------
 fs/btrfs/sysfs.h   |  4 +-
 fs/btrfs/volumes.h |  3 +-
 5 files changed, 54 insertions(+), 57 deletions(-)

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index b2e8fd8a8e59..94446ed7a872 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -739,6 +739,8 @@ struct btrfs_fs_info {
 	struct task_struct *cleaner_kthread;
 	u32 thread_pool_size;
 
+	/* sysfs kobjects */
+	struct kobject fsid_kobj;
 	struct kobject *space_info_kobj;
 
 	u64 total_pinned;
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index ab888d89d844..85ec0a3d2019 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3082,10 +3082,10 @@ int __cold open_ctree(struct super_block *sb,
 
 	btrfs_free_extra_devids(fs_devices, 1);
 
-	ret = btrfs_sysfs_add_fsid(fs_devices);
+	ret = btrfs_sysfs_add_fsid(fs_info);
 	if (ret) {
 		btrfs_err(fs_info, "failed to init sysfs fsid interface: %d",
-				ret);
+			  ret);
 		goto fail_block_groups;
 	}
 
@@ -3308,7 +3308,7 @@ int __cold open_ctree(struct super_block *sb,
 	btrfs_sysfs_remove_mounted(fs_info);
 
 fail_fsdev_sysfs:
-	btrfs_sysfs_remove_fsid(fs_info->fs_devices);
+	btrfs_sysfs_remove_fsid(fs_info);
 
 fail_block_groups:
 	btrfs_put_block_group_cache(fs_info);
@@ -4006,7 +4006,7 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
 			   percpu_counter_sum(&fs_info->dio_bytes));
 
 	btrfs_sysfs_remove_mounted(fs_info);
-	btrfs_sysfs_remove_fsid(fs_info->fs_devices);
+	btrfs_sysfs_remove_fsid(fs_info);
 
 	btrfs_free_fs_roots(fs_info);
 
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index b42245104bc9..806676ef008a 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -62,7 +62,6 @@ static struct btrfs_feature_attr btrfs_attr_features_##_name = {	     \
 	BTRFS_FEAT_ATTR(name, FEAT_INCOMPAT, BTRFS_FEATURE_INCOMPAT, feature)
 
 static inline struct btrfs_fs_info *to_fs_info(struct kobject *kobj);
-static inline struct btrfs_fs_devices *to_fs_devs(struct kobject *kobj);
 
 static struct btrfs_feature_attr *to_btrfs_feature_attr(struct kobj_attribute *a)
 {
@@ -655,10 +654,10 @@ static const struct attribute *btrfs_attrs[] = {
 
 static void btrfs_release_fsid_kobj(struct kobject *kobj)
 {
-	struct btrfs_fs_devices *fs_devs = to_fs_devs(kobj);
+	struct btrfs_fs_info *fs_info = to_fs_info(kobj);
 
-	memset(&fs_devs->fsid_kobj, 0, sizeof(struct kobject));
-	complete(&fs_devs->kobj_unregister);
+	memset(&fs_info->fsid_kobj, 0, sizeof(struct kobject));
+	complete(&fs_info->fs_devices->kobj_unregister);
 }
 
 static struct kobj_type btrfs_ktype = {
@@ -666,18 +665,11 @@ static struct kobj_type btrfs_ktype = {
 	.release	= btrfs_release_fsid_kobj,
 };
 
-static inline struct btrfs_fs_devices *to_fs_devs(struct kobject *kobj)
-{
-	if (kobj->ktype != &btrfs_ktype)
-		return NULL;
-	return container_of(kobj, struct btrfs_fs_devices, fsid_kobj);
-}
-
 static inline struct btrfs_fs_info *to_fs_info(struct kobject *kobj)
 {
 	if (kobj->ktype != &btrfs_ktype)
 		return NULL;
-	return to_fs_devs(kobj)->fs_info;
+	return container_of(kobj, struct btrfs_fs_info, fsid_kobj);
 }
 
 #define NUM_FEATURE_BITS 64
@@ -719,12 +711,12 @@ static int addrm_unknown_feature_attrs(struct btrfs_fs_info *fs_info, bool add)
 			attrs[0] = &fa->kobj_attr.attr;
 			if (add) {
 				int ret;
-				ret = sysfs_merge_group(&fs_info->fs_devices->fsid_kobj,
+				ret = sysfs_merge_group(&fs_info->fsid_kobj,
 							&agroup);
 				if (ret)
 					return ret;
 			} else
-				sysfs_unmerge_group(&fs_info->fs_devices->fsid_kobj,
+				sysfs_unmerge_group(&fs_info->fsid_kobj,
 						    &agroup);
 		}
 
@@ -1001,8 +993,8 @@ void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
 		kobject_put(fs_info->space_info_kobj);
 	}
 	addrm_unknown_feature_attrs(fs_info, false);
-	sysfs_remove_group(&fs_info->fs_devices->fsid_kobj, &btrfs_feature_attr_group);
-	sysfs_remove_files(&fs_info->fs_devices->fsid_kobj, btrfs_attrs);
+	sysfs_remove_group(&fs_info->fsid_kobj, &btrfs_feature_attr_group);
+	sysfs_remove_files(&fs_info->fsid_kobj, btrfs_attrs);
 	btrfs_sysfs_remove_device_info(fs_info->fs_devices, NULL);
 }
 
@@ -1010,7 +1002,7 @@ int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info)
 {
 	int error;
 	struct btrfs_fs_devices *fs_devs = fs_info->fs_devices;
-	struct kobject *fsid_kobj = &fs_devs->fsid_kobj;
+	struct kobject *fsid_kobj = &fs_info->fsid_kobj;
 
 	btrfs_set_fs_info_ptr(fs_info);
 
@@ -1067,41 +1059,47 @@ void btrfs_sysfs_update_sprout_fsid(struct btrfs_fs_devices *fs_devices,
 	 * directory
 	 */
 	snprintf(fsid_buf, BTRFS_UUID_UNPARSED_SIZE, "%pU", fsid);
-	if (kobject_rename(&fs_devices->fsid_kobj, fsid_buf))
+	if (kobject_rename(&fs_devices->fs_info->fsid_kobj, fsid_buf))
 		btrfs_warn(fs_devices->fs_info,
-				"sysfs: failed to create fsid for sprout");
+			   "sysfs: failed to create fsid for sprout");
 }
 
 /* /sys/fs/btrfs/ entry */
 static struct kset *btrfs_kset;
 
-static void __btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs)
+/*
+ * Remove /sys/fs/btrfs/UUID/devices and /sysfs/btrfs/UUID
+ */
+static void __btrfs_sysfs_remove_fsid(struct btrfs_fs_info *fs_info)
 {
-	if (fs_devs->devices_dir_kobj) {
-		kobject_del(fs_devs->devices_dir_kobj);
-		kobject_put(fs_devs->devices_dir_kobj);
-		fs_devs->devices_dir_kobj = NULL;
+	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
+
+	if (fs_devices->devices_dir_kobj) {
+		kobject_del(fs_devices->devices_dir_kobj);
+		kobject_put(fs_devices->devices_dir_kobj);
+		fs_devices->devices_dir_kobj = NULL;
 	}
 
-	if (fs_devs->fsid_kobj.state_initialized) {
-		kobject_del(&fs_devs->fsid_kobj);
-		kobject_put(&fs_devs->fsid_kobj);
-		wait_for_completion(&fs_devs->kobj_unregister);
+	if (fs_info->fsid_kobj.state_initialized) {
+		kobject_del(&fs_info->fsid_kobj);
+		kobject_put(&fs_info->fsid_kobj);
+		wait_for_completion(&fs_devices->kobj_unregister);
 	}
 }
 
-/* when fs_devs is NULL it will remove all fsid kobject */
-void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs)
+/* when fs_info is NULL it will remove all fs_info::fsid kobject */
+void btrfs_sysfs_remove_fsid(struct btrfs_fs_info *fs_info)
 {
 	struct list_head *fs_uuids = btrfs_get_fs_uuids();
+	struct btrfs_fs_devices *fs_devices;
 
-	if (fs_devs) {
-		__btrfs_sysfs_remove_fsid(fs_devs);
+	if (fs_info) {
+		__btrfs_sysfs_remove_fsid(fs_info);
 		return;
 	}
 
-	list_for_each_entry(fs_devs, fs_uuids, fs_list) {
-		__btrfs_sysfs_remove_fsid(fs_devs);
+	list_for_each_entry(fs_devices, fs_uuids, fs_list) {
+		__btrfs_sysfs_remove_fsid(fs_devices->fs_info);
 	}
 }
 
@@ -1109,25 +1107,25 @@ void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs)
  * Creates:
  * 	/sys/fs/btrfs/UUID
  */
-int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs)
+int btrfs_sysfs_add_fsid(struct btrfs_fs_info *fs_info)
 {
 	int error;
+	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
 
-	init_completion(&fs_devs->kobj_unregister);
-	fs_devs->fsid_kobj.kset = btrfs_kset;
-	error = kobject_init_and_add(&fs_devs->fsid_kobj, &btrfs_ktype, NULL,
-				     "%pU", fs_devs->fsid);
+	init_completion(&fs_devices->kobj_unregister);
+	fs_info->fsid_kobj.kset = btrfs_kset;
+	error = kobject_init_and_add(&fs_info->fsid_kobj, &btrfs_ktype, NULL,
+				     "%pU", fs_devices->fsid);
 	if (error) {
-		kobject_put(&fs_devs->fsid_kobj);
+		kobject_put(&fs_info->fsid_kobj);
 		return error;
 	}
 
-	fs_devs->devices_dir_kobj = kobject_create_and_add("devices",
-							&fs_devs->fsid_kobj);
-	if (!fs_devs->devices_dir_kobj) {
-		btrfs_err(fs_devs->fs_info,
-			  "failed to init sysfs device interface");
-		kobject_put(&fs_devs->fsid_kobj);
+	fs_devices->devices_dir_kobj = kobject_create_and_add("devices",
+							&fs_info->fsid_kobj);
+	if (!fs_devices->devices_dir_kobj) {
+		btrfs_err(fs_info, "failed to init sysfs device interface");
+		kobject_put(&fs_info->fsid_kobj);
 		return -ENOMEM;
 	}
 
@@ -1141,7 +1139,6 @@ int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs)
 void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
 		u64 bit, enum btrfs_feature_set set)
 {
-	struct btrfs_fs_devices *fs_devs;
 	struct kobject *fsid_kobj;
 	u64 features;
 	int ret;
@@ -1152,8 +1149,7 @@ void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
 	features = get_features(fs_info, set);
 	ASSERT(bit & supported_feature_masks[set]);
 
-	fs_devs = fs_info->fs_devices;
-	fsid_kobj = &fs_devs->fsid_kobj;
+	fsid_kobj = &fs_info->fsid_kobj;
 
 	if (!fsid_kobj->state_initialized)
 		return;
diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
index e7ae91f68e36..0648afbb40ca 100644
--- a/fs/btrfs/sysfs.h
+++ b/fs/btrfs/sysfs.h
@@ -18,8 +18,8 @@ int btrfs_sysfs_add_device_info(struct btrfs_fs_devices *fs_devices,
 		struct btrfs_device *one_device);
 int btrfs_sysfs_remove_device_info(struct btrfs_fs_devices *fs_devices,
                 struct btrfs_device *one_device);
-int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs);
-void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs);
+int btrfs_sysfs_add_fsid(struct btrfs_fs_info *fs_info);
+void btrfs_sysfs_remove_fsid(struct btrfs_fs_info *fs_info);
 void btrfs_sysfs_update_sprout_fsid(struct btrfs_fs_devices *fs_devices,
 				    const u8 *fsid);
 void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index 012e75f29fe0..3fd7f2b348b0 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -254,9 +254,8 @@ struct btrfs_fs_devices {
 
 	struct btrfs_fs_info *fs_info;
 	/* sysfs kobjects */
-	struct kobject fsid_kobj;
-	struct kobject *devices_dir_kobj;
 	struct completion kobj_unregister;
+	struct kobject *devices_dir_kobj;
 };
 
 #define BTRFS_BIO_INLINE_CSUM_SIZE	64
-- 
2.23.0

[PATCH 12/15] btrfs: sysfs, unexport btrfs_sysfs_add_mounted()

[Anand Jain]

In open_ctree() we call btrfs_sysfs_add_fsid() to create
/sys/fs/btrfs/UUID kobject, and in the following line of code in
open_ctree() we call btrfs_sysfs_add_mounted() to create its attributes.
And there is no other users of btrfs_sysfs_add_mounted(). So let
btrfs_sysfs_add_fsid() also create its attributes and make
btrfs_sysfs_add_mounted() a local static function.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/disk-io.c | 8 --------
 fs/btrfs/sysfs.c   | 9 ++++++++-
 fs/btrfs/sysfs.h   | 1 -
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 85ec0a3d2019..8f6a08bef490 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3089,12 +3089,6 @@ int __cold open_ctree(struct super_block *sb,
 		goto fail_block_groups;
 	}
 
-	ret = btrfs_sysfs_add_mounted(fs_info);
-	if (ret) {
-		btrfs_err(fs_info, "failed to init sysfs interface: %d", ret);
-		goto fail_fsdev_sysfs;
-	}
-
 	ret = btrfs_init_space_info(fs_info);
 	if (ret) {
 		btrfs_err(fs_info, "failed to initialize space info: %d", ret);
@@ -3306,8 +3300,6 @@ int __cold open_ctree(struct super_block *sb,
 
 fail_sysfs:
 	btrfs_sysfs_remove_mounted(fs_info);
-
-fail_fsdev_sysfs:
 	btrfs_sysfs_remove_fsid(fs_info);
 
 fail_block_groups:
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 806676ef008a..2ea27c2d9ef1 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -998,7 +998,7 @@ void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
 	btrfs_sysfs_remove_device_info(fs_info->fs_devices, NULL);
 }
 
-int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info)
+static int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info)
 {
 	int error;
 	struct btrfs_fs_devices *fs_devs = fs_info->fs_devices;
@@ -1129,6 +1129,13 @@ int btrfs_sysfs_add_fsid(struct btrfs_fs_info *fs_info)
 		return -ENOMEM;
 	}
 
+	error = btrfs_sysfs_add_mounted(fs_info);
+	if (error) {
+		btrfs_err(fs_info, "failed to init sysfs interface: %d", error);
+		btrfs_sysfs_remove_fsid(fs_info);
+		return error;
+	}
+
 	return 0;
 }
 
diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
index 0648afbb40ca..a977fe3bec64 100644
--- a/fs/btrfs/sysfs.h
+++ b/fs/btrfs/sysfs.h
@@ -28,7 +28,6 @@ void btrfs_kobject_uevent(struct block_device *bdev, enum kobject_action action)
 
 int __init btrfs_init_sysfs(void);
 void __cold btrfs_exit_sysfs(void);
-int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info);
 void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info);
 void btrfs_sysfs_add_block_group_type(struct btrfs_block_group *cache);
 int btrfs_sysfs_add_space_info_type(struct btrfs_fs_info *fs_info,
-- 
2.23.0

[PATCH 13/15] btrfs: sysfs, cleanup btrfs_sysfs_remove_fsid()

[Anand Jain]

btrfs_sysfs_remove_fsid() when called with NULL argument then it shall
cleanup all kobjects belonging to btrfs. However no function is using
btrfs_sysfs_remove_fsid() with NULL argument.

This happened in the commit 2e3e1281 (Btrfs: sysfs: provide framework to
remove all fsid sysfs kobject) which is a helper commit to the proposed
patch [1], which proposed to show /sys/fs/btrfs/UUID in the unmounted but
scanned context as well, so that there is a way to know fsid and devices which
are scanned. As of now there isn't anyway that the user/script can figure
out all the scanned fsids/devices in the system, but its been long time,
probably there isn't such a need at all. For developers to debug the
struct fs_devices list there is procfs boiler-plate patch in the mailing
list which comes handy.
 [1] [PATCH] btrfs: Introduce device pool sysfs attributes

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/sysfs.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 2ea27c2d9ef1..34bf080ea530 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -1087,20 +1087,9 @@ static void __btrfs_sysfs_remove_fsid(struct btrfs_fs_info *fs_info)
 	}
 }
 
-/* when fs_info is NULL it will remove all fs_info::fsid kobject */
 void btrfs_sysfs_remove_fsid(struct btrfs_fs_info *fs_info)
 {
-	struct list_head *fs_uuids = btrfs_get_fs_uuids();
-	struct btrfs_fs_devices *fs_devices;
-
-	if (fs_info) {
-		__btrfs_sysfs_remove_fsid(fs_info);
-		return;
-	}
-
-	list_for_each_entry(fs_devices, fs_uuids, fs_list) {
-		__btrfs_sysfs_remove_fsid(fs_devices->fs_info);
-	}
+	__btrfs_sysfs_remove_fsid(fs_info);
 }
 
 /*
-- 
2.23.0

[PATCH 14/15] btrfs: sysfs, merge btrfs_sysfs_remove_fsid() helper function

[Anand Jain]

Merge __btrfs_sysfs_remove_fsid() into its only caller
btrfs_sysfs_remove_fsid().

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/sysfs.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 34bf080ea530..74210ef59641 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -1070,7 +1070,7 @@ static struct kset *btrfs_kset;
 /*
  * Remove /sys/fs/btrfs/UUID/devices and /sysfs/btrfs/UUID
  */
-static void __btrfs_sysfs_remove_fsid(struct btrfs_fs_info *fs_info)
+void btrfs_sysfs_remove_fsid(struct btrfs_fs_info *fs_info)
 {
 	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
 
@@ -1087,11 +1087,6 @@ static void __btrfs_sysfs_remove_fsid(struct btrfs_fs_info *fs_info)
 	}
 }
 
-void btrfs_sysfs_remove_fsid(struct btrfs_fs_info *fs_info)
-{
-	__btrfs_sysfs_remove_fsid(fs_info);
-}
-
 /*
  * Creates:
  * 	/sys/fs/btrfs/UUID
-- 
2.23.0

[PATCH 15/15] btrfs: sysfs, unexport btrfs_sysfs_remove_mounted()

[Anand Jain]

btrfs_sysfs_remove_mounted() is always called along with
btrfs_sysfs_remove_fsid(), call btrfs_sysfs_remove_mounted() in
btrfs_sysfs_remove_fsid() as well. And unexport
btrfs_sysfs_remove_mounted().

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/disk-io.c | 2 --
 fs/btrfs/sysfs.c   | 4 +++-
 fs/btrfs/sysfs.h   | 1 -
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 8f6a08bef490..28db7bbb437e 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3299,7 +3299,6 @@ int __cold open_ctree(struct super_block *sb,
 	filemap_write_and_wait(fs_info->btree_inode->i_mapping);
 
 fail_sysfs:
-	btrfs_sysfs_remove_mounted(fs_info);
 	btrfs_sysfs_remove_fsid(fs_info);
 
 fail_block_groups:
@@ -3997,7 +3996,6 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info)
 		btrfs_info(fs_info, "at unmount dio bytes count %lld",
 			   percpu_counter_sum(&fs_info->dio_bytes));
 
-	btrfs_sysfs_remove_mounted(fs_info);
 	btrfs_sysfs_remove_fsid(fs_info);
 
 	btrfs_free_fs_roots(fs_info);
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 74210ef59641..468a42c442cf 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -983,7 +983,7 @@ void btrfs_kobject_uevent(struct block_device *bdev, enum kobject_action action)
 			&disk_to_dev(bdev->bd_disk)->kobj);
 }
 
-void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
+static void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info)
 {
 	btrfs_reset_fs_info_ptr(fs_info);
 
@@ -1074,6 +1074,8 @@ void btrfs_sysfs_remove_fsid(struct btrfs_fs_info *fs_info)
 {
 	struct btrfs_fs_devices *fs_devices = fs_info->fs_devices;
 
+	btrfs_sysfs_remove_mounted(fs_info);
+
 	if (fs_devices->devices_dir_kobj) {
 		kobject_del(fs_devices->devices_dir_kobj);
 		kobject_put(fs_devices->devices_dir_kobj);
diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
index a977fe3bec64..2b01b3af5e50 100644
--- a/fs/btrfs/sysfs.h
+++ b/fs/btrfs/sysfs.h
@@ -28,7 +28,6 @@ void btrfs_kobject_uevent(struct block_device *bdev, enum kobject_action action)
 
 int __init btrfs_init_sysfs(void);
 void __cold btrfs_exit_sysfs(void);
-void btrfs_sysfs_remove_mounted(struct btrfs_fs_info *fs_info);
 void btrfs_sysfs_add_block_group_type(struct btrfs_block_group *cache);
 int btrfs_sysfs_add_space_info_type(struct btrfs_fs_info *fs_info,
 				    struct btrfs_space_info *space_info);
-- 
2.23.0

Re: [PATCH 00/15] btrfs: sysfs, cleanups

[David Sterba]

On Mon, Nov 18, 2019 at 04:46:41PM +0800, Anand Jain wrote:
> Mostly cleanups patches.
> 
> Patches 1-7 are renames, code moves patches and there are no
> functional changes.
> 
> Patch 8 drops unused argument in the function btrfs_sysfs_add_fsid().
> Patch 9 merges two small functions which is an extension of the other.
> 
> Patches 10,11 and 13 removes unnecessary features in the functions, 
> originally it was planned to provide sysfs attributes for the scanned
> and unmounted devices, as in the un-merged patch in the mailing list [1]
>    [1] [PATCH] btrfs: Introduce device pool sysfs attributes

We want something like that, I don't recall all the past discussions,
but a separate directory for all the new sysfs files should be
introduced. Extending the existing /devices/ that contains just the
sysfs device like should stay as is.

/sys/fs/btrfs/UUID/
	devinfo/
		1/
			uuid
			state
			...
		2/
			...

Re: [PATCH 00/15] btrfs: sysfs, cleanups

[Anand Jain]

On 11/18/19 11:45 PM, David Sterba wrote:
> On Mon, Nov 18, 2019 at 04:46:41PM +0800, Anand Jain wrote:
>> Mostly cleanups patches.
>>
>> Patches 1-7 are renames, code moves patches and there are no
>> functional changes.
>>
>> Patch 8 drops unused argument in the function btrfs_sysfs_add_fsid().
>> Patch 9 merges two small functions which is an extension of the other.
>>
>> Patches 10,11 and 13 removes unnecessary features in the functions,
>> originally it was planned to provide sysfs attributes for the scanned
>> and unmounted devices, as in the un-merged patch in the mailing list [1]
>>     [1] [PATCH] btrfs: Introduce device pool sysfs attributes
> 
> We want something like that,

  Oh.

  Ok then I shall relook at these patches with a mind that we might
  introduce the sysfs for non mounted devices.

> I don't recall all the past discussions,

  No worries. There wasn't any discussions on this specific topic.

> but a separate directory for all the new sysfs files should be
> introduced. Extending the existing /devices/ that contains just the
> sysfs device like should stay as is.
> 
> /sys/fs/btrfs/UUID/
> 	devinfo/
> 		1/
> 			uuid
> 			state
> 			...
> 		2/
> 			...
> 

  umm how about..

$ btrfs fi show
Label: none  uuid: 52ad6beb-524d-4cd8-8979-0890d0b74314
	Total devices 4 FS bytes used 384.00KiB
	devid    1 size 2.93GiB used 368.00MiB path /dev/sdb
	devid    2 size 2.93GiB used 368.00MiB path /dev/sdc
	devid    3 size 2.93GiB used 368.00MiB path /dev/sdd
	devid    4 size 2.93GiB used 368.00MiB path /dev/sde


# ls -l /sys/fs/btrfs/52ad6beb-524d-4cd8-8979-0890d0b74314/devices/
total 0
drwxr-xr-x 2 root root 0 Nov 19 14:39 1_sdb
drwxr-xr-x 2 root root 0 Nov 19 14:39 2_sdc
drwxr-xr-x 2 root root 0 Nov 19 14:39 3_sdd
drwxr-xr-x 2 root root 0 Nov 19 14:39 4_sde
lrwxrwxrwx 1 root root 0 Nov 19 14:39 sdb -> 
../../../../devices/pci0000:00/0000:00:0d.0/ata2/host1/target1:0:0/1:0:0:0/block/sdb
lrwxrwxrwx 1 root root 0 Nov 19 14:39 sdc -> 
../../../../devices/pci0000:00/0000:00:0d.0/ata3/host2/target2:0:0/2:0:0:0/block/sdc
lrwxrwxrwx 1 root root 0 Nov 19 14:39 sdd -> 
../../../../devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdd
lrwxrwxrwx 1 root root 0 Nov 19 14:39 sde -> 
../../../../devices/pci0000:00/0000:00:0d.0/ata5/host4/target4:0:0/4:0:0:0/block/sde

# cd /sys/fs/btrfs/52ad6beb-524d-4cd8-8979-0890d0b74314/devices/1_sdb
# ls -l
dev_state

(Currently its been coded to support only dev_state (patches under tests 
with me)).

Thanks, Anand

Re: [PATCH 00/15] btrfs: sysfs, cleanups

[David Sterba]

On Tue, Nov 19, 2019 at 02:44:10PM +0800, Anand Jain wrote:
> 
> 
> On 11/18/19 11:45 PM, David Sterba wrote:
> > On Mon, Nov 18, 2019 at 04:46:41PM +0800, Anand Jain wrote:
> >> Mostly cleanups patches.
> >>
> >> Patches 1-7 are renames, code moves patches and there are no
> >> functional changes.
> >>
> >> Patch 8 drops unused argument in the function btrfs_sysfs_add_fsid().
> >> Patch 9 merges two small functions which is an extension of the other.
> >>
> >> Patches 10,11 and 13 removes unnecessary features in the functions,
> >> originally it was planned to provide sysfs attributes for the scanned
> >> and unmounted devices, as in the un-merged patch in the mailing list [1]
> >>     [1] [PATCH] btrfs: Introduce device pool sysfs attributes
> > 
> > We want something like that,
> 
>   Oh.
> 
>   Ok then I shall relook at these patches with a mind that we might
>   introduce the sysfs for non mounted devices.
> 
> > I don't recall all the past discussions,
> 
>   No worries. There wasn't any discussions on this specific topic.

There were several patchsets sent adding device information exports, and
commented, eg.

https://lore.kernel.org/linux-btrfs/1423439785-10260-1-git-send-email-anand.jain@oracle.com/t/#u
https://lore.kernel.org/linux-btrfs/1416814173-16945-1-git-send-email-anand.jain@oracle.com/t/#u

and maybe followups.

> 
> > but a separate directory for all the new sysfs files should be
> > introduced. Extending the existing /devices/ that contains just the
> > sysfs device like should stay as is.
> > 
> > /sys/fs/btrfs/UUID/
> > 	devinfo/
> > 		1/
> > 			uuid
> > 			state
> > 			...
> > 		2/
> > 			...
> > 
> 
>   umm how about..
> 
> $ btrfs fi show
> Label: none  uuid: 52ad6beb-524d-4cd8-8979-0890d0b74314
> 	Total devices 4 FS bytes used 384.00KiB
> 	devid    1 size 2.93GiB used 368.00MiB path /dev/sdb
> 	devid    2 size 2.93GiB used 368.00MiB path /dev/sdc
> 	devid    3 size 2.93GiB used 368.00MiB path /dev/sdd
> 	devid    4 size 2.93GiB used 368.00MiB path /dev/sde
> 
> 
> # ls -l /sys/fs/btrfs/52ad6beb-524d-4cd8-8979-0890d0b74314/devices/
> total 0
> drwxr-xr-x 2 root root 0 Nov 19 14:39 1_sdb

Something like that has been suggested in the patchsets, I disagree with
the device id and name being glued together. The sysfs files should
server scripting, the enumeration should be straightforward and not
requiring parsing of the filenames.

> drwxr-xr-x 2 root root 0 Nov 19 14:39 2_sdc
> drwxr-xr-x 2 root root 0 Nov 19 14:39 3_sdd
> drwxr-xr-x 2 root root 0 Nov 19 14:39 4_sde
> lrwxrwxrwx 1 root root 0 Nov 19 14:39 sdb -> 
> ../../../../devices/pci0000:00/0000:00:0d.0/ata2/host1/target1:0:0/1:0:0:0/block/sdb
> lrwxrwxrwx 1 root root 0 Nov 19 14:39 sdc -> 
> ../../../../devices/pci0000:00/0000:00:0d.0/ata3/host2/target2:0:0/2:0:0:0/block/sdc
> lrwxrwxrwx 1 root root 0 Nov 19 14:39 sdd -> 
> ../../../../devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdd
> lrwxrwxrwx 1 root root 0 Nov 19 14:39 sde -> 
> ../../../../devices/pci0000:00/0000:00:0d.0/ata5/host4/target4:0:0/4:0:0:0/block/sde

If you want to put the directories under /sys/fs/btrfs/UUID/deevices/
then it's probably ok as long as there device node links are plain files
and the directories represent the ids. But just the ids, the actual
device name depends on the assignment by block layer. This is not
persistent.

So two possible layouts:

fs/UUID/devices
	sda
	sdb
	sdc
	1/
		...
	2/
		...
	3/
		...

Or the one suggested before, where devices by id are in a separate
directory. This is modelled after /dev/disk/by-id and the like, but I
don't think we need to make it granular like that.

Re: [PATCH 00/15] btrfs: sysfs, cleanups

[Anand Jain]

On 11/19/19 8:37 PM, David Sterba wrote:
> On Tue, Nov 19, 2019 at 02:44:10PM +0800, Anand Jain wrote:
>>
>>
>> On 11/18/19 11:45 PM, David Sterba wrote:
>>> On Mon, Nov 18, 2019 at 04:46:41PM +0800, Anand Jain wrote:
>>>> Mostly cleanups patches.
>>>>
>>>> Patches 1-7 are renames, code moves patches and there are no
>>>> functional changes.
>>>>
>>>> Patch 8 drops unused argument in the function btrfs_sysfs_add_fsid().
>>>> Patch 9 merges two small functions which is an extension of the other.
>>>>
>>>> Patches 10,11 and 13 removes unnecessary features in the functions,
>>>> originally it was planned to provide sysfs attributes for the scanned
>>>> and unmounted devices, as in the un-merged patch in the mailing list [1]
>>>>      [1] [PATCH] btrfs: Introduce device pool sysfs attributes
>>>
>>> We want something like that,
>>
>>    Oh.
>>
>>    Ok then I shall relook at these patches with a mind that we might
>>    introduce the sysfs for non mounted devices.
>>
>>> I don't recall all the past discussions,
>>
>>    No worries. There wasn't any discussions on this specific topic.
> 
> There were several patchsets sent adding device information exports, and
> commented, eg.
> 
> https://lore.kernel.org/linux-btrfs/1423439785-10260-1-git-send-email-anand.jain@oracle.com/t/#u
> https://lore.kernel.org/linux-btrfs/1416814173-16945-1-git-send-email-anand.jain@oracle.com/t/#u
> 
> and maybe followups.
> 

  Ah I missed those reply as I checked the much later patch.

 
https://lore.kernel.org/linux-btrfs/1ac04f0d-f33e-8161-4688-eebb42d51a54@oracle.com/

>>
>>> but a separate directory for all the new sysfs files should be
>>> introduced. Extending the existing /devices/ that contains just the
>>> sysfs device like should stay as is.
>>>
>>> /sys/fs/btrfs/UUID/
>>> 	devinfo/
>>> 		1/
>>> 			uuid
>>> 			state
>>> 			...
>>> 		2/
>>> 			...
>>>
>>
>>    umm how about..
>>
>> $ btrfs fi show
>> Label: none  uuid: 52ad6beb-524d-4cd8-8979-0890d0b74314
>> 	Total devices 4 FS bytes used 384.00KiB
>> 	devid    1 size 2.93GiB used 368.00MiB path /dev/sdb
>> 	devid    2 size 2.93GiB used 368.00MiB path /dev/sdc
>> 	devid    3 size 2.93GiB used 368.00MiB path /dev/sdd
>> 	devid    4 size 2.93GiB used 368.00MiB path /dev/sde
>>
>>
>> # ls -l /sys/fs/btrfs/52ad6beb-524d-4cd8-8979-0890d0b74314/devices/
>> total 0
>> drwxr-xr-x 2 root root 0 Nov 19 14:39 1_sdb
> 
> Something like that has been suggested in the patchsets, I disagree with
> the device id and name being glued together. The sysfs files should
> server scripting, the enumeration should be straightforward and not
> requiring parsing of the filenames.
> 

  Oh right point.

>> drwxr-xr-x 2 root root 0 Nov 19 14:39 2_sdc
>> drwxr-xr-x 2 root root 0 Nov 19 14:39 3_sdd
>> drwxr-xr-x 2 root root 0 Nov 19 14:39 4_sde
>> lrwxrwxrwx 1 root root 0 Nov 19 14:39 sdb ->
>> ../../../../devices/pci0000:00/0000:00:0d.0/ata2/host1/target1:0:0/1:0:0:0/block/sdb
>> lrwxrwxrwx 1 root root 0 Nov 19 14:39 sdc ->
>> ../../../../devices/pci0000:00/0000:00:0d.0/ata3/host2/target2:0:0/2:0:0:0/block/sdc
>> lrwxrwxrwx 1 root root 0 Nov 19 14:39 sdd ->
>> ../../../../devices/pci0000:00/0000:00:0d.0/ata4/host3/target3:0:0/3:0:0:0/block/sdd
>> lrwxrwxrwx 1 root root 0 Nov 19 14:39 sde ->
>> ../../../../devices/pci0000:00/0000:00:0d.0/ata5/host4/target4:0:0/4:0:0:0/block/sde
> 
> If you want to put the directories under /sys/fs/btrfs/UUID/deevices/
> then it's probably ok as long as there device node links are plain files
> and the directories represent the ids. But just the ids, the actual
> device name depends on the assignment by block layer. This is not
> persistent.
> 
> So two possible layouts:
> 
> fs/UUID/devices
> 	sda
> 	sdb
> 	sdc
> 	1/
> 		...
> 	2/
> 		...
> 	3/
> 		...

  Will use this layout.

Thanks, Anand

> Or the one suggested before, where devices by id are in a separate
> directory. This is modelled after /dev/disk/by-id and the like, but I
> don't think we need to make it granular like that.
>