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

Message ID 20191118084656.3089-6-anand.jain@oracle.com
State New
Headers show
Series
  • btrfs: sysfs, cleanups
Related show

Commit Message

Anand Jain Nov. 18, 2019, 8:46 a.m. UTC
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(-)

Comments

Nikolay Borisov Nov. 19, 2019, 9:24 a.m. UTC | #1
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.
David Sterba Nov. 19, 2019, 10:58 a.m. UTC | #2
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.
Anand Jain Nov. 20, 2019, 5:56 a.m. UTC | #3
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.
David Sterba Nov. 22, 2019, 5:48 p.m. UTC | #4
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.
Anand Jain Nov. 25, 2019, 3:18 a.m. UTC | #5
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
diff mbox series

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