Linux-BTRFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 0/5] btrfs: sysfs, cleanups
@ 2019-11-21  9:33 Anand Jain
  2019-11-21  9:33 ` [PATCH v2 1/5] btrfs: sysfs, rename devices kobject holder to devices_kobj Anand Jain
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Anand Jain @ 2019-11-21  9:33 UTC (permalink / raw)
  To: linux-btrfs

v2:
  As it is learnt that we might add sysfs attributes for the
  scanned devices at some point, V2 does not cleanup those which
  might be required to implement it. Last attempt to add is here
  [1] in the mailing list.
   [1] [PATCH] btrfs: Introduce device pool sysfs attributes

  This set also drops the patches which moved the local struct declare
  to the place where it was actually used. Agreed top of the file is
  better.

  Drops the patches which was trying to bring the related functions
  together in sysfs.c. I am not sure why this isn't a nice cleanup,
  as in super.c we don't place exit_btrfs_fs() and init_btrfs_fs()
  apart we place them together the idea was similar in sysfs.c.
  Anyway as there is disagreement I have dropped them from the list.

  Dropped the patch '[PATCH 07/15] btrfs: sysfs, delete code in a
  comment' as its already integrated in misc-next.

  In v2 I have used better naming compared to v1. For example
  btrfs_fs_devices::devices_dir_kobj vs btrfs_fs_devices::devices_kobj
  and btrfs_sysfs_add_device_info() vs btrfs_sysfs_add_devices_attr,
  as I am following a pattern that sysfs directories are inherently
  kobjects, which holds files as attributes. We could drop sysfs prefix
  also because kobj and attr already indicate that they are part of
  sysfs. But people not familiar with sysfs terminology might have to
  wonder a little bit, so didn't make that bold changes.

  And I hope I didn't miss any other changes in v2.

Testing:

 As this is a cleanup patches, it should be made sure not to introduce any
 regression in the sysfs layout. So here is a script which compares with
 the golden md5sum of the sysfs layout. There are two golden md5sum
 because as I am using find command which looks like due to some
 optimization the list of file was in the same order all the time, it was
 in two different orders. If there is any better ways to do this, or tell
 find to not to optimize I will be happy to know and use it.

 (sorry for the hard-coded device paths).
-----------8<----------------
#!/bin/bash

known_uuid="12345678-1234-1234-1234-123456789012"
golden_sum1="7ae7b72ee3e5efc32f2fab152dd0fcd5"
golden_sum2="6cefff853e55a31047c57f5a14ffb636"

cleanup()
{
	umount /btrfs > /dev/null 2>&1
	modprobe -r btrfs
}

test()
{
	cleanup

	mkfs.btrfs -fq -draid1 -mraid1 /dev/sdb /dev/sdc
	uuid=$(btrfs fi show /dev/sdb | grep uuid | awk '{print $4}')

	mount -o compress=lzo,device=/dev/sdc /dev/sdb /btrfs
	xfs_io -f -c "pwrite -S 0xab 0 500M" /btrfs/foo  > /dev/null

	find -O0 /sys/fs/btrfs -type f | sed s/$uuid/$known_uuid/g > /tmp/golden.output
	test_sum=$(md5sum /tmp/golden.output|awk '{print $1}')

	umount /btrfs
	modprobe -r btrfs

	[ "$golden_sum1" == "$test_sum" ] && return
	[ "$golden_sum2" == "$test_sum" ] && return

	echo "Failed: test_sum $test_sum"
	echo "Golden:        1 $golden_sum1"
	echo "Golden:        2 $golden_sum2"
}

test
-----------8<----------------



v1: cover-letter

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 (5):
  btrfs: sysfs, rename devices kobject holder to devices_kobj
  btrfs: sysfs, rename device_link add,remove functions
  btrfs: sysfs, btrfs_sysfs_add_fsid() drop unused argument parent
  btrfs: sysfs, rename btrfs_sysfs_add_device()
  btrfs: sysfs, merge btrfs_sysfs_add devices_kobj and fsid

 fs/btrfs/dev-replace.c |  4 ++--
 fs/btrfs/disk-io.c     |  9 +-------
 fs/btrfs/sysfs.c       | 62 ++++++++++++++++++++++++--------------------------
 fs/btrfs/sysfs.h       |  8 +++----
 fs/btrfs/volumes.c     |  8 +++----
 fs/btrfs/volumes.h     |  2 +-
 6 files changed, 41 insertions(+), 52 deletions(-)

-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 1/5] btrfs: sysfs, rename devices kobject holder to devices_kobj
  2019-11-21  9:33 [PATCH v2 0/5] btrfs: sysfs, cleanups Anand Jain
@ 2019-11-21  9:33 ` Anand Jain
  2019-11-26 16:41   ` David Sterba
  2019-11-21  9:33 ` [PATCH v2 2/5] btrfs: sysfs, rename device_link add,remove functions Anand Jain
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Anand Jain @ 2019-11-21  9:33 UTC (permalink / raw)
  To: linux-btrfs

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

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2: Rename devices_dir_kobj to devices_kobj

 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 5ebbe8a5ee76..29e82832ff18 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -734,10 +734,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_kobj) {
+		kobject_del(fs_devs->devices_kobj);
+		kobject_put(fs_devs->devices_kobj);
+		fs_devs->devices_kobj = NULL;
 	}
 
 	if (fs_devs->fsid_kobj.state_initialized) {
@@ -969,15 +969,15 @@ int btrfs_sysfs_rm_device_link(struct btrfs_fs_devices *fs_devices,
 	struct hd_struct *disk;
 	struct kobject *disk_kobj;
 
-	if (!fs_devices->device_dir_kobj)
+	if (!fs_devices->devices_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_kobj,
+				  disk_kobj->name);
 	}
 
 	if (one_device)
@@ -990,8 +990,8 @@ int btrfs_sysfs_rm_device_link(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_kobj,
+				  disk_kobj->name);
 	}
 
 	return 0;
