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
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 <redacted>
---
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(-)
@@ -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 */
@@ -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 */-intbtrfs_sysfs_rm_device_link(structbtrfs_fs_devices*fs_devices,+intbtrfs_sysfs_remove_device_info(structbtrfs_fs_devices*fs_devices,structbtrfs_device*one_device){structhd_struct*disk;
@@ -1019,7 +1019,7 @@ int btrfs_sysfs_add_device(struct btrfs_fs_devices *fs_devs)return0;}-intbtrfs_sysfs_add_device_link(structbtrfs_fs_devices*fs_devices,+intbtrfs_sysfs_add_device_info(structbtrfs_fs_devices*fs_devices,structbtrfs_device*one_device){interror=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)returnerror;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);returnerror;}
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 <redacted>
---
fs/btrfs/disk-io.c | 2 +-
fs/btrfs/sysfs.c | 2 +-
fs/btrfs/sysfs.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
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 <redacted>
---
fs/btrfs/sysfs.c | 28 ++++++++++++++--------------
fs/btrfs/volumes.h | 2 +-
2 files changed, 15 insertions(+), 15 deletions(-)
@@ -742,36 +742,6 @@ static int addrm_unknown_feature_attrs(struct btrfs_fs_info *fs_info, bool add)return0;}-staticvoid__btrfs_sysfs_remove_fsid(structbtrfs_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 */-voidbtrfs_sysfs_remove_fsid(structbtrfs_fs_devices*fs_devs)-{-structlist_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);-}-}-voidbtrfs_sysfs_remove_mounted(structbtrfs_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 */staticstructkset*btrfs_kset;+staticvoid__btrfs_sysfs_remove_fsid(structbtrfs_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 */+voidbtrfs_sysfs_remove_fsid(structbtrfs_fs_devices*fs_devs)+{+structlist_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);+}+}+/**Canbecalledbythedevicediscoverythread.*Andparentcanbespecifiedforseeddevice
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 <redacted>
---
fs/btrfs/sysfs.c | 132 +++++++++++++++++++++++------------------------
1 file changed, 66 insertions(+), 66 deletions(-)
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 <redacted>
---
fs/btrfs/disk-io.c | 2 +-
fs/btrfs/sysfs.c | 11 +++++------
fs/btrfs/sysfs.h | 3 +--
3 files changed, 7 insertions(+), 9 deletions(-)
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 <redacted>
---
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(-)
@@ -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);}
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 <redacted>
---
fs/btrfs/disk-io.c | 7 -------
fs/btrfs/sysfs.c | 22 +++++++++-------------
fs/btrfs/sysfs.h | 1 -
3 files changed, 9 insertions(+), 21 deletions(-)
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 <redacted>
---
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(-)
@@ -1010,7 +1002,7 @@ int btrfs_sysfs_add_mounted(struct btrfs_fs_info *fs_info){interror;structbtrfs_fs_devices*fs_devs=fs_info->fs_devices;-structkobject*fsid_kobj=&fs_devs->fsid_kobj;+structkobject*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 */staticstructkset*btrfs_kset;-staticvoid__btrfs_sysfs_remove_fsid(structbtrfs_fs_devices*fs_devs)+/*+*Remove/sys/fs/btrfs/UUID/devicesand/sysfs/btrfs/UUID+*/+staticvoid__btrfs_sysfs_remove_fsid(structbtrfs_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;+structbtrfs_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 */-voidbtrfs_sysfs_remove_fsid(structbtrfs_fs_devices*fs_devs)+/* when fs_info is NULL it will remove all fs_info::fsid kobject */+voidbtrfs_sysfs_remove_fsid(structbtrfs_fs_info*fs_info){structlist_head*fs_uuids=btrfs_get_fs_uuids();+structbtrfs_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*/-intbtrfs_sysfs_add_fsid(structbtrfs_fs_devices*fs_devs)+intbtrfs_sysfs_add_fsid(structbtrfs_fs_info*fs_info){interror;+structbtrfs_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);returnerror;}-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)voidbtrfs_sysfs_feature_update(structbtrfs_fs_info*fs_info,u64bit,enumbtrfs_feature_setset){-structbtrfs_fs_devices*fs_devs;structkobject*fsid_kobj;u64features;intret;
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 <redacted>
---
fs/btrfs/disk-io.c | 8 --------
fs/btrfs/sysfs.c | 9 ++++++++-
fs/btrfs/sysfs.h | 1 -
3 files changed, 8 insertions(+), 10 deletions(-)
@@ -3089,12 +3089,6 @@ int __cold open_ctree(struct super_block *sb,gotofail_block_groups;}-ret=btrfs_sysfs_add_mounted(fs_info);-if(ret){-btrfs_err(fs_info,"failed to init sysfs interface: %d",ret);-gotofail_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:
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 <redacted>
---
fs/btrfs/sysfs.c | 13 +------------
1 file changed, 1 insertion(+), 12 deletions(-)
@@ -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 */voidbtrfs_sysfs_remove_fsid(structbtrfs_fs_info*fs_info){-structlist_head*fs_uuids=btrfs_get_fs_uuids();-structbtrfs_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);}/*
From: David Sterba <hidden> Date: 2019-11-18 15:45:58
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/
...
On Mon, Nov 18, 2019 at 04:46:41PM +0800, Anand Jain wrote:
quoted
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
From: Nikolay Borisov <hidden> Date: 2019-11-19 09:24:43
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 <redacted>
From: David Sterba <hidden> Date: 2019-11-19 10:58:58
On Tue, Nov 19, 2019 at 11:24:37AM +0200, Nikolay Borisov wrote:
On 18.11.19 г. 10:46 ч., Anand Jain wrote:
quoted
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 <redacted>
This seems like pointless code motion.
Yeah, unless there's some other reason to move the code, just plain
moves are not desired.
From: David Sterba <hidden> Date: 2019-11-19 12:37:54
On Tue, Nov 19, 2019 at 02:44:10PM +0800, Anand Jain wrote:
On 11/18/19 11:45 PM, David Sterba wrote:
quoted
On Mon, Nov 18, 2019 at 04:46:41PM +0800, Anand Jain wrote:
quoted
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.
quoted
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
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.
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.
On Tue, Nov 19, 2019 at 11:24:37AM +0200, Nikolay Borisov wrote:
quoted
On 18.11.19 г. 10:46 ч., Anand Jain wrote:
quoted
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 <redacted>
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 Tue, Nov 19, 2019 at 02:44:10PM +0800, Anand Jain wrote:
quoted
On 11/18/19 11:45 PM, David Sterba wrote:
quoted
On Mon, Nov 18, 2019 at 04:46:41PM +0800, Anand Jain wrote:
quoted
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.
quoted
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
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.
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.
From: David Sterba <hidden> Date: 2019-11-22 17:48:40
On Wed, Nov 20, 2019 at 01:56:04PM +0800, Anand Jain wrote:
On 11/19/19 6:58 PM, David Sterba wrote:
quoted
On Tue, Nov 19, 2019 at 11:24:37AM +0200, Nikolay Borisov wrote:
quoted
On 18.11.19 г. 10:46 ч., Anand Jain wrote:
quoted
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 <redacted>
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 Wed, Nov 20, 2019 at 01:56:04PM +0800, Anand Jain wrote:
quoted
On 11/19/19 6:58 PM, David Sterba wrote:
quoted
On Tue, Nov 19, 2019 at 11:24:37AM +0200, Nikolay Borisov wrote:
quoted
On 18.11.19 г. 10:46 ч., Anand Jain wrote:
quoted
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 <redacted>
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.