On 19.08.21 г. 12:08, Nikolay Borisov wrote:

On 19.08.21 г. 2:48, David Sterba wrote:

quoted

On Tue, Aug 10, 2021 at 08:30:51AM +0800, Anand Jain wrote:

quoted

  As in the proposed patch which adds a table to figure out the correct 
table to add the attribute, adding to the 
btrfs_supported_static_feature_attrs attribute list will add only to 
/sys/fs/btrfs/features, however adding the attribute to 
btrfs_supported_feature_attrs adds to both /sys/fs/btrfs/features and 
/sys/fs/btrfs/uuid/features.

I can't see it in the code right now, but that would mean that eg. zoned
would show up in static features once such filesystem is mounted. And
that does not happen or I'm missing something.

This happens because when initialising sysfs we create the
btrfs_feature_attr_group which contains btrfs_supported_feature_attrs
attributes, which have
#ifdef CONFIG_BLK_DEV_ZONED

         BTRFS_FEAT_ATTR_PTR(zoned),

#endif

Subsequently you define the static feature via
BTRFS_ATTR(static_feature, zoned, zoned_show);

and finally we call:

ret = sysfs_merge_group(&btrfs_kset->kobj,
&btrfs_static_feature_attr_group);


Which merges the static feature to the feature group. That's why the
message about duplication. So one of the 2 definitions needs to go.

Looking at the maze that btrfs' sysfs code is it seems the decision
which of the two sets to use when defining a feature is whether it can
be enabled or not at runtime. I think for zoned block device we can't
really disable the support at runtime? If so then it needs to only be
defined as a static feature? Just because the kernel supports it doesn't
necessarily mean it's going to be used if the underlying device is not
zoned, right, it should depend on the runtime characteristics of the
underlying device?