btrfs: remove runtime tweakable feature sysfs interface

There are 2 features that are marked runtime tweakable inside
/sys/fs/btrfs/features/

- acl
  Which is a mount option, and it will not show up in
  /sys/fs/btrfs/<fsid>/features/ directory anyway.

- extended_iref
  This feature can only be enabled, but not disabled at runtime.
  Furthermore it's already the default behavior since 3.12.

  So it means this feature is always enabled and cannot be disabled for
  modern btrfs.

So there is no need to maintain the ability to modify btrfs' runtime
features through sysfs.

And furthermore, the existing btrfs_feature_attr_store() is race-prone,
it relies on fs_info->transaction_kthread, but our sysfs interfaces are
enabled before transaction_kthread.

Meaning at mount time a sysfs write can trigger NULL pointer dereference
if the transaction_kthread is not yet initialized.
The opposite is also possible during unmount.

Thankfully that race is not possible in the real world, as the only
supported feature is already enabled.

But it also means we do not really need to keep the race-prone
infrastructure, so just remove it completely, and make the per-module
and per-mount features files to be completely read-only.

Even with the sysfs tweakable features removed, we can still enable
extended_iref feature through ioctl.

Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 1df6340a71234fd552cfaafb4a405338fb6331ac..c5bb1c7eac6afeed148210973c5c5f2ee4b69f56 100644 (file)
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -83,8 +83,7 @@ struct raid_kobject {
 #define BTRFS_FEAT_ATTR(_name, _feature_set, _feature_prefix, _feature_bit)  \
 static struct btrfs_feature_attr btrfs_attr_features_##_name = {            \
        .kobj_attr = __INIT_KOBJ_ATTR(_name, S_IRUGO,                        \
-                                     btrfs_feature_attr_show,               \
-                                     btrfs_feature_attr_store),             \
+                                     btrfs_feature_attr_show, NULL),        \
        .feature_set    = _feature_set,                                      \
        .feature_bit    = _feature_prefix ##_## _feature_bit,                \
 }
@@ -130,130 +129,20 @@ static u64 get_features(struct btrfs_fs_info *fs_info,
                return btrfs_super_incompat_flags(disk_super);
 }
 
-static void set_features(struct btrfs_fs_info *fs_info,
-                        enum btrfs_feature_set set, u64 features)
-{
-       struct btrfs_super_block *disk_super = fs_info->super_copy;
-       if (set == FEAT_COMPAT)
-               btrfs_set_super_compat_flags(disk_super, features);
-       else if (set == FEAT_COMPAT_RO)
-               btrfs_set_super_compat_ro_flags(disk_super, features);
-       else
-               btrfs_set_super_incompat_flags(disk_super, features);
-}
-
-static int can_modify_feature(struct btrfs_feature_attr *fa)
-{
-       int val = 0;
-       u64 set, clear;
-       switch (fa->feature_set) {
-       case FEAT_COMPAT:
-               set = BTRFS_FEATURE_COMPAT_SAFE_SET;
-               clear = BTRFS_FEATURE_COMPAT_SAFE_CLEAR;
-               break;
-       case FEAT_COMPAT_RO:
-               set = BTRFS_FEATURE_COMPAT_RO_SAFE_SET;
-               clear = BTRFS_FEATURE_COMPAT_RO_SAFE_CLEAR;
-               break;
-       case FEAT_INCOMPAT:
-               set = BTRFS_FEATURE_INCOMPAT_SAFE_SET;
-               clear = BTRFS_FEATURE_INCOMPAT_SAFE_CLEAR;
-               break;
-       default:
-               btrfs_warn(NULL, "sysfs: unknown feature set %d", fa->feature_set);
-               return 0;
-       }
-
-       if (set & fa->feature_bit)
-               val |= 1;
-       if (clear & fa->feature_bit)
-               val |= 2;
-
-       return val;
-}
-
 static ssize_t btrfs_feature_attr_show(struct kobject *kobj,
                                       struct kobj_attribute *a, char *buf)
 {
        int val = 0;
        struct btrfs_fs_info *fs_info = to_fs_info(kobj);
        struct btrfs_feature_attr *fa = to_btrfs_feature_attr(a);
+
        if (fs_info) {
                u64 features = get_features(fs_info, fa->feature_set);
                if (features & fa->feature_bit)
                        val = 1;
-       } else
-               val = can_modify_feature(fa);
-
-       return sysfs_emit(buf, "%d\n", val);
-}
-
-static ssize_t btrfs_feature_attr_store(struct kobject *kobj,
-                                       struct kobj_attribute *a,
-                                       const char *buf, size_t count)
-{
-       struct btrfs_fs_info *fs_info;
-       struct btrfs_feature_attr *fa = to_btrfs_feature_attr(a);
-       u64 features, set, clear;
-       unsigned long val;
-       int ret;
-
-       fs_info = to_fs_info(kobj);
-       if (!fs_info)
-               return -EPERM;
-
-       if (sb_rdonly(fs_info->sb))
-               return -EROFS;
-
-       ret = kstrtoul(skip_spaces(buf), 0, &val);
-       if (ret)
-               return ret;
-
-       if (fa->feature_set == FEAT_COMPAT) {
-               set = BTRFS_FEATURE_COMPAT_SAFE_SET;
-               clear = BTRFS_FEATURE_COMPAT_SAFE_CLEAR;
-       } else if (fa->feature_set == FEAT_COMPAT_RO) {
-               set = BTRFS_FEATURE_COMPAT_RO_SAFE_SET;
-               clear = BTRFS_FEATURE_COMPAT_RO_SAFE_CLEAR;
-       } else {
-               set = BTRFS_FEATURE_INCOMPAT_SAFE_SET;
-               clear = BTRFS_FEATURE_INCOMPAT_SAFE_CLEAR;
        }
 
-       features = get_features(fs_info, fa->feature_set);
-
-       /* Nothing to do */
-       if ((val && (features & fa->feature_bit)) ||
-           (!val && !(features & fa->feature_bit)))
-               return count;
-
-       if ((val && !(set & fa->feature_bit)) ||
-           (!val && !(clear & fa->feature_bit))) {
-               btrfs_info(fs_info,
-                       "%sabling feature %s on mounted fs is not supported.",
-                       val ? "En" : "Dis", fa->kobj_attr.attr.name);
-               return -EPERM;
-       }
-
-       btrfs_info(fs_info, "%s %s feature flag",
-                  val ? "Setting" : "Clearing", fa->kobj_attr.attr.name);
-
-       spin_lock(&fs_info->super_lock);
-       features = get_features(fs_info, fa->feature_set);
-       if (val)
-               features |= fa->feature_bit;
-       else
-               features &= ~fa->feature_bit;
-       set_features(fs_info, fa->feature_set, features);
-       spin_unlock(&fs_info->super_lock);
-
-       /*
-        * We don't want to do full transaction commit from inside sysfs
-        */
-       set_bit(BTRFS_FS_NEED_TRANS_COMMIT, &fs_info->flags);
-       wake_up_process(fs_info->transaction_kthread);
-
-       return count;
+       return sysfs_emit(buf, "%d\n", val);
 }
 
 static umode_t btrfs_feature_visible(struct kobject *kobj,
@@ -269,9 +158,7 @@ static umode_t btrfs_feature_visible(struct kobject *kobj,
                fa = attr_to_btrfs_feature_attr(attr);
                features = get_features(fs_info, fa->feature_set);
 
-               if (can_modify_feature(fa))
-                       mode |= S_IWUSR;
-               else if (!(features & fa->feature_bit))
+               if (!(features & fa->feature_bit))
                        mode = 0;
        }