| Message ID | 20191118084656.3089-6-anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: sysfs, cleanups | expand |
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.
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.
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.
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.
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
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
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(-)