Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

btrfs: sysfs: update fs features directory asynchronously

[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>

authored by

Qu Wenruo and committed by
David Sterba
b7625f46 58e36c2a

+27 -23
+3
fs/btrfs/disk-io.c
··· 1914 1914 goto sleep; 1915 1915 } 1916 1916 1917 + if (test_and_clear_bit(BTRFS_FS_FEATURE_CHANGED, &fs_info->flags)) 1918 + btrfs_sysfs_feature_update(fs_info); 1919 + 1917 1920 btrfs_run_delayed_iputs(fs_info); 1918 1921 1919 1922 again = btrfs_clean_one_deleted_snapshot(fs_info);
+4
fs/btrfs/fs.c
··· 24 24 name, flag); 25 25 } 26 26 spin_unlock(&fs_info->super_lock); 27 + set_bit(BTRFS_FS_FEATURE_CHANGED, &fs_info->flags); 27 28 } 28 29 } 29 30 ··· 47 46 name, flag); 48 47 } 49 48 spin_unlock(&fs_info->super_lock); 49 + set_bit(BTRFS_FS_FEATURE_CHANGED, &fs_info->flags); 50 50 } 51 51 } 52 52 ··· 70 68 name, flag); 71 69 } 72 70 spin_unlock(&fs_info->super_lock); 71 + set_bit(BTRFS_FS_FEATURE_CHANGED, &fs_info->flags); 73 72 } 74 73 } 75 74 ··· 93 90 name, flag); 94 91 } 95 92 spin_unlock(&fs_info->super_lock); 93 + set_bit(BTRFS_FS_FEATURE_CHANGED, &fs_info->flags); 96 94 } 97 95 }
+6
fs/btrfs/fs.h
··· 125 125 */ 126 126 BTRFS_FS_NO_OVERCOMMIT, 127 127 128 + /* 129 + * Indicate if we have some features changed, this is mostly for 130 + * cleaner thread to update the sysfs interface. 131 + */ 132 + BTRFS_FS_FEATURE_CHANGED, 133 + 128 134 #if BITS_PER_LONG == 32 129 135 /* Indicate if we have error/warn message printed on 32bit systems */ 130 136 BTRFS_FS_32BIT_ERROR,
+8 -21
fs/btrfs/sysfs.c
··· 2272 2272 * Change per-fs features in /sys/fs/btrfs/UUID/features to match current 2273 2273 * values in superblock. Call after any changes to incompat/compat_ro flags 2274 2274 */ 2275 - void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info, 2276 - u64 bit, enum btrfs_feature_set set) 2275 + void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info) 2277 2276 { 2278 - struct btrfs_fs_devices *fs_devs; 2279 2277 struct kobject *fsid_kobj; 2280 - u64 __maybe_unused features; 2281 - int __maybe_unused ret; 2278 + int ret; 2282 2279 2283 2280 if (!fs_info) 2284 2281 return; 2285 2282 2286 - /* 2287 - * See 14e46e04958df74 and e410e34fad913dd, feature bit updates are not 2288 - * safe when called from some contexts (eg. balance) 2289 - */ 2290 - features = get_features(fs_info, set); 2291 - ASSERT(bit & supported_feature_masks[set]); 2292 - 2293 - fs_devs = fs_info->fs_devices; 2294 - fsid_kobj = &fs_devs->fsid_kobj; 2295 - 2283 + fsid_kobj = &fs_info->fs_devices->fsid_kobj; 2296 2284 if (!fsid_kobj->state_initialized) 2297 2285 return; 2298 2286 2299 - /* 2300 - * FIXME: this is too heavy to update just one value, ideally we'd like 2301 - * to use sysfs_update_group but some refactoring is needed first. 2302 - */ 2303 - sysfs_remove_group(fsid_kobj, &btrfs_feature_attr_group); 2304 - ret = sysfs_create_group(fsid_kobj, &btrfs_feature_attr_group); 2287 + ret = sysfs_update_group(fsid_kobj, &btrfs_feature_attr_group); 2288 + if (ret < 0) 2289 + btrfs_warn(fs_info, 2290 + "failed to update /sys/fs/btrfs/%pU/features: %d", 2291 + fs_info->fs_devices->fsid, ret); 2305 2292 } 2306 2293 2307 2294 int __init btrfs_init_sysfs(void)
+1 -2
fs/btrfs/sysfs.h
··· 19 19 int btrfs_sysfs_add_fsid(struct btrfs_fs_devices *fs_devs); 20 20 void btrfs_sysfs_remove_fsid(struct btrfs_fs_devices *fs_devs); 21 21 void btrfs_sysfs_update_sprout_fsid(struct btrfs_fs_devices *fs_devices); 22 - void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info, 23 - u64 bit, enum btrfs_feature_set set); 22 + void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info); 24 23 void btrfs_kobject_uevent(struct block_device *bdev, enum kobject_action action); 25 24 26 25 int __init btrfs_init_sysfs(void);
+5
fs/btrfs/transaction.c
··· 2464 2464 wake_up(&fs_info->transaction_wait); 2465 2465 btrfs_trans_state_lockdep_release(fs_info, BTRFS_LOCKDEP_TRANS_UNBLOCKED); 2466 2466 2467 + /* If we have features changed, wake up the cleaner to update sysfs. */ 2468 + if (test_bit(BTRFS_FS_FEATURE_CHANGED, &fs_info->flags) && 2469 + fs_info->cleaner_kthread) 2470 + wake_up_process(fs_info->cleaner_kthread); 2471 + 2467 2472 ret = btrfs_write_and_wait_transaction(trans); 2468 2473 if (ret) { 2469 2474 btrfs_handle_fs_error(fs_info, ret,