btrfs: sysfs: update fs features directory asynchronously
authorQu Wenruo <wqu@suse.com>
Fri, 13 Jan 2023 11:11:39 +0000 (19:11 +0800)
committerDavid Sterba <dsterba@suse.com>
Mon, 13 Feb 2023 16:50:35 +0000 (17:50 +0100)
[BUG]
Since the introduction of per-fs feature sysfs interface
(/sys/fs/btrfs/<UUID>/features/), the content of that directory is never
updated.

Thus for the following case, that directory will not show the new
features like RAID56:

  # mkfs.btrfs -f $dev1 $dev2 $dev3
  # mount $dev1 $mnt
  # btrfs balance start -f -mconvert=raid5 $mnt
  # ls /sys/fs/btrfs/$uuid/features/
  extended_iref  free_space_tree  no_holes  skinny_metadata

While after unmount and mount, we got the correct features:

  # umount $mnt
  # mount $dev1 $mnt
  # ls /sys/fs/btrfs/$uuid/features/
  extended_iref  free_space_tree  no_holes  raid56 skinny_metadata

[CAUSE]
Because we never really try to update the content of per-fs features/
directory.

We had an attempt to update the features directory dynamically in commit
14e46e04958d ("btrfs: synchronize incompat feature bits with sysfs
files"), but unfortunately it get reverted in commit e410e34fad91
("Revert "btrfs: synchronize incompat feature bits with sysfs files"").
The problem in the original patch is, in the context of
btrfs_create_chunk(), we can not afford to update the sysfs group.

The exported but never utilized function, btrfs_sysfs_feature_update()
is the leftover of such attempt.  As even if we go sysfs_update_group(),
new files will need extra memory allocation, and we have no way to
specify the sysfs update to go GFP_NOFS.

[FIX]
This patch will address the old problem by doing asynchronous sysfs
update in the cleaner thread.

This involves the following changes:

- Make __btrfs_(set|clear)_fs_(incompat|compat_ro) helpers to set
  BTRFS_FS_FEATURE_CHANGED flag when needed

- Update btrfs_sysfs_feature_update() to use sysfs_update_group()
  And drop unnecessary arguments.

- Call btrfs_sysfs_feature_update() in cleaner_kthread
  If we have the BTRFS_FS_FEATURE_CHANGED flag set.

- Wake up cleaner_kthread in btrfs_commit_transaction if we have
  BTRFS_FS_FEATURE_CHANGED flag

By this, all the previously dangerous call sites like
btrfs_create_chunk() need no new changes, as above helpers would
have already set the BTRFS_FS_FEATURE_CHANGED flag.

The real work happens at cleaner_kthread, thus we pay the cost of
delaying the update to sysfs directory, but the delayed time should be
small enough that end user can not distinguish though it might get
delayed if the cleaner thread is busy with removing subvolumes or
defrag.

CC: stable@vger.kernel.org # 4.14+
Reviewed-by: Anand Jain <anand.jain@oracle.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
fs/btrfs/disk-io.c
fs/btrfs/fs.c
fs/btrfs/fs.h
fs/btrfs/sysfs.c
fs/btrfs/sysfs.h
fs/btrfs/transaction.c

index 7586a8e9b718925a290fcf2c65f6af9bd228b087..a6f89ac1c0865c91ee146c403ac85d2a43e21dee 100644 (file)
@@ -1914,6 +1914,9 @@ static int cleaner_kthread(void *arg)
                        goto sleep;
                }
 
+               if (test_and_clear_bit(BTRFS_FS_FEATURE_CHANGED, &fs_info->flags))
+                       btrfs_sysfs_feature_update(fs_info);
+
                btrfs_run_delayed_iputs(fs_info);
 
                again = btrfs_clean_one_deleted_snapshot(fs_info);
index 5553e1f8afe8e9e46c1f88c4748400da15e53f30..31c1648bc0b4692243a2750f5b55ccc0a4e1591e 100644 (file)
@@ -24,6 +24,7 @@ void __btrfs_set_fs_incompat(struct btrfs_fs_info *fs_info, u64 flag,
                                name, flag);
                }
                spin_unlock(&fs_info->super_lock);
+               set_bit(BTRFS_FS_FEATURE_CHANGED, &fs_info->flags);
        }
 }
 
@@ -46,6 +47,7 @@ void __btrfs_clear_fs_incompat(struct btrfs_fs_info *fs_info, u64 flag,
                                name, flag);
                }
                spin_unlock(&fs_info->super_lock);
+               set_bit(BTRFS_FS_FEATURE_CHANGED, &fs_info->flags);
        }
 }
 
