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