* [PATCH v2 0/2] provide fsid in sysfs devinfo
@ 2021-10-21 15:31 Anand Jain
2021-10-21 15:31 ` [PATCH v2 1/2] btrfs: sysfs convert scnprintf and snprintf to use sysfs_emit Anand Jain
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Anand Jain @ 2021-10-21 15:31 UTC (permalink / raw)
To: linux-btrfs
v2:
Fix sysfs_emit conversion at the two non-sysfs show functions in patch1.
btrfs-progs tries to read the fsid from the super-block for a missing
device and, it fails. It needs to find out if the device is a seed
device. It does it by comparing the device's fsid with the fsid of the
mounted filesystem. To help this scenario introduce a new sysfs file to
read the fsid from the kernel.
/sys/fs/btrfs/<fsid>/devinfo/<devid>/fsid
Patch 1 is a cleanup converts scnprtin()f and snprintf() to sysfs_emit()
Patch 2 introduces the new sysfs interface as above
The other implementation choice is to add another parameter to the
struct btrfs_ioctl_dev_info_args and use BTRFS_IOC_DEV_INFO ioctl. But
then backward kernel compatibility with the newer btrfs-progs is more
complicated. If needed, we can add that too.
Related btrfs-progs patches:
btrfs-progs: prepare helper device_is_seed
btrfs-progs: read fsid from the sysfs in device_is_seed
Anand Jain (2):
btrfs: sysfs convert scnprintf and snprintf to use sysfs_emit
btrfs: sysfs add devinfo/fsid to retrieve fsid from the device
fs/btrfs/sysfs.c | 106 +++++++++++++++++++++++++----------------------
1 file changed, 57 insertions(+), 49 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v2 1/2] btrfs: sysfs convert scnprintf and snprintf to use sysfs_emit 2021-10-21 15:31 [PATCH v2 0/2] provide fsid in sysfs devinfo Anand Jain @ 2021-10-21 15:31 ` Anand Jain 2021-10-21 16:19 ` David Sterba 2021-10-21 15:31 ` [PATCH v2 2/2] btrfs: sysfs add devinfo/fsid to retrieve fsid from the device Anand Jain 2021-11-09 10:09 ` [PATCH v2 0/2] provide fsid in sysfs devinfo Anand Jain 2 siblings, 1 reply; 13+ messages in thread From: Anand Jain @ 2021-10-21 15:31 UTC (permalink / raw) To: linux-btrfs; +Cc: kernel test robot commit 2efc459d06f1 (sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs out) merged in 5.10 introduced two new functions sysfs_emit() and sysfs_emit_at() which are aware of the PAGE_SIZE max_limit of the buf. Use the above two new functions instead of scnprintf() and snprintf() in various sysfs show(). Signed-off-by: Anand Jain <anand.jain@oracle.com> Reported-by: kernel test robot <oliver.sang@intel.com> --- fs/btrfs/sysfs.c | 95 +++++++++++++++++++++++------------------------- 1 file changed, 46 insertions(+), 49 deletions(-) diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index 25a6f587852b..28ff7fce1ac5 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -177,7 +177,7 @@ static ssize_t btrfs_feature_attr_show(struct kobject *kobj, } else val = can_modify_feature(fa); - return scnprintf(buf, PAGE_SIZE, "%d\n", val); + return sysfs_emit(buf, "%d\n", val); } static ssize_t btrfs_feature_attr_store(struct kobject *kobj, @@ -330,7 +330,7 @@ static const struct attribute_group btrfs_feature_attr_group = { static ssize_t rmdir_subvol_show(struct kobject *kobj, struct kobj_attribute *ka, char *buf) { - return scnprintf(buf, PAGE_SIZE, "0\n"); + return sysfs_emit(buf, "0\n"); } BTRFS_ATTR(static_feature, rmdir_subvol, rmdir_subvol_show); @@ -345,12 +345,12 @@ static ssize_t supported_checksums_show(struct kobject *kobj, * This "trick" only works as long as 'enum btrfs_csum_type' has * no holes in it */ - ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%s%s", - (i == 0 ? "" : " "), btrfs_super_csum_name(i)); + ret += sysfs_emit_at(buf, ret, "%s%s", (i == 0 ? "" : " "), + btrfs_super_csum_name(i)); } - ret += scnprintf(buf + ret, PAGE_SIZE - ret, "\n"); + ret += sysfs_emit_at(buf, ret, "\n"); return ret; } BTRFS_ATTR(static_feature, supported_checksums, supported_checksums_show); @@ -358,7 +358,7 @@ BTRFS_ATTR(static_feature, supported_checksums, supported_checksums_show); static ssize_t send_stream_version_show(struct kobject *kobj, struct kobj_attribute *ka, char *buf) { - return snprintf(buf, PAGE_SIZE, "%d\n", BTRFS_SEND_STREAM_VERSION); + return sysfs_emit(buf, "%d\n", BTRFS_SEND_STREAM_VERSION); } BTRFS_ATTR(static_feature, send_stream_version, send_stream_version_show); @@ -378,9 +378,9 @@ static ssize_t supported_rescue_options_show(struct kobject *kobj, int i; for (i = 0; i < ARRAY_SIZE(rescue_opts); i++) - ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%s%s", - (i ? " " : ""), rescue_opts[i]); - ret += scnprintf(buf + ret, PAGE_SIZE - ret, "\n"); + ret += sysfs_emit_at(buf, ret, "%s%s", (i ? " " : ""), + rescue_opts[i]); + ret += sysfs_emit_at(buf, ret, "\n"); return ret; } BTRFS_ATTR(static_feature, supported_rescue_options, @@ -394,10 +394,10 @@ static ssize_t supported_sectorsizes_show(struct kobject *kobj, /* 4K sector size is also supported with 64K page size */ if (PAGE_SIZE == SZ_64K) - ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%u ", SZ_4K); + ret += sysfs_emit_at(buf, ret, "%u ", SZ_4K); /* Only sectorsize == PAGE_SIZE is now supported */ - ret += scnprintf(buf + ret, PAGE_SIZE - ret, "%lu\n", PAGE_SIZE); + ret += sysfs_emit_at(buf, ret, "%lu\n", PAGE_SIZE); return ret; } @@ -437,7 +437,7 @@ static ssize_t btrfs_discardable_bytes_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = discard_to_fs_info(kobj); - return scnprintf(buf, PAGE_SIZE, "%lld\n", + return sysfs_emit(buf, "%lld\n", atomic64_read(&fs_info->discard_ctl.discardable_bytes)); } BTRFS_ATTR(discard, discardable_bytes, btrfs_discardable_bytes_show); @@ -448,7 +448,7 @@ static ssize_t btrfs_discardable_extents_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = discard_to_fs_info(kobj); - return scnprintf(buf, PAGE_SIZE, "%d\n", + return sysfs_emit(buf, "%d\n", atomic_read(&fs_info->discard_ctl.discardable_extents)); } BTRFS_ATTR(discard, discardable_extents, btrfs_discardable_extents_show); @@ -459,8 +459,8 @@ static ssize_t btrfs_discard_bitmap_bytes_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = discard_to_fs_info(kobj); - return scnprintf(buf, PAGE_SIZE, "%llu\n", - fs_info->discard_ctl.discard_bitmap_bytes); + return sysfs_emit(buf, "%llu\n", + fs_info->discard_ctl.discard_bitmap_bytes); } BTRFS_ATTR(discard, discard_bitmap_bytes, btrfs_discard_bitmap_bytes_show); @@ -470,7 +470,7 @@ static ssize_t btrfs_discard_bytes_saved_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = discard_to_fs_info(kobj); - return scnprintf(buf, PAGE_SIZE, "%lld\n", + return sysfs_emit(buf, "%lld\n", atomic64_read(&fs_info->discard_ctl.discard_bytes_saved)); } BTRFS_ATTR(discard, discard_bytes_saved, btrfs_discard_bytes_saved_show); @@ -481,8 +481,8 @@ static ssize_t btrfs_discard_extent_bytes_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = discard_to_fs_info(kobj); - return scnprintf(buf, PAGE_SIZE, "%llu\n", - fs_info->discard_ctl.discard_extent_bytes); + return sysfs_emit(buf, "%llu\n", + fs_info->discard_ctl.discard_extent_bytes); } BTRFS_ATTR(discard, discard_extent_bytes, btrfs_discard_extent_bytes_show); @@ -492,8 +492,8 @@ static ssize_t btrfs_discard_iops_limit_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = discard_to_fs_info(kobj); - return scnprintf(buf, PAGE_SIZE, "%u\n", - READ_ONCE(fs_info->discard_ctl.iops_limit)); + return sysfs_emit(buf, "%u\n", + READ_ONCE(fs_info->discard_ctl.iops_limit)); } static ssize_t btrfs_discard_iops_limit_store(struct kobject *kobj, @@ -523,8 +523,8 @@ static ssize_t btrfs_discard_kbps_limit_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = discard_to_fs_info(kobj); - return scnprintf(buf, PAGE_SIZE, "%u\n", - READ_ONCE(fs_info->discard_ctl.kbps_limit)); + return sysfs_emit(buf, "%u\n", + READ_ONCE(fs_info->discard_ctl.kbps_limit)); } static ssize_t btrfs_discard_kbps_limit_store(struct kobject *kobj, @@ -553,8 +553,8 @@ static ssize_t btrfs_discard_max_discard_size_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = discard_to_fs_info(kobj); - return scnprintf(buf, PAGE_SIZE, "%llu\n", - READ_ONCE(fs_info->discard_ctl.max_discard_size)); + return sysfs_emit(buf, "%llu\n", + READ_ONCE(fs_info->discard_ctl.max_discard_size)); } static ssize_t btrfs_discard_max_discard_size_store(struct kobject *kobj, @@ -627,7 +627,7 @@ static ssize_t btrfs_show_u64(u64 *value_ptr, spinlock_t *lock, char *buf) val = *value_ptr; if (lock) spin_unlock(lock); - return scnprintf(buf, PAGE_SIZE, "%llu\n", val); + return sysfs_emit(buf, "%llu\n", val); } static ssize_t global_rsv_size_show(struct kobject *kobj, @@ -673,7 +673,7 @@ static ssize_t raid_bytes_show(struct kobject *kobj, val += block_group->used; } up_read(&sinfo->groups_sem); - return scnprintf(buf, PAGE_SIZE, "%llu\n", val); + return sysfs_emit(buf, "%llu\n", val); } /* @@ -771,7 +771,7 @@ static ssize_t btrfs_label_show(struct kobject *kobj, ssize_t ret; spin_lock(&fs_info->super_lock); - ret = scnprintf(buf, PAGE_SIZE, label[0] ? "%s\n" : "%s", label); + ret = sysfs_emit(buf, label[0] ? "%s\n" : "%s", label); spin_unlock(&fs_info->super_lock); return ret; @@ -819,7 +819,7 @@ static ssize_t btrfs_nodesize_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = to_fs_info(kobj); - return scnprintf(buf, PAGE_SIZE, "%u\n", fs_info->super_copy->nodesize); + return sysfs_emit(buf, "%u\n", fs_info->super_copy->nodesize); } BTRFS_ATTR(, nodesize, btrfs_nodesize_show); @@ -829,8 +829,7 @@ static ssize_t btrfs_sectorsize_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = to_fs_info(kobj); - return scnprintf(buf, PAGE_SIZE, "%u\n", - fs_info->super_copy->sectorsize); + return sysfs_emit(buf, "%u\n", fs_info->super_copy->sectorsize); } BTRFS_ATTR(, sectorsize, btrfs_sectorsize_show); @@ -840,7 +839,7 @@ static ssize_t btrfs_clone_alignment_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = to_fs_info(kobj); - return scnprintf(buf, PAGE_SIZE, "%u\n", fs_info->super_copy->sectorsize); + return sysfs_emit(buf, "%u\n", fs_info->super_copy->sectorsize); } BTRFS_ATTR(, clone_alignment, btrfs_clone_alignment_show); @@ -852,7 +851,7 @@ static ssize_t quota_override_show(struct kobject *kobj, int quota_override; quota_override = test_bit(BTRFS_FS_QUOTA_OVERRIDE, &fs_info->flags); - return scnprintf(buf, PAGE_SIZE, "%d\n", quota_override); + return sysfs_emit(buf, "%d\n", quota_override); } static ssize_t quota_override_store(struct kobject *kobj, @@ -890,8 +889,7 @@ static ssize_t btrfs_metadata_uuid_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = to_fs_info(kobj); - return scnprintf(buf, PAGE_SIZE, "%pU\n", - fs_info->fs_devices->metadata_uuid); + return sysfs_emit(buf, "%pU\n", fs_info->fs_devices->metadata_uuid); } BTRFS_ATTR(, metadata_uuid, btrfs_metadata_uuid_show); @@ -902,9 +900,9 @@ static ssize_t btrfs_checksum_show(struct kobject *kobj, struct btrfs_fs_info *fs_info = to_fs_info(kobj); u16 csum_type = btrfs_super_csum_type(fs_info->super_copy); - return scnprintf(buf, PAGE_SIZE, "%s (%s)\n", - btrfs_super_csum_name(csum_type), - crypto_shash_driver_name(fs_info->csum_shash)); + return sysfs_emit(buf, "%s (%s)\n", + btrfs_super_csum_name(csum_type), + crypto_shash_driver_name(fs_info->csum_shash)); } BTRFS_ATTR(, checksum, btrfs_checksum_show); @@ -941,7 +939,7 @@ static ssize_t btrfs_exclusive_operation_show(struct kobject *kobj, str = "UNKNOWN\n"; break; } - return scnprintf(buf, PAGE_SIZE, "%s", str); + return sysfs_emit(buf, "%s", str); } BTRFS_ATTR(, exclusive_operation, btrfs_exclusive_operation_show); @@ -950,7 +948,7 @@ static ssize_t btrfs_generation_show(struct kobject *kobj, { struct btrfs_fs_info *fs_info = to_fs_info(kobj); - return scnprintf(buf, PAGE_SIZE, "%llu\n", fs_info->generation); + return sysfs_emit(buf, "%llu\n", fs_info->generation); } BTRFS_ATTR(, generation, btrfs_generation_show); @@ -1028,8 +1026,8 @@ static ssize_t btrfs_bg_reclaim_threshold_show(struct kobject *kobj, struct btrfs_fs_info *fs_info = to_fs_info(kobj); ssize_t ret; - ret = scnprintf(buf, PAGE_SIZE, "%d\n", - READ_ONCE(fs_info->bg_reclaim_threshold)); + ret = sysfs_emit(buf, "%d\n", + READ_ONCE(fs_info->bg_reclaim_threshold)); return ret; } @@ -1471,7 +1469,7 @@ static ssize_t btrfs_devinfo_in_fs_metadata_show(struct kobject *kobj, val = !!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA, &device->dev_state); - return scnprintf(buf, PAGE_SIZE, "%d\n", val); + return sysfs_emit(buf, "%d\n", val); } BTRFS_ATTR(devid, in_fs_metadata, btrfs_devinfo_in_fs_metadata_show); @@ -1484,7 +1482,7 @@ static ssize_t btrfs_devinfo_missing_show(struct kobject *kobj, val = !!test_bit(BTRFS_DEV_STATE_MISSING, &device->dev_state); - return scnprintf(buf, PAGE_SIZE, "%d\n", val); + return sysfs_emit(buf, "%d\n", val); } BTRFS_ATTR(devid, missing, btrfs_devinfo_missing_show); @@ -1498,7 +1496,7 @@ static ssize_t btrfs_devinfo_replace_target_show(struct kobject *kobj, val = !!test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state); - return scnprintf(buf, PAGE_SIZE, "%d\n", val); + return sysfs_emit(buf, "%d\n", val); } BTRFS_ATTR(devid, replace_target, btrfs_devinfo_replace_target_show); @@ -1509,8 +1507,7 @@ static ssize_t btrfs_devinfo_scrub_speed_max_show(struct kobject *kobj, struct btrfs_device *device = container_of(kobj, struct btrfs_device, devid_kobj); - return scnprintf(buf, PAGE_SIZE, "%llu\n", - READ_ONCE(device->scrub_speed_max)); + return sysfs_emit(buf, "%llu\n", READ_ONCE(device->scrub_speed_max)); } static ssize_t btrfs_devinfo_scrub_speed_max_store(struct kobject *kobj, @@ -1538,7 +1535,7 @@ static ssize_t btrfs_devinfo_writeable_show(struct kobject *kobj, val = !!test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state); - return scnprintf(buf, PAGE_SIZE, "%d\n", val); + return sysfs_emit(buf, "%d\n", val); } BTRFS_ATTR(devid, writeable, btrfs_devinfo_writeable_show); @@ -1549,14 +1546,14 @@ static ssize_t btrfs_devinfo_error_stats_show(struct kobject *kobj, devid_kobj); if (!device->dev_stats_valid) - return scnprintf(buf, PAGE_SIZE, "invalid\n"); + return sysfs_emit(buf, "invalid\n"); /* * Print all at once so we get a snapshot of all values from the same * time. Keep them in sync and in order of definition of * btrfs_dev_stat_values. */ - return scnprintf(buf, PAGE_SIZE, + return sysfs_emit(buf, "write_errs %d\n" "read_errs %d\n" "flush_errs %d\n" -- 2.31.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/2] btrfs: sysfs convert scnprintf and snprintf to use sysfs_emit 2021-10-21 15:31 ` [PATCH v2 1/2] btrfs: sysfs convert scnprintf and snprintf to use sysfs_emit Anand Jain @ 2021-10-21 16:19 ` David Sterba 2021-10-21 16:25 ` David Sterba 2021-10-21 16:27 ` Anand Jain 0 siblings, 2 replies; 13+ messages in thread From: David Sterba @ 2021-10-21 16:19 UTC (permalink / raw) To: Anand Jain; +Cc: linux-btrfs, kernel test robot On Thu, Oct 21, 2021 at 11:31:16PM +0800, Anand Jain wrote: > commit 2efc459d06f1 (sysfs: Add sysfs_emit and sysfs_emit_at to format > sysfs out) Please use the common formatting which is 2efc459d06f1 ("sysfs: Add sysfs_emit and sysfs_emit_at to format sysfs output") in this case. > merged in 5.10 introduced two new functions sysfs_emit() and > sysfs_emit_at() which are aware of the PAGE_SIZE max_limit of the buf. > > Use the above two new functions instead of scnprintf() and snprintf() > in various sysfs show(). > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > Reported-by: kernel test robot <oliver.sang@intel.com> Reported-by should be used only for when the patch is fixing something based on that report. I've applied the patch over the local version 1 but I don't see any fix related to the the crash report, is that correct? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/2] btrfs: sysfs convert scnprintf and snprintf to use sysfs_emit 2021-10-21 16:19 ` David Sterba @ 2021-10-21 16:25 ` David Sterba 2021-10-21 16:27 ` Anand Jain 2021-10-21 16:27 ` Anand Jain 1 sibling, 1 reply; 13+ messages in thread From: David Sterba @ 2021-10-21 16:25 UTC (permalink / raw) To: dsterba, Anand Jain, linux-btrfs, kernel test robot On Thu, Oct 21, 2021 at 06:19:39PM +0200, David Sterba wrote: > On Thu, Oct 21, 2021 at 11:31:16PM +0800, Anand Jain wrote: > I've applied the patch over the local version 1 but I don't see any > fix related to the the crash report, is that correct? I saw the reply, the warning was caused by the removed hunks, so it's sorted now. Patch added to misc-next, thanks. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/2] btrfs: sysfs convert scnprintf and snprintf to use sysfs_emit 2021-10-21 16:25 ` David Sterba @ 2021-10-21 16:27 ` Anand Jain 0 siblings, 0 replies; 13+ messages in thread From: Anand Jain @ 2021-10-21 16:27 UTC (permalink / raw) To: dsterba, linux-btrfs, kernel test robot On 22/10/2021 00:25, David Sterba wrote: > On Thu, Oct 21, 2021 at 06:19:39PM +0200, David Sterba wrote: >> On Thu, Oct 21, 2021 at 11:31:16PM +0800, Anand Jain wrote: >> I've applied the patch over the local version 1 but I don't see any >> fix related to the the crash report, is that correct? > > I saw the reply, the warning was caused by the removed hunks, so it's > sorted now. Patch added to misc-next, thanks. > Oh. Ok. Thanks. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/2] btrfs: sysfs convert scnprintf and snprintf to use sysfs_emit 2021-10-21 16:19 ` David Sterba 2021-10-21 16:25 ` David Sterba @ 2021-10-21 16:27 ` Anand Jain 1 sibling, 0 replies; 13+ messages in thread From: Anand Jain @ 2021-10-21 16:27 UTC (permalink / raw) To: dsterba, linux-btrfs, kernel test robot On 22/10/2021 00:19, David Sterba wrote: > On Thu, Oct 21, 2021 at 11:31:16PM +0800, Anand Jain wrote: >> commit 2efc459d06f1 (sysfs: Add sysfs_emit and sysfs_emit_at to format >> sysfs out) > > Please use the common formatting which is 2efc459d06f1 ("sysfs: Add > sysfs_emit and sysfs_emit_at to format sysfs output") in this case. > Err. My bad. You told me that before. But, for some reason the related config isn't working. So I am doing it manually. I am going to try the config again. >> merged in 5.10 introduced two new functions sysfs_emit() and >> sysfs_emit_at() which are aware of the PAGE_SIZE max_limit of the buf. >> >> Use the above two new functions instead of scnprintf() and snprintf() >> in various sysfs show(). >> >> Signed-off-by: Anand Jain <anand.jain@oracle.com> >> Reported-by: kernel test robot <oliver.sang@intel.com> > > Reported-by should be used only for when the patch is fixing something > based on that report. Right. Ok. > > I've applied the patch over the local version 1 but I don't see any > fix related to the the crash report, is that correct? Right. There is no crash report for the v1. Thanks, Anand > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/2] btrfs: sysfs add devinfo/fsid to retrieve fsid from the device 2021-10-21 15:31 [PATCH v2 0/2] provide fsid in sysfs devinfo Anand Jain 2021-10-21 15:31 ` [PATCH v2 1/2] btrfs: sysfs convert scnprintf and snprintf to use sysfs_emit Anand Jain @ 2021-10-21 15:31 ` Anand Jain 2021-11-16 17:16 ` David Sterba 2021-12-07 18:56 ` David Sterba 2021-11-09 10:09 ` [PATCH v2 0/2] provide fsid in sysfs devinfo Anand Jain 2 siblings, 2 replies; 13+ messages in thread From: Anand Jain @ 2021-10-21 15:31 UTC (permalink / raw) To: linux-btrfs; +Cc: Josef Bacik In the case of the seed device, the fsid can be different from the mounted sprout fsid. The userland has to read the device superblock to know the fsid but, that idea fails if the device is missing. So add a sysfs interface devinfo/<devid>/fsid to show the fsid of the device. For example: $ cd /sys/fs/btrfs/b10b02a5-f9de-4276-b9e8-2bfd09a578a8 $ cat devinfo/1/fsid c44d771f-639d-4df3-99ec-5bc7ad2af93b $ cat devinfo/3/fsid b10b02a5-f9de-4276-b9e8-2bfd09a578a8 Signed-off-by: Anand Jain <anand.jain@oracle.com> Reviewed-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/sysfs.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index 28ff7fce1ac5..591d49deb552 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -1539,6 +1539,16 @@ static ssize_t btrfs_devinfo_writeable_show(struct kobject *kobj, } BTRFS_ATTR(devid, writeable, btrfs_devinfo_writeable_show); +static ssize_t btrfs_devinfo_fsid_show(struct kobject *kobj, + struct kobj_attribute *a, char *buf) +{ + struct btrfs_device *device = container_of(kobj, struct btrfs_device, + devid_kobj); + + return sysfs_emit(buf, "%pU\n", device->fs_devices->fsid); +} +BTRFS_ATTR(devid, fsid, btrfs_devinfo_fsid_show); + static ssize_t btrfs_devinfo_error_stats_show(struct kobject *kobj, struct kobj_attribute *a, char *buf) { @@ -1579,6 +1589,7 @@ static struct attribute *devid_attrs[] = { BTRFS_ATTR_PTR(devid, replace_target), BTRFS_ATTR_PTR(devid, scrub_speed_max), BTRFS_ATTR_PTR(devid, writeable), + BTRFS_ATTR_PTR(devid, fsid), NULL }; ATTRIBUTE_GROUPS(devid); -- 2.31.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] btrfs: sysfs add devinfo/fsid to retrieve fsid from the device 2021-10-21 15:31 ` [PATCH v2 2/2] btrfs: sysfs add devinfo/fsid to retrieve fsid from the device Anand Jain @ 2021-11-16 17:16 ` David Sterba 2021-11-17 3:29 ` Anand Jain 2021-12-07 18:56 ` David Sterba 1 sibling, 1 reply; 13+ messages in thread From: David Sterba @ 2021-11-16 17:16 UTC (permalink / raw) To: Anand Jain; +Cc: linux-btrfs, Josef Bacik On Thu, Oct 21, 2021 at 11:31:17PM +0800, Anand Jain wrote: > In the case of the seed device, the fsid can be different from the mounted > sprout fsid. The userland has to read the device superblock to know the > fsid but, that idea fails if the device is missing. So add a sysfs > interface devinfo/<devid>/fsid to show the fsid of the device. > > For example: > $ cd /sys/fs/btrfs/b10b02a5-f9de-4276-b9e8-2bfd09a578a8 > > $ cat devinfo/1/fsid > c44d771f-639d-4df3-99ec-5bc7ad2af93b > $ cat devinfo/3/fsid > b10b02a5-f9de-4276-b9e8-2bfd09a578a8 From user perspective, it's another fsid, one is in the path, so I'm wondering if it should be named like read_fsid or sprout_fsid or if the seed/sprout information should be put into another directory completely. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] btrfs: sysfs add devinfo/fsid to retrieve fsid from the device 2021-11-16 17:16 ` David Sterba @ 2021-11-17 3:29 ` Anand Jain 2021-12-01 16:56 ` Anand Jain 2021-12-07 18:56 ` David Sterba 0 siblings, 2 replies; 13+ messages in thread From: Anand Jain @ 2021-11-17 3:29 UTC (permalink / raw) To: dsterba, linux-btrfs, Josef Bacik On 17/11/2021 01:16, David Sterba wrote: > On Thu, Oct 21, 2021 at 11:31:17PM +0800, Anand Jain wrote: >> In the case of the seed device, the fsid can be different from the mounted >> sprout fsid. The userland has to read the device superblock to know the >> fsid but, that idea fails if the device is missing. So add a sysfs >> interface devinfo/<devid>/fsid to show the fsid of the device. >> >> For example: >> $ cd /sys/fs/btrfs/b10b02a5-f9de-4276-b9e8-2bfd09a578a8 >> >> $ cat devinfo/1/fsid >> c44d771f-639d-4df3-99ec-5bc7ad2af93b >> $ cat devinfo/3/fsid >> b10b02a5-f9de-4276-b9e8-2bfd09a578a8 > > From user perspective, it's another fsid, one is in the path, so I'm > wondering if it should be named like read_fsid or sprout_fsid or if the > seed/sprout information should be put into another directory completely. I am viewing it as fsid as per the device's sb. This fsid matches with the blkid(8) output. A path to the device's fsid will help to script. So I am not voting for sprout_fsid because it does not exist in the most common non-sprouted fsid. Now to show whether a device is seed or sprout, the user friendly approach will be like for example: /sys/fs/btrfs/<fsid>/devinfo/<devid>/device_state to show all that apply, for example: SEED|SPROUT|READ_ONLY|WRITABLE|ZONED|METADATA_ONLY|\ DATA_ONLY|READ_PREFERRED|REPLACE_TGT|REPLACE_SRC|\ SCRUB_RUNNING However, per kernel general rule of thumb one value per sys-fs file, so it should be for example: /sys/fs/btrfs/<fsid>/devinfo/<devid>/seed 0 /sys/fs/btrfs/<fsid>/devinfo/<devid>/sprout 1 So based on this my patch "btrfs: introduce new device-state read_preferred" in the mailing list already proposed /sys/fs/btrfs/<fsid>/devinfo/<devid>/read_preferred So to summarize, I can rename fsid to read_fsid. And, progressively in a separate patch, we can add <>/seed <>/sprout sysfs files as above. What do you think? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] btrfs: sysfs add devinfo/fsid to retrieve fsid from the device 2021-11-17 3:29 ` Anand Jain @ 2021-12-01 16:56 ` Anand Jain 2021-12-07 18:56 ` David Sterba 1 sibling, 0 replies; 13+ messages in thread From: Anand Jain @ 2021-12-01 16:56 UTC (permalink / raw) To: dsterba; +Cc: Josef Bacik, linux-btrfs David, Gentle ping on this patch? Thanks, Anand On 17/11/2021 11:29, Anand Jain wrote: > > > On 17/11/2021 01:16, David Sterba wrote: >> On Thu, Oct 21, 2021 at 11:31:17PM +0800, Anand Jain wrote: >>> In the case of the seed device, the fsid can be different from the >>> mounted >>> sprout fsid. The userland has to read the device superblock to know the >>> fsid but, that idea fails if the device is missing. So add a sysfs >>> interface devinfo/<devid>/fsid to show the fsid of the device. >>> >>> For example: >>> $ cd /sys/fs/btrfs/b10b02a5-f9de-4276-b9e8-2bfd09a578a8 >>> >>> $ cat devinfo/1/fsid >>> c44d771f-639d-4df3-99ec-5bc7ad2af93b >>> $ cat devinfo/3/fsid >>> b10b02a5-f9de-4276-b9e8-2bfd09a578a8 >> >> From user perspective, it's another fsid, one is in the path, so I'm >> wondering if it should be named like read_fsid or sprout_fsid or if the >> seed/sprout information should be put into another directory completely. > > I am viewing it as fsid as per the device's sb. This fsid matches with > the blkid(8) output. A path to the device's fsid will help to script. > So I am not voting for sprout_fsid because it does not exist in the > most common non-sprouted fsid. > > Now to show whether a device is seed or sprout, the user friendly > approach will be like for example: > /sys/fs/btrfs/<fsid>/devinfo/<devid>/device_state > to show all that apply, for example: > SEED|SPROUT|READ_ONLY|WRITABLE|ZONED|METADATA_ONLY|\ > DATA_ONLY|READ_PREFERRED|REPLACE_TGT|REPLACE_SRC|\ > SCRUB_RUNNING > > However, per kernel general rule of thumb one value per sys-fs file, > so it should be for example: > /sys/fs/btrfs/<fsid>/devinfo/<devid>/seed > 0 > /sys/fs/btrfs/<fsid>/devinfo/<devid>/sprout > 1 > > So based on this my patch "btrfs: introduce new device-state > read_preferred" in the mailing list already proposed > /sys/fs/btrfs/<fsid>/devinfo/<devid>/read_preferred > > So to summarize, I can rename fsid to read_fsid. And, progressively > in a separate patch, we can add <>/seed <>/sprout sysfs files as above. > What do you think? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] btrfs: sysfs add devinfo/fsid to retrieve fsid from the device 2021-11-17 3:29 ` Anand Jain 2021-12-01 16:56 ` Anand Jain @ 2021-12-07 18:56 ` David Sterba 1 sibling, 0 replies; 13+ messages in thread From: David Sterba @ 2021-12-07 18:56 UTC (permalink / raw) To: Anand Jain; +Cc: dsterba, linux-btrfs, Josef Bacik On Wed, Nov 17, 2021 at 11:29:26AM +0800, Anand Jain wrote: > On 17/11/2021 01:16, David Sterba wrote: > > On Thu, Oct 21, 2021 at 11:31:17PM +0800, Anand Jain wrote: > >> In the case of the seed device, the fsid can be different from the mounted > >> sprout fsid. The userland has to read the device superblock to know the > >> fsid but, that idea fails if the device is missing. So add a sysfs > >> interface devinfo/<devid>/fsid to show the fsid of the device. > >> > >> For example: > >> $ cd /sys/fs/btrfs/b10b02a5-f9de-4276-b9e8-2bfd09a578a8 > >> > >> $ cat devinfo/1/fsid > >> c44d771f-639d-4df3-99ec-5bc7ad2af93b > >> $ cat devinfo/3/fsid > >> b10b02a5-f9de-4276-b9e8-2bfd09a578a8 > > > > From user perspective, it's another fsid, one is in the path, so I'm > > wondering if it should be named like read_fsid or sprout_fsid or if the > > seed/sprout information should be put into another directory completely. > > I am viewing it as fsid as per the device's sb. That's a reasonable point. > This fsid matches with > the blkid(8) output. A path to the device's fsid will help to script. > So I am not voting for sprout_fsid because it does not exist in the > most common non-sprouted fsid. Yeah agreed, I've put this to the changelog as it explains the naming. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/2] btrfs: sysfs add devinfo/fsid to retrieve fsid from the device 2021-10-21 15:31 ` [PATCH v2 2/2] btrfs: sysfs add devinfo/fsid to retrieve fsid from the device Anand Jain 2021-11-16 17:16 ` David Sterba @ 2021-12-07 18:56 ` David Sterba 1 sibling, 0 replies; 13+ messages in thread From: David Sterba @ 2021-12-07 18:56 UTC (permalink / raw) To: Anand Jain; +Cc: linux-btrfs, Josef Bacik On Thu, Oct 21, 2021 at 11:31:17PM +0800, Anand Jain wrote: > In the case of the seed device, the fsid can be different from the mounted > sprout fsid. The userland has to read the device superblock to know the > fsid but, that idea fails if the device is missing. So add a sysfs > interface devinfo/<devid>/fsid to show the fsid of the device. > > For example: > $ cd /sys/fs/btrfs/b10b02a5-f9de-4276-b9e8-2bfd09a578a8 > > $ cat devinfo/1/fsid > c44d771f-639d-4df3-99ec-5bc7ad2af93b > $ cat devinfo/3/fsid > b10b02a5-f9de-4276-b9e8-2bfd09a578a8 > > Signed-off-by: Anand Jain <anand.jain@oracle.com> > Reviewed-by: Josef Bacik <josef@toxicpanda.com> Added to misc-next, thanks. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/2] provide fsid in sysfs devinfo 2021-10-21 15:31 [PATCH v2 0/2] provide fsid in sysfs devinfo Anand Jain 2021-10-21 15:31 ` [PATCH v2 1/2] btrfs: sysfs convert scnprintf and snprintf to use sysfs_emit Anand Jain 2021-10-21 15:31 ` [PATCH v2 2/2] btrfs: sysfs add devinfo/fsid to retrieve fsid from the device Anand Jain @ 2021-11-09 10:09 ` Anand Jain 2 siblings, 0 replies; 13+ messages in thread From: Anand Jain @ 2021-11-09 10:09 UTC (permalink / raw) To: David Sterba; +Cc: linux-btrfs David, And this one could also merge? Thanks, Anand On 21/10/21 11:31 pm, Anand Jain wrote: > v2: > Fix sysfs_emit conversion at the two non-sysfs show functions in patch1. > > btrfs-progs tries to read the fsid from the super-block for a missing > device and, it fails. It needs to find out if the device is a seed > device. It does it by comparing the device's fsid with the fsid of the > mounted filesystem. To help this scenario introduce a new sysfs file to > read the fsid from the kernel. > /sys/fs/btrfs/<fsid>/devinfo/<devid>/fsid > > Patch 1 is a cleanup converts scnprtin()f and snprintf() to sysfs_emit() > Patch 2 introduces the new sysfs interface as above > > The other implementation choice is to add another parameter to the > struct btrfs_ioctl_dev_info_args and use BTRFS_IOC_DEV_INFO ioctl. But > then backward kernel compatibility with the newer btrfs-progs is more > complicated. If needed, we can add that too. > > Related btrfs-progs patches: > btrfs-progs: prepare helper device_is_seed > btrfs-progs: read fsid from the sysfs in device_is_seed > > Anand Jain (2): > btrfs: sysfs convert scnprintf and snprintf to use sysfs_emit > btrfs: sysfs add devinfo/fsid to retrieve fsid from the device > > fs/btrfs/sysfs.c | 106 +++++++++++++++++++++++++---------------------- > 1 file changed, 57 insertions(+), 49 deletions(-) > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-12-07 18:56 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-21 15:31 [PATCH v2 0/2] provide fsid in sysfs devinfo Anand Jain 2021-10-21 15:31 ` [PATCH v2 1/2] btrfs: sysfs convert scnprintf and snprintf to use sysfs_emit Anand Jain 2021-10-21 16:19 ` David Sterba 2021-10-21 16:25 ` David Sterba 2021-10-21 16:27 ` Anand Jain 2021-10-21 16:27 ` Anand Jain 2021-10-21 15:31 ` [PATCH v2 2/2] btrfs: sysfs add devinfo/fsid to retrieve fsid from the device Anand Jain 2021-11-16 17:16 ` David Sterba 2021-11-17 3:29 ` Anand Jain 2021-12-01 16:56 ` Anand Jain 2021-12-07 18:56 ` David Sterba 2021-12-07 18:56 ` David Sterba 2021-11-09 10:09 ` [PATCH v2 0/2] provide fsid in sysfs devinfo Anand Jain
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).