On 2018年02月13日 15:20, Nikolay Borisov wrote: > > > On 13.02.2018 03:13, Qu Wenruo wrote: >> There are reports in mail list, even with latest mainline kernel, btrfs >> can't survive a power loss. >> >> Unlike journal based filesystem, btrfs doesn't use journal for such >> work. (log tree is an optimization for fsync, not to keep fs healthy) >> In btrfs we use metadata CoW to ensure all tree blocks are as atomic as >> superblock. >> >> This leads to an obvious assumption, some code breaks such metadata CoW >> makes btrfs no longer bullet-proof against power loss. >> >> This patch adds extra runtime selftest to find_free_extent(), which >> will check the range in commit root of extent tree to ensure there is no >> overlap at all. >> >> And hopes this could help us to catch the cause of the problem. >> >> Signed-off-by: Qu Wenruo <w...@suse.com> >> --- >> Unfortunately, no new problem exposed yet. >> --- >> fs/btrfs/extent-tree.c | 118 >> +++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 118 insertions(+) >> >> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c >> index 2f4328511ac8..3b3cd82bce3a 100644 >> --- a/fs/btrfs/extent-tree.c >> +++ b/fs/btrfs/extent-tree.c >> @@ -7458,6 +7458,114 @@ btrfs_release_block_group(struct >> btrfs_block_group_cache *cache, >> btrfs_put_block_group(cache); >> } >> >> +#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS >> +/* >> + * Verify if the given extent range [@start, @start + @len) conflicts any >> + * existing extent in commit root. >> + * >> + * Btrfs doesn't use journal, but depends on metadata (and data) CoW to keep >> + * the whole filesystem consistent against powerloss. >> + * If we have overwritten any extent used by previous trans (commit root), >> + * and powerloss happen we will corrupt our filesystem. > > Currently do we know if the corruption is caused due to extent overlap > or some other subtle bug?
No direct evidence yet.
But some strange behavior like valid tree block but wrong level (parent
level is 1, leave level is still 1, but otherwise completely valid)
happens after power loss.
And I strongly suspect transid error after powerloss can be related to this.
Overwriting commit root is just one possible cause.
This patch is just trying to catch some clue.
I am also working on new test case using dm-flakey and dm-log to try to
create some crazy powerloss test case to try to reproduce it.
> I.e is there a guarnteee that this self-check
> should trigger?
No guarantee yet.
But at least for normal tree allocation and new data extent allocation,
it is going through such call chain and should trigger this self-check:
For tree allocation:
btrfs_alloc_tree_block()
|- btrfs_reserve_extent()
|- find_free_extent()
|- check_extent_conflicts()
For data extent, there are several cases:
For prealloc
btrfs_prealloc_file_range() or btrfs_prealloc_file_range_trans()
|- __btrfs_prealloc_file_range()
|- btrfs_reserve_extent()
For data CoW:
cow_file_range()
|- btrfs_reserve_extent()
For compression:
submit_compressed_extents()
|- btrfs_reserve_extent()
For direct:
btrfs_new_extent_direct()
|- btrfs_reserve_extent()
So btrfs_reserve_extent() should be a quite good entry point, although
I'm still not 100% sure if there is any other case which could reserve
extent without calling btrfs_reserve_extent()
If we could found that, it may be the cause.
Thanks,
Qu
>
>> + *
>> + * Return 0 if nothing wrong.
>> + * Return <0 (including ENOMEM) means we have something wrong.
>> + * Except NOEMEM, this normally means we have extent conflicts with previous
>> + * transaction.
>> + */
>> +static int check_extent_conflicts(struct btrfs_fs_info *fs_info,
>> + u64 start, u64 len)
>> +{
>> + struct btrfs_key key;
>> + struct btrfs_path *path;
>> + struct btrfs_root *extent_root = fs_info->extent_root;
>> + u64 extent_start;
>> + u64 extent_len;
>> + int ret;
>> +
>> + path = btrfs_alloc_path();
>> + if (!path)
>> + return -ENOMEM;
>> +
>> +
>> + key.objectid = start + len;
>> + key.type = 0;
>> + key.offset = 0;
>> +
>> + /*
>> + * Here we use extent commit root to search for any conflicts.
>> + * If extent commit tree is already overwritten, we will get transid
>> + * error and error out any way.
>> + * If extent commit tree is OK, but other extent conflicts, we will
>> + * find it.
>> + * So anyway, such search should be OK to find the conflicts.
>> + */
>> + path->search_commit_root = true;
>> + ret = btrfs_search_slot(NULL, extent_root, &key, path, 0, 0);
>> +
>> + if (ret < 0)
>> + goto out;
>> + /* Impossible as no type/offset should be (u64)-1 */
>> + if (ret == 0) {
>> + ret = -EUCLEAN;
>> + goto out;
>> + }
>> + ret = btrfs_previous_extent_item(extent_root, path, start);
>> + if (ret < 0)
>> + goto out;
>> + if (ret == 0) {
>> + btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>> + extent_start = key.objectid;
>> + if (key.type == BTRFS_EXTENT_ITEM_KEY)
>> + extent_len = key.offset;
>> + else
>> + extent_len = fs_info->nodesize;
>> + goto report;
>> + }
>> + /*
>> + * Even we didn't found extent starts after @start, we still need to
>> + * ensure previous extent doesn't overlap with [@start, @start + @len)
>> + */
>> + while (1) {
>> + extent_len = 0;
>> + btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>> + if (key.type == BTRFS_EXTENT_ITEM_KEY)
>> + extent_len = key.offset;
>> + else if (key.type == BTRFS_METADATA_ITEM_KEY)
>> + extent_len = fs_info->nodesize;
>> +
>> + if (extent_len) {
>> + if (extent_len + key.objectid <= start) {
>> + ret = 0;
>> + goto out;
>> + }
>> + extent_start = key.objectid;
>> + goto report;
>> + }
>> + if (path->slots[0] == 0) {
>> + ret = btrfs_prev_leaf(extent_root, path);
>> + if (ret > 0) {
>> + ret = 0;
>> + goto out;
>> + }
>> + if (ret < 0)
>> + goto out;
>> + } else {
>> + path->slots[0]--;
>> + }
>> + }
>> +out:
>> + btrfs_free_path(path);
>> + return ret;
>> +report:
>> + WARN(1,
>> +"broken CoW detected: old extent [%llu, %llu) new extent [%llu, %llu)\n",
>> + extent_start, extent_start + extent_len, start, start + len);
>> + btrfs_free_path(path);
>> + return -EEXIST;
>> +}
>> +#endif
>> +
>> /*
>> * walks the btree of allocated extents and find a hole of a given size.
>> * The key ins is changed to record the hole:
>> @@ -7949,6 +8057,16 @@ static noinline int find_free_extent(struct
>> btrfs_fs_info *fs_info,
>> spin_unlock(&space_info->lock);
>> ins->offset = max_extent_size;
>> }
>> +#ifdef CONFIG_BTRFS_FS_RUN_SANITY_TESTS
>> + if (!ret) {
>> + /*
>> + * Any extent allocated must not conflict with any extent in
>> + * commit root
>> + */
>> + ret = check_extent_conflicts(fs_info, ins->objectid,
>> + ins->offset);
>> + }
>> +#endif
>> return ret;
>> }
>>
>>
> --
> 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
>
signature.asc
Description: OpenPGP digital signature