@@ -999,11 +999,11 @@ int btrfs_sysfs_rm_device_link(struct btrfs_fs_devices *fs_devices,
 
 int btrfs_sysfs_add_device(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_kobj)
+		fs_devs->devices_kobj = kobject_create_and_add("devices",
+							&fs_devs->fsid_kobj);
 
-	if (!fs_devs->device_dir_kobj)
+	if (!fs_devs->devices_kobj)
 		return -ENOMEM;
 
 	return 0;
@@ -1028,7 +1028,7 @@ int btrfs_sysfs_add_device_link(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_kobj,
 					  disk_kobj, disk_kobj->name);
 		if (error)
 			break;
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index fc1b564b9cfe..3c56ef571b00 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_kobj;
 	struct completion kobj_unregister;
 };
 
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 2/5] btrfs: sysfs, rename device_link add,remove functions
  2019-11-21  9:33 [PATCH v2 0/5] btrfs: sysfs, cleanups Anand Jain
  2019-11-21  9:33 ` [PATCH v2 1/5] btrfs: sysfs, rename devices kobject holder to devices_kobj Anand Jain
@ 2019-11-21  9:33 ` Anand Jain
  2019-11-26 16:40   ` David Sterba
  2019-11-21  9:33 ` [PATCH v2 3/5] btrfs: sysfs, btrfs_sysfs_add_fsid() drop unused argument parent Anand Jain
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Anand Jain @ 2019-11-21  9:33 UTC (permalink / raw)
  To: linux-btrfs

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_devices_attr() and btrfs_sysfs_remove_devices_attr() 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>
---
v2: rename to ..devices_attr() instead of ..device_info()

 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..9a29d6de9017 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_devices_attr(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_devices_attr(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 29e82832ff18..a9bb507a06f1 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -774,7 +774,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_devices_attr(fs_info->fs_devices, NULL);
 }
 
 static const char * const btrfs_feature_set_names[FEAT_MAX] = {
@@ -963,7 +963,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_devices_attr(struct btrfs_fs_devices *fs_devices,
 		struct btrfs_device *one_device)
 {
 	struct hd_struct *disk;
@@ -1009,7 +1009,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_devices_attr(struct btrfs_fs_devices *fs_devices,
 				struct btrfs_device *one_device)
 {
 	int error = 0;
@@ -1095,13 +1095,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_devices_attr(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_devices_attr(fs_devs, NULL);
 		return error;
 	}
 
diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
index e10c3adfc30f..4a9057def6ad 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_devices_attr(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_devices_attr(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..c8d5a8af224c 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_devices_attr(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_devices_attr(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_devices_attr(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_devices_attr(fs_devices, device);
 	mutex_lock(&fs_info->fs_devices->device_list_mutex);
 	mutex_lock(&fs_info->chunk_mutex);
 	list_del_rcu(&device->dev_list);
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 3/5] btrfs: sysfs, btrfs_sysfs_add_fsid() drop unused argument parent
  2019-11-21  9:33 [PATCH v2 0/5] btrfs: sysfs, cleanups Anand Jain
  2019-11-21  9:33 ` [PATCH v2 1/5] btrfs: sysfs, rename devices kobject holder to devices_kobj Anand Jain
  2019-11-21  9:33 ` [PATCH v2 2/5] btrfs: sysfs, rename device_link add,remove functions Anand Jain
@ 2019-11-21  9:33 ` Anand Jain
  2019-11-21  9:33 ` [PATCH v2 4/5] btrfs: sysfs, rename btrfs_sysfs_add_device() Anand Jain
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Anand Jain @ 2019-11-21  9:33 UTC (permalink / raw)
  To: linux-btrfs

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>
---
V2: comment updated.

 fs/btrfs/disk-io.c |  2 +-
 fs/btrfs/sysfs.c   | 11 ++++++-----
 fs/btrfs/sysfs.h   |  5 ++---
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index b521e9085228..264c8f0d7dfa 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3083,7 +3083,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 a9bb507a06f1..3fda2b3a74b8 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -1067,18 +1067,19 @@ void btrfs_sysfs_update_sprout_fsid(struct btrfs_fs_devices *fs_devices,
 static struct kset *btrfs_kset;
 
 /*
+ * Creates:
+ *		/sys/fs/btrfs/UUID
+ *
  * Can be called by the device discovery thread.
- * And parent can be specified for seed 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 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 4a9057def6ad..f7ddcbf4a40d 100644
--- a/fs/btrfs/sysfs.h
+++ b/fs/btrfs/sysfs.h
@@ -18,9 +18,8 @@ int btrfs_sysfs_add_devices_attr(struct btrfs_fs_devices *fs_devices,
 		struct btrfs_device *one_device);
 int btrfs_sysfs_remove_devices_attr(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_fsid(struct btrfs_fs_devices *fs_devs);
+int btrfs_sysfs_add_devices_kobj(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);
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 4/5] btrfs: sysfs, rename btrfs_sysfs_add_device()
  2019-11-21  9:33 [PATCH v2 0/5] btrfs: sysfs, cleanups Anand Jain
                   ` (2 preceding siblings ...)
  2019-11-21  9:33 ` [PATCH v2 3/5] btrfs: sysfs, btrfs_sysfs_add_fsid() drop unused argument parent Anand Jain
@ 2019-11-21  9:33 ` Anand Jain
  2019-11-21  9:33 ` [PATCH v2 5/5] btrfs: sysfs, merge btrfs_sysfs_add devices_kobj and fsid Anand Jain
  2019-11-26 16:54 ` [PATCH v2 0/5] btrfs: sysfs, cleanups David Sterba
  5 siblings, 0 replies; 11+ messages in thread
From: Anand Jain @ 2019-11-21  9:33 UTC (permalink / raw)
  To: linux-btrfs

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_kobj() instead. No functional changes.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
V2: rename to btrfs_sysfs_add_devices_kobj, instead of
    btrfs_sysfs_add_devices_dir

 fs/btrfs/disk-io.c | 2 +-
 fs/btrfs/sysfs.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 264c8f0d7dfa..c4cebc44f683 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3090,7 +3090,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_kobj(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 3fda2b3a74b8..e8dfa2561909 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -997,7 +997,7 @@ int btrfs_sysfs_remove_devices_attr(struct btrfs_fs_devices *fs_devices,
 	return 0;
 }
 
-int btrfs_sysfs_add_device(struct btrfs_fs_devices *fs_devs)
+int btrfs_sysfs_add_devices_kobj(struct btrfs_fs_devices *fs_devs)
 {
 	if (!fs_devs->devices_kobj)
 		fs_devs->devices_kobj = kobject_create_and_add("devices",
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 5/5] btrfs: sysfs, merge btrfs_sysfs_add devices_kobj and fsid
  2019-11-21  9:33 [PATCH v2 0/5] btrfs: sysfs, cleanups Anand Jain
                   ` (3 preceding siblings ...)
  2019-11-21  9:33 ` [PATCH v2 4/5] btrfs: sysfs, rename btrfs_sysfs_add_device() Anand Jain
@ 2019-11-21  9:33 ` Anand Jain
  2019-11-26 16:44   ` David Sterba
  2019-11-26 16:54 ` [PATCH v2 0/5] btrfs: sysfs, cleanups David Sterba
  5 siblings, 1 reply; 11+ messages in thread
From: Anand Jain @ 2019-11-21  9:33 UTC (permalink / raw)
  To: linux-btrfs

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

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v2: nothing in specific to this patch actually, but the changes
    before this patch makes v1 apply fail. So fixed in v2.

 fs/btrfs/disk-io.c |  7 -------
 fs/btrfs/sysfs.c   | 21 +++++++++------------
 fs/btrfs/sysfs.h   |  1 -
 3 files changed, 9 insertions(+), 20 deletions(-)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index c4cebc44f683..d93eeb048193 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3090,13 +3090,6 @@ int __cold open_ctree(struct super_block *sb,
 		goto fail_block_groups;
 	}
 
-	ret = btrfs_sysfs_add_devices_kobj(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 e8dfa2561909..61b4c04f5a1c 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -997,18 +997,6 @@ int btrfs_sysfs_remove_devices_attr(struct btrfs_fs_devices *fs_devices,
 	return 0;
 }
 
-int btrfs_sysfs_add_devices_kobj(struct btrfs_fs_devices *fs_devs)
-{
-	if (!fs_devs->devices_kobj)
-		fs_devs->devices_kobj = kobject_create_and_add("devices",
-							&fs_devs->fsid_kobj);
-
-	if (!fs_devs->devices_kobj)
-		return -ENOMEM;
-
-	return 0;
-}
-
 int btrfs_sysfs_add_devices_attr(struct btrfs_fs_devices *fs_devices,
 				struct btrfs_device *one_device)
 {
@@ -1085,6 +1073,15 @@ int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs)
 		return error;
 	}
 
+	fs_devs->devices_kobj = kobject_create_and_add("devices",
+						       &fs_devs->fsid_kobj);
+	if (!fs_devs->devices_kobj) {
+		btrfs_err(fs_devs->fs_info,
+			  "failed to init sysfs device interface");
+		kobject_put(&fs_devs->fsid_kobj);
+		return -ENOMEM;
+	}
+
 	return 0;
 }
 
diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h
index f7ddcbf4a40d..9d97b3c8db4e 100644
--- a/fs/btrfs/sysfs.h
+++ b/fs/btrfs/sysfs.h
@@ -19,7 +19,6 @@ int btrfs_sysfs_add_devices_attr(struct btrfs_fs_devices *fs_devices,
 int btrfs_sysfs_remove_devices_attr(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_kobj(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);
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/5] btrfs: sysfs, rename device_link add,remove functions
  2019-11-21  9:33 ` [PATCH v2 2/5] btrfs: sysfs, rename device_link add,remove functions Anand Jain
@ 2019-11-26 16:40   ` David Sterba
  2019-12-03  5:42     ` Anand Jain
  0 siblings, 1 reply; 11+ messages in thread
From: David Sterba @ 2019-11-26 16:40 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Thu, Nov 21, 2019 at 05:33:31PM +0800, Anand Jain wrote:
> In preparation to add btrfs_device::dev_state attribute in
>   /sys/fs/btrfs/UUID/devices/

But we don't want to add any attributes to that directory, is this some
leftover from the previous iterations?

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

I think the function name matches what it does, it really links the
device.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/5] btrfs: sysfs, rename devices kobject holder to devices_kobj
  2019-11-21  9:33 ` [PATCH v2 1/5] btrfs: sysfs, rename devices kobject holder to devices_kobj Anand Jain
@ 2019-11-26 16:41   ` David Sterba
  0 siblings, 0 replies; 11+ messages in thread
From: David Sterba @ 2019-11-26 16:41 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Thu, Nov 21, 2019 at 05:33:30PM +0800, Anand Jain wrote:
> The struct member btrfs_device::device_dir_kobj holds the kobj of the
> sysfs directory /sys/fs/btrfs/UUID/devices, so rename it from
> device_dir_kobj to devices_kobj. No functional changes.

That naming scheme looks good, ok.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 5/5] btrfs: sysfs, merge btrfs_sysfs_add devices_kobj and fsid
  2019-11-21  9:33 ` [PATCH v2 5/5] btrfs: sysfs, merge btrfs_sysfs_add devices_kobj and fsid Anand Jain
@ 2019-11-26 16:44   ` David Sterba
  0 siblings, 0 replies; 11+ messages in thread
From: David Sterba @ 2019-11-26 16:44 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Thu, Nov 21, 2019 at 05:33:34PM +0800, Anand Jain wrote:
> Merge btrfs_sysfs_add_fsid() and btrfs_sysfs_add_devices_kobj() functions
> as these two are small and they are called one after the other.

Makes sense, all the subdirectories under the fsid directory can be
created inside one function.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 0/5] btrfs: sysfs, cleanups
  2019-11-21  9:33 [PATCH v2 0/5] btrfs: sysfs, cleanups Anand Jain
                   ` (4 preceding siblings ...)
  2019-11-21  9:33 ` [PATCH v2 5/5] btrfs: sysfs, merge btrfs_sysfs_add devices_kobj and fsid Anand Jain
@ 2019-11-26 16:54 ` David Sterba
  5 siblings, 0 replies; 11+ messages in thread
From: David Sterba @ 2019-11-26 16:54 UTC (permalink / raw)
  To: Anand Jain; +Cc: linux-btrfs

On Thu, Nov 21, 2019 at 05:33:29PM +0800, Anand Jain wrote:
> v2:
>   In v2 I have used better naming compared to v1. For example
>   btrfs_fs_devices::devices_dir_kobj vs btrfs_fs_devices::devices_kobj
>   and btrfs_sysfs_add_device_info() vs btrfs_sysfs_add_devices_attr,
>   as I am following a pattern that sysfs directories are inherently
>   kobjects, which holds files as attributes. We could drop sysfs prefix
>   also because kobj and attr already indicate that they are part of
>   sysfs. But people not familiar with sysfs terminology might have to
>   wonder a little bit, so didn't make that bold changes.

I think the _sysfs_ part of the function is a good thing and makes it
clear that it belongs to the sysfs interfacing wrappers.

> Anand Jain (5):
>   btrfs: sysfs, rename devices kobject holder to devices_kobj
>   btrfs: sysfs, rename device_link add,remove functions
>   btrfs: sysfs, btrfs_sysfs_add_fsid() drop unused argument parent
>   btrfs: sysfs, rename btrfs_sysfs_add_device()
>   btrfs: sysfs, merge btrfs_sysfs_add devices_kobj and fsid

1,3,4,5 applied, thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 2/5] btrfs: sysfs, rename device_link add,remove functions
  2019-11-26 16:40   ` David Sterba
@ 2019-12-03  5:42     ` Anand Jain
  0 siblings, 0 replies; 11+ messages in thread
From: Anand Jain @ 2019-12-03  5:42 UTC (permalink / raw)
  To: dsterba, linux-btrfs

On 11/27/19 12:40 AM, David Sterba wrote:
> On Thu, Nov 21, 2019 at 05:33:31PM +0800, Anand Jain wrote:
>> In preparation to add btrfs_device::dev_state attribute in
>>    /sys/fs/btrfs/UUID/devices/
> 
> But we don't want to add any attributes to that directory, is this some
> leftover from the previous iterations?

No actually. We discussed to add <devid>/dev_state under UUID/devices
here [1].
[1]
https://lore.kernel.org/linux-btrfs/516598e6-4968-4535-cf6b-12402518b7cc@oracle.com/

Looks like you still prefer separate UUID/devinfo directory as you
mentioned here [2]

[2]
https://lore.kernel.org/linux-btrfs/20191118154556.GJ3001@twin.jikos.cz/

?

Thanks, Anand

>> Rename btrfs_sysfs_add_device_link() and btrfs_sysfs_rm_device_link() to
>> btrfs_sysfs_add_devices_attr() and btrfs_sysfs_remove_devices_attr() as
>> these functions is going to create more attributes rather than just the
>> link to the disk. No functional changes.
> 
> I think the function name matches what it does, it really links the
> device.
> 




^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, back to index

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-21  9:33 [PATCH v2 0/5] btrfs: sysfs, cleanups Anand Jain
2019-11-21  9:33 ` [PATCH v2 1/5] btrfs: sysfs, rename devices kobject holder to devices_kobj Anand Jain
2019-11-26 16:41   ` David Sterba
2019-11-21  9:33 ` [PATCH v2 2/5] btrfs: sysfs, rename device_link add,remove functions Anand Jain
2019-11-26 16:40   ` David Sterba
2019-12-03  5:42     ` Anand Jain
2019-11-21  9:33 ` [PATCH v2 3/5] btrfs: sysfs, btrfs_sysfs_add_fsid() drop unused argument parent Anand Jain
2019-11-21  9:33 ` [PATCH v2 4/5] btrfs: sysfs, rename btrfs_sysfs_add_device() Anand Jain
2019-11-21  9:33 ` [PATCH v2 5/5] btrfs: sysfs, merge btrfs_sysfs_add devices_kobj and fsid Anand Jain
2019-11-26 16:44   ` David Sterba
2019-11-26 16:54 ` [PATCH v2 0/5] btrfs: sysfs, cleanups David Sterba

Linux-BTRFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-btrfs/0 linux-btrfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-btrfs linux-btrfs/ https://lore.kernel.org/linux-btrfs \
		linux-btrfs@vger.kernel.org
	public-inbox-index linux-btrfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-btrfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git