On thu, 11 Apr 2013 16:29:48 +0200, Jan Schmidt wrote:
> On Thu, April 11, 2013 at 12:35 (+0200), Miao Xie wrote:
>> Now, we set incompat flag EXTEND_IREF when we actually need insert a
>> extend inode reference, not when making a fs. But some users may hope
>> that the fs still can be mounted on the old kernel, and don't hope we
>> insert any extend inode references. So we introduce noextiref mount
>> option to close this function.
>
> That's a much better approach compared to setting the flag on mkfs, I agree.
>
>> Signed-off-by: Miao Xie <mi...@cn.fujitsu.com>
>> Cc: Mark Fasheh <mfas...@suse.de>
>> ---
>> fs/btrfs/ctree.h | 1 +
>> fs/btrfs/disk-io.c | 9 +++++++++
>> fs/btrfs/inode-item.c | 2 +-
>> fs/btrfs/super.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>> 4 files changed, 51 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
>> index a883e47..db88963 100644
>> --- a/fs/btrfs/ctree.h
>> +++ b/fs/btrfs/ctree.h
>> @@ -1911,6 +1911,7 @@ struct btrfs_ioctl_defrag_range_args {
>> #define BTRFS_MOUNT_CHECK_INTEGRITY (1 << 20)
>> #define BTRFS_MOUNT_CHECK_INTEGRITY_INCLUDING_EXTENT_DATA (1 << 21)
>> #define BTRFS_MOUNT_PANIC_ON_FATAL_ERROR (1 << 22)
>> +#define BTRFS_MOUNT_NOEXTIREF (1 << 23)
>>
>> #define btrfs_clear_opt(o, opt) ((o) &= ~BTRFS_MOUNT_##opt)
>> #define btrfs_set_opt(o, opt) ((o) |= BTRFS_MOUNT_##opt)
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index ab8ef37..ee00448 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -2269,6 +2269,15 @@ int open_ctree(struct super_block *sb,
>> goto fail_alloc;
>> }
>>
>> + if ((btrfs_super_incompat_flags(disk_super) &
>> + BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF) &&
>> + btrfs_test_opt(tree_root, NOEXTIREF)) {
>> + printk(KERN_ERR "BTRFS: couldn't mount because the extend iref"
>> + "can not be close.\n");
>> + err = -EINVAL;
>> + goto fail_alloc;
>> + }
>> +
>> if (btrfs_super_leafsize(disk_super) !=
>> btrfs_super_nodesize(disk_super)) {
>> printk(KERN_ERR "BTRFS: couldn't mount because metadata "
>> diff --git a/fs/btrfs/inode-item.c b/fs/btrfs/inode-item.c
>> index f07eb45..7c4f880 100644
>> --- a/fs/btrfs/inode-item.c
>> +++ b/fs/btrfs/inode-item.c
>> @@ -442,7 +442,7 @@ int btrfs_insert_inode_ref(struct btrfs_trans_handle
>> *trans,
>> out:
>> btrfs_free_path(path);
>>
>> - if (ret == -EMLINK) {
>> + if (ret == -EMLINK && !btrfs_test_opt(root, NOEXTIREF)) {
>> /*
>> * We ran out of space in the ref array. Need to add an
>> * extended ref.
>> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
>> index 0f03569..fd375b3 100644
>> --- a/fs/btrfs/super.c
>> +++ b/fs/btrfs/super.c
>> @@ -315,7 +315,7 @@ enum {
>> Opt_nodatacow, Opt_max_inline, Opt_alloc_start, Opt_nobarrier, Opt_ssd,
>> Opt_nossd, Opt_ssd_spread, Opt_thread_pool, Opt_noacl, Opt_compress,
>> Opt_compress_type, Opt_compress_force, Opt_compress_force_type,
>> - Opt_notreelog, Opt_ratio, Opt_flushoncommit, Opt_discard,
>> + Opt_notreelog, Opt_noextiref, Opt_ratio, Opt_flushoncommit, Opt_discard,
>> Opt_space_cache, Opt_clear_cache, Opt_user_subvol_rm_allowed,
>> Opt_enospc_debug, Opt_subvolrootid, Opt_defrag, Opt_inode_cache,
>> Opt_no_space_cache, Opt_recovery, Opt_skip_balance,
>> @@ -344,6 +344,7 @@ static match_table_t tokens = {
>> {Opt_nossd, "nossd"},
>> {Opt_noacl, "noacl"},
>> {Opt_notreelog, "notreelog"},
>> + {Opt_noextiref, "noextiref"},
>> {Opt_flushoncommit, "flushoncommit"},
>> {Opt_ratio, "metadata_ratio=%d"},
>> {Opt_discard, "discard"},
>> @@ -535,6 +536,10 @@ int btrfs_parse_options(struct btrfs_root *root, char
>> *options)
>> printk(KERN_INFO "btrfs: disabling tree log\n");
>> btrfs_set_opt(info->mount_opt, NOTREELOG);
>> break;
>> + case Opt_noextiref:
>> + printk(KERN_INFO "btrfs: disabling extend inode ref\n");
>> + btrfs_set_opt(info->mount_opt, NOEXTIREF);
>> + break;
>> case Opt_flushoncommit:
>> printk(KERN_INFO "btrfs: turning on flush-on-commit\n");
>> btrfs_set_opt(info->mount_opt, FLUSHONCOMMIT);
>> @@ -1202,6 +1207,35 @@ static void btrfs_resize_thread_pool(struct
>> btrfs_fs_info *fs_info,
>> new_pool_size);
>> }
>>
>> +static int btrfs_close_extend_iref(struct btrfs_fs_info *fs_info,
>> + unsigned long old_opts)
>
> The name irritated me, it's more like "unset" instead of "close", isn't it?
Maybe "btrfs_set_no_extend_iref()" is better, the other developers might think
we will clear BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF.
>
>> +{
>> + struct btrfs_trans_handle *trans;
>> + int ret;
>> +
>> + if (btrfs_raw_test_opt(old_opts, NOEXTIREF) ||
>> + !btrfs_raw_test_opt(fs_info->mount_opt, NOEXTIREF))
>> + return 0;
>> +
>> + trans = btrfs_attach_transaction(fs_info->tree_root);
>> + if (IS_ERR(trans)) {
>> + if (PTR_ERR(trans) != -ENOENT)
>> + return PTR_ERR(trans);
>> + } else {
>> + ret = btrfs_commit_transaction(trans, fs_info->tree_root);
>> + if (ret)
>> + return ret;
>> + }
>
> Huh? I don't see why we need to commit the transaction here. Can you please
> explain?
We need avoid the case that we check incompat flag is set or not between the
extended iref insertion and incompat flag set.
Task1 Task2
start_transaction()
insert extended iref
set NOEXTIREF
check incompat flag
set incompat flag
checking incompat flag after transaction commit can make sure our check happens
after the flag is set.
Thanks
Miao
>
> Thanks,
> -Jan
>
>> +
>> + if (btrfs_super_incompat_flags(fs_info->super_copy) &
>> + BTRFS_FEATURE_INCOMPAT_EXTENDED_IREF) {
>> + printk(KERN_ERR "BTRFS: could not close extend iref.\n");
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static inline void btrfs_remount_prepare(struct btrfs_fs_info *fs_info)
>> {
>> set_bit(BTRFS_FS_STATE_REMOUNTING, &fs_info->fs_state);
>> @@ -1259,6 +1293,11 @@ static int btrfs_remount(struct super_block *sb, int
>> *flags, char *data)
>> }
>>
>> btrfs_remount_begin(fs_info, old_opts, *flags);
>> +
>> + ret = btrfs_close_extend_iref(fs_info, old_opts);
>> + if (ret)
>> + goto restore;
>> +
>> btrfs_resize_thread_pool(fs_info,
>> fs_info->thread_pool_size, old_thread_pool_size);
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