@@ -68,6 +70,7 @@ void __btrfs_set_fs_compat_ro(struct btrfs_fs_info *fs_info, u64 flag,
                                name, flag);
                }
                spin_unlock(&fs_info->super_lock);
+               set_bit(BTRFS_FS_FEATURE_CHANGED, &fs_info->flags);
        }
 }
 
@@ -90,5 +93,6 @@ void __btrfs_clear_fs_compat_ro(struct btrfs_fs_info *fs_info, u64 flag,
                                name, flag);
                }
                spin_unlock(&fs_info->super_lock);
+               set_bit(BTRFS_FS_FEATURE_CHANGED, &fs_info->flags);
        }
 }
index 37b86acfcbcf880ded4a2fbed2407a0e6f905982..3d8156fc8523f253ba33464a8e259528ef7b8fee 100644 (file)
@@ -125,6 +125,12 @@ enum {
         */
        BTRFS_FS_NO_OVERCOMMIT,
 
+       /*
+        * Indicate if we have some features changed, this is mostly for
+        * cleaner thread to update the sysfs interface.
+        */
+       BTRFS_FS_FEATURE_CHANGED,
+
 #if BITS_PER_LONG == 32
        /* Indicate if we have error/warn message printed on 32bit systems */
        BTRFS_FS_32BIT_ERROR,
index 45615ce3649887e601f448b4cd42abcdccdf6e9d..108aa38761860ba92989c73821371c6a996eeef1 100644 (file)
@@ -2272,36 +2272,23 @@ void btrfs_sysfs_del_one_qgroup(struct btrfs_fs_info *fs_info,
  * Change per-fs features in /sys/fs/btrfs/UUID/features to match current
  * values in superblock. Call after any changes to incompat/compat_ro flags
  */
-void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
-               u64 bit, enum btrfs_feature_set set)
+void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info)
 {
-       struct btrfs_fs_devices *fs_devs;
        struct kobject *fsid_kobj;
-       u64 __maybe_unused features;
-       int __maybe_unused ret;
+       int ret;
 
        if (!fs_info)
                return;
 
-       /*
-        * See 14e46e04958df74 and e410e34fad913dd, feature bit updates are not
-        * safe when called from some contexts (eg. balance)
-        */
-       features = get_features(fs_info, set);
-       ASSERT(bit & supported_feature_masks[set]);
-
-       fs_devs = fs_info->fs_devices;
-       fsid_kobj = &fs_devs->fsid_kobj;
-
+       fsid_kobj = &fs_info->fs_devices->fsid_kobj;
        if (!fsid_kobj->state_initialized)
                return;
 
-       /*
-        * FIXME: this is too heavy to update just one value, ideally we'd like
-        * to use sysfs_update_group but some refactoring is needed first.
-        */
-       sysfs_remove_group(fsid_kobj, &btrfs_feature_attr_group);
-       ret = sysfs_create_group(fsid_kobj, &btrfs_feature_attr_group);
+       ret = sysfs_update_group(fsid_kobj, &btrfs_feature_attr_group);
+       if (ret < 0)
+               btrfs_warn(fs_info,
+                          "failed to update /sys/fs/btrfs/%pU/features: %d",
+                          fs_info->fs_devices->fsid, ret);
 }
 
 int __init btrfs_init_sysfs(void)
index bacef43f72672ba594420dcf65c8fd17d27f563d..86c7eef128731ea5aa88efefb8a43d251d8fad34 100644 (file)
@@ -19,8 +19,7 @@ void btrfs_sysfs_remove_device(struct btrfs_device *device);
 int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs);
 void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs);
 void btrfs_sysfs_update_sprout_fsid(struct btrfs_fs_devices *fs_devices);
-void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
-               u64 bit, enum btrfs_feature_set set);
+void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info);
 void btrfs_kobject_uevent(struct block_device *bdev, enum kobject_action action);
 
 int __init btrfs_init_sysfs(void);
index 528efe559866b814497af4567f3bcf71c36a6b0d..18329ebcb1cbffa0e3c98b9eaea654ec39529662 100644 (file)
@@ -2464,6 +2464,11 @@ int btrfs_commit_transaction(struct btrfs_trans_handle *trans)
        wake_up(&fs_info->transaction_wait);
        btrfs_trans_state_lockdep_release(fs_info, BTRFS_LOCKDEP_TRANS_UNBLOCKED);
 
+       /* If we have features changed, wake up the cleaner to update sysfs. */
+       if (test_bit(BTRFS_FS_FEATURE_CHANGED, &fs_info->flags) &&
+           fs_info->cleaner_kthread)
+               wake_up_process(fs_info->cleaner_kthread);
+
        ret = btrfs_write_and_wait_transaction(trans);
        if (ret) {
                btrfs_handle_fs_error(fs_info, ret,