From: Zygo Blaxell Date: Sat, 19 Sep 2026 16:30:26 +0000 (-0400) Subject: btrfs: stripe_alloc: drop the mount options, the property is the interface X-Git-Url: http://git.hungrycats.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=refs%2Fheads%2F6.18%2Ftopics%2Fstripe-alloc;p=linux btrfs: stripe_alloc: drop the mount options, the property is the interface stripe_alloc and stripe_alloc_allow_rmw were mount options and filesystem properties both. Upstream will not take new mount options for this, and the two interfaces have to be kept in step by hand -- a bug class this series has already paid for twice, once when the property skipped the mixed-block-group refusal the option made, and once when the two disagreed about a cache-less filesystem. Remove the options. Nothing is lost: - btrfs_stripe_alloc_check_support(), which the property calls, already refuses everything btrfs_check_mountopts() refused -- v1 space cache, zoned, mixed block groups -- and one thing more, a v1 cache still present on disk from an earlier mount. So the mount-time block was the weaker of the two checks, not the stronger. - The runtime state is untouched. BTRFS_MOUNT_STRIPE_ALLOC and fs_info->stripe_rmw_opt stay; btrfs_enable_stripe_alloc() and the stripe_alloc_allow_rmw property already own them, and every btrfs_test_opt(fs_info, STRIPE_ALLOC) in the allocator is unchanged. - btrfs_parse_stripe_rmw() and btrfs_show_stripe_rmw() keep their other callers in props.c and block-group.c. The mount-time report collapses to one source, so it no longer has to say which of two things turned the policy on, and btrfs_fill_super() no longer has to sample the flag before open_ctree() to tell them apart. The remount path loses its report with the option: a remount can no longer change the policy, and the property announces its own changes. Enabling the policy is now: mount, then setfattr -n btrfs.stripe_alloc -v 1 /mnt on the top-level root directory, which persists across mounts. The "does not cover raid56 system chunks" warning moves from btrfs_read_block_groups() to btrfs_enable_stripe_alloc(): block groups are read before the root directory's properties apply, so with the option gone the flag was always clear at the old call site and the warning could never fire (bg-gate-test.sh caught it). Assisted-by: Claude:claude-opus-5[1m] Assisted-by: Claude:claude-fable-5-1 --- diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c index cc44e400fb3ef..828969daa2675 100644 --- a/fs/btrfs/block-group.c +++ b/fs/btrfs/block-group.c @@ -2115,6 +2115,33 @@ static void stripe_alloc_sweep_groups(struct btrfs_fs_info *fs_info, bool arm) } } +/* + * stripe_alloc covers raid56 *data* block groups only. A filesystem whose + * metadata is also raid56 keeps the write hole there. The data guarantee is + * easy to read as a filesystem-wide one, so say once, when the policy turns + * on, what is not covered. That is btrfs_enable_stripe_alloc(): at mount the + * property applies after the block groups are read, so a report from + * btrfs_read_block_groups() would find the flag still clear. (Mixed block + * groups cannot get here: the property refuses them.) + */ +static void warn_stripe_alloc_uncovered(struct btrfs_fs_info *info) +{ + if (!btrfs_test_opt(info, STRIPE_ALLOC)) + return; + + /* + * raid56 metadata is covered now that the policy follows the chunk + * profile. System chunks are not: btrfs_is_stripe_meta_bg() tests + * the METADATA flag, and a system chunk does not carry it, so a + * raid56 system chunk keeps the legacy read-modify-write and its + * write hole. They are small and rewritten rarely, but say so + * rather than let the earlier "covered" message imply otherwise. + */ + if (info->avail_system_alloc_bits & BTRFS_BLOCK_GROUP_RAID56_MASK) + btrfs_warn(info, +"stripe_alloc does not cover raid56 system chunks: the write hole remains for them, consider converting to raid1c3/raid1c4"); +} + /* * Turn on stripe-exclusive allocation, from the "stripe_alloc" filesystem * property on the top-level subvolume's root directory. Runs during mount @@ -2170,6 +2197,7 @@ int btrfs_enable_stripe_alloc(struct btrfs_fs_info *fs_info) if (live) set_bit(BTRFS_FS_STRIPE_ALLOC_ENABLING, &fs_info->flags); btrfs_set_opt(fs_info->mount_opt, STRIPE_ALLOC); + warn_stripe_alloc_uncovered(fs_info); /* * Arm stripe_unusable accounting for raid56 data groups whose caches * loaded before the policy was enabled: block groups are read before @@ -5352,31 +5380,6 @@ static int fill_dummy_bgs(struct btrfs_fs_info *fs_info) return ret; } -/* - * stripe_alloc covers raid56 *data* block groups only. A filesystem whose - * metadata is also raid56 keeps the write hole there. The data guarantee is - * easy to read as a filesystem-wide one, so say once at mount what is not - * covered. (Mixed block groups cannot get here: the mount is refused, see - * btrfs_check_mountopts().) - */ -static void warn_stripe_alloc_uncovered(struct btrfs_fs_info *info) -{ - if (!btrfs_test_opt(info, STRIPE_ALLOC)) - return; - - /* - * raid56 metadata is covered now that the policy follows the chunk - * profile. System chunks are not: btrfs_is_stripe_meta_bg() tests - * the METADATA flag, and a system chunk does not carry it, so a - * raid56 system chunk keeps the legacy read-modify-write and its - * write hole. They are small and rewritten rarely, but say so - * rather than let the earlier "covered" message imply otherwise. - */ - if (info->avail_system_alloc_bits & BTRFS_BLOCK_GROUP_RAID56_MASK) - btrfs_warn(info, -"stripe_alloc does not cover raid56 system chunks: the write hole remains for them, consider converting to raid1c3/raid1c4"); -} - int btrfs_read_block_groups(struct btrfs_fs_info *info) { struct btrfs_root *root = btrfs_block_group_root(info); @@ -5473,7 +5476,6 @@ int btrfs_read_block_groups(struct btrfs_fs_info *info) inc_block_group_ro(cache, 1); } - warn_stripe_alloc_uncovered(info); /* * The mount options rule out space_cache=v1, but the cache format can * only be changed at mount, so a filesystem can arrive here with cache diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c index dfd51fea28f3c..09eaec5d5c637 100644 --- a/fs/btrfs/super.c +++ b/fs/btrfs/super.c @@ -82,7 +82,6 @@ struct btrfs_fs_context { u32 commit_interval; u32 metadata_ratio; u32 thread_pool_size; - u32 stripe_rmw_opt; unsigned long long mount_opt; unsigned long compress_type:4; int compress_level; @@ -115,8 +114,6 @@ enum { Opt_rescan_uuid_tree, Opt_skip_balance, Opt_space_cache, - Opt_stripe_alloc, - Opt_stripe_alloc_allow_rmw, Opt_space_cache_version, Opt_ssd, Opt_ssd_spread, @@ -239,9 +236,6 @@ static const struct fs_parameter_spec btrfs_fs_parameters[] = { fsparam_flag("skip_balance", Opt_skip_balance), fsparam_flag_no("space_cache", Opt_space_cache), fsparam_enum("space_cache", Opt_space_cache_version, btrfs_parameter_space_cache), - fsparam_flag_no("stripe_alloc", Opt_stripe_alloc), - fsparam_string("stripe_alloc_allow_rmw", - Opt_stripe_alloc_allow_rmw), fsparam_flag_no("ssd", Opt_ssd), fsparam_flag_no("ssd_spread", Opt_ssd_spread), fsparam_string("subvol", Opt_subvol), @@ -487,22 +481,6 @@ static int btrfs_parse_param(struct fs_context *fc, struct fs_parameter *param) else btrfs_set_opt(ctx->mount_opt, FLUSHONCOMMIT); break; - case Opt_stripe_alloc: - if (result.negated) - btrfs_clear_opt(ctx->mount_opt, STRIPE_ALLOC); - else - btrfs_set_opt(ctx->mount_opt, STRIPE_ALLOC); - break; - case Opt_stripe_alloc_allow_rmw: - if (btrfs_parse_stripe_rmw(param->string, - strlen(param->string), - &ctx->stripe_rmw_opt)) { - btrfs_err(NULL, - "invalid stripe_alloc_allow_rmw value %s", - param->string); - return -EINVAL; - } - break; case Opt_ratio: ctx->metadata_ratio = result.uint_32; break; @@ -731,33 +709,6 @@ bool btrfs_check_options(const struct btrfs_fs_info *info, ret = false; } - if (btrfs_raw_test_opt(*mount_opt, STRIPE_ALLOC)) { - /* - * Only v1 is a problem: its cache inode is nodatacow and - * preallocated, so the cache is overwritten in place during - * commit -- a sub-stripe write into a data block group, into - * whatever committed stripes the cache occupies, and nodatasum - * so nothing would show the damage afterwards. The free space - * tree and no cache at all are both fine; nothing here needs - * either one. - */ - if (btrfs_raw_test_opt(*mount_opt, SPACE_CACHE)) { - btrfs_err(info, - "stripe_alloc is not supported with space_cache=v1"); - ret = false; - } - if (btrfs_is_zoned(info)) { - btrfs_err(info, - "stripe_alloc is not supported on zoned filesystems"); - ret = false; - } - if (btrfs_fs_incompat(info, MIXED_GROUPS)) { - btrfs_err(info, - "stripe_alloc is not supported on mixed block groups"); - ret = false; - } - } - if (btrfs_check_mountopts_zoned(info, mount_opt)) ret = false; @@ -1003,25 +954,20 @@ static int get_default_subvol_objectid(struct btrfs_fs_info *fs_info, u64 *objec } /* - * State whether stripe-exclusive allocation is on, and which of the two things - * that can turn it on did. + * State whether stripe-exclusive allocation is on. * - * Neither path said so before. The mount option sets the flag while - * btrfs_emit_options() has no entry to print for it, and the - * btrfs.stripe_alloc property applies later still, when the root directory's - * inode is read. So nothing in the log distinguished a filesystem running the - * policy from one that was not: silence meant "off" and "on via the mount - * option" equally well, and the only way to answer the question was - * /proc/mounts on a live system -- no use at all when reading a log after a - * crash. Say it once, whatever turned it on. + * Nothing said so before. The btrfs.stripe_alloc property applies while the + * root directory's inode is read, and no line went to the log, so silence + * meant "off" and "on" equally well and the only way to answer the question + * was to look at a live filesystem -- no use at all when reading a log after a + * crash. Say it once at mount. */ -static void btrfs_emit_stripe_alloc_state(struct btrfs_fs_info *info, - bool from_option) +static void btrfs_emit_stripe_alloc_state(struct btrfs_fs_info *info) { if (!btrfs_test_opt(info, STRIPE_ALLOC)) return; - btrfs_info(info, "using stripe-exclusive allocation for raid56 data (%s)", - from_option ? "mount option" : "filesystem property"); + btrfs_info(info, + "using stripe-exclusive allocation for raid56 data (filesystem property)"); } static int btrfs_fill_super(struct super_block *sb, @@ -1029,7 +975,6 @@ static int btrfs_fill_super(struct super_block *sb, { struct btrfs_inode *inode; struct btrfs_fs_info *fs_info = btrfs_sb(sb); - bool stripe_alloc_from_option; int ret; sb->s_maxbytes = MAX_LFS_FILESIZE; @@ -1050,15 +995,6 @@ static int btrfs_fill_super(struct super_block *sb, return ret; } - /* - * Sample what the mount option asked for before open_ctree(): the mount - * context has already been copied into fs_info by now, and open_ctree() - * goes far enough into the mount to read the root directory's inode and - * apply btrfs.stripe_alloc from it. Sampling afterwards would report - * every property-enabled filesystem as an option-enabled one. - */ - stripe_alloc_from_option = btrfs_test_opt(fs_info, STRIPE_ALLOC); - ret = open_ctree(sb, fs_devices); if (ret) { btrfs_err(fs_info, "open_ctree failed: %d", ret); @@ -1074,7 +1010,7 @@ static int btrfs_fill_super(struct super_block *sb, goto fail_close; } - btrfs_emit_stripe_alloc_state(fs_info, stripe_alloc_from_option); + btrfs_emit_stripe_alloc_state(fs_info); sb->s_root = d_make_root(&inode->vfs_inode); if (!sb->s_root) { @@ -1191,12 +1127,6 @@ static int btrfs_show_options(struct seq_file *seq, struct dentry *dentry) print_rescue_option(seq, "ignoresuperflags", &printed); if (btrfs_test_opt(info, FLUSHONCOMMIT)) seq_puts(seq, ",flushoncommit"); - if (btrfs_test_opt(info, STRIPE_ALLOC)) - seq_puts(seq, ",stripe_alloc"); - if (info->stripe_rmw_opt) { - seq_puts(seq, ",stripe_alloc_allow_rmw="); - btrfs_show_stripe_rmw(seq, info->stripe_rmw_opt); - } if (btrfs_test_opt(info, DISCARD_SYNC)) seq_puts(seq, ",discard"); if (btrfs_test_opt(info, DISCARD_ASYNC)) @@ -1498,7 +1428,6 @@ static void btrfs_ctx_to_info(struct btrfs_fs_info *fs_info, struct btrfs_fs_con fs_info->max_inline = ctx->max_inline; fs_info->commit_interval = ctx->commit_interval; fs_info->metadata_ratio = ctx->metadata_ratio; - fs_info->stripe_rmw_opt = ctx->stripe_rmw_opt; fs_info->thread_pool_size = ctx->thread_pool_size; fs_info->mount_opt = ctx->mount_opt; fs_info->compress_type = ctx->compress_type; @@ -1670,18 +1599,6 @@ static int btrfs_reconfigure(struct fs_context *fc) fc->sb_flags_mask |= SB_POSIXACL; btrfs_emit_options(fs_info, &old_ctx); - /* - * A remount can only change this through the option; the property path - * reports itself. Say so only when it actually changed. - */ - if (!btrfs_raw_test_opt(old_ctx.mount_opt, STRIPE_ALLOC) != - !btrfs_test_opt(fs_info, STRIPE_ALLOC)) { - if (btrfs_test_opt(fs_info, STRIPE_ALLOC)) - btrfs_emit_stripe_alloc_state(fs_info, true); - else - btrfs_info(fs_info, - "stripe-exclusive allocation for raid56 data is off"); - } wake_up_process(fs_info->transaction_kthread); btrfs_remount_cleanup(fs_info, old_ctx.mount_opt); btrfs_clear_oneshot_options(fs_info);