On 14.02.20 г. 11:33 ч., Qu Wenruo wrote:
>
>
> On 2020/2/14 下午5:19, Nikolay Borisov wrote:
>>
>>
>> On 14.02.20 г. 10:13 ч., Qu Wenruo wrote:
>>> Due to the complex nature of btrfs extent tree, when we want to iterate
>>> all backrefs of one extent, it involves quite a lot of works, like
>>> search the EXTENT_ITEM/METADATA_ITEM, iteration through inline and keyed
>>> backrefs.
>>>
>>> Normally this would result pretty complex code, something like:
>>> btrfs_search_slot()
>>> /* Ensure we are at EXTENT_ITEM/METADATA_ITEM */
>>> while (1) { /* Loop for extent tree items */
>>> while (ptr < end) { /* Loop for inlined items */
>>> /* REAL WORK HERE */
>>> }
>>> next:
>>> ret = btrfs_next_item()
>>> /* Ensure we're still at keyed item for specified bytenr */
>>> }
>>>
>>> The idea of btrfs_backref_iterator is to avoid such complex and hard to
>>> read code structure, but something like the following:
>>>
>>> iterator = btrfs_backref_iterator_alloc();
>>> ret = btrfs_backref_iterator_start(iterator, bytenr);
>>> if (ret < 0)
>>> goto out;
>>> for (; ; ret = btrfs_backref_iterator_next(iterator)) {
>>> /* REAL WORK HERE */
>>> }
>>> out:
>>> btrfs_backref_iterator_free(iterator);
>>>
>>> This patch is just the skeleton + btrfs_backref_iterator_start() code.
>>>
>>> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
>>> ---
>>> fs/btrfs/backref.c | 58 +++++++++++++++++++++++++++++++++++
>>> fs/btrfs/backref.h | 76 ++++++++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 134 insertions(+)
>>>
>>> diff --git a/fs/btrfs/backref.c b/fs/btrfs/backref.c
>>> index e5d85311d5d5..73c4829609c9 100644
>>> --- a/fs/btrfs/backref.c
>>> +++ b/fs/btrfs/backref.c
>>> @@ -2252,3 +2252,61 @@ void free_ipath(struct inode_fs_paths *ipath)
>>> kvfree(ipath->fspath);
>>> kfree(ipath);
>>> }
>>> +
>>> +int btrfs_backref_iterator_start(struct btrfs_backref_iterator *iterator,
>>> + u64 bytenr)
>>> +{
>>> + struct btrfs_fs_info *fs_info = iterator->fs_info;
>>> + struct btrfs_path *path = iterator->path;
>>> + struct btrfs_extent_item *ei;
>>> + struct btrfs_key key;
>>> + int ret;
>>> +
>>> + key.objectid = bytenr;
>>> + key.type = BTRFS_METADATA_ITEM_KEY;
>>> + key.offset = (u64)-1;
>>> +
>>> + ret = btrfs_search_slot(NULL, fs_info->extent_root, &key, path, 0, 0);
>>> + if (ret < 0)
>>> + return ret;
>>> + if (ret == 0) {
>>> + ret = -EUCLEAN;
>>> + goto release;
>>> + }
>>> + if (path->slots[0] == 0) {
>>> + WARN_ON(IS_ENABLED(CONFIG_BTRFS_DEBUG));
>>> + ret = -EUCLEAN;
>>> + goto release;
>>> + }
>>> + path->slots[0]--;
>>> +
>>> + btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>>> + if (!(key.type == BTRFS_EXTENT_ITEM_KEY ||
>>> + key.type == BTRFS_METADATA_ITEM_KEY) || key.objectid != bytenr) {
>>> + ret = -ENOENT;
>>> + goto release;
>>> + }
>>> + memcpy(&iterator->cur_key, &key, sizeof(key));
>>> + iterator->end_ptr = btrfs_item_end_nr(path->nodes[0], path->slots[0]);
>>> + iterator->item_ptr = btrfs_item_ptr_offset(path->nodes[0],
>>> + path->slots[0]);
>>> + ei = btrfs_item_ptr(path->nodes[0], path->slots[0],
>>> + struct btrfs_extent_item);
>>> +
>>> + /* Only support iteration on tree backref yet */
>>> + if (btrfs_extent_flags(path->nodes[0], ei) & BTRFS_EXTENT_FLAG_DATA) {
>>> + ret = -ENOTTY;
>>> + goto release;
>>> + }
>>
>> Isn't this implied bye the fact you are searching for METADATA ITEMS to
>> begin with ? Considering this shouldn't detecting EXTENT_FLAG_DATA in
>> the backrefs of a METADATA_EXTENT be considered a corruption?
>
> For non skinny-metadata fs, we can hit with EXTENT_ITEM.
> So it's still possible to hit a corruption undetected by tree-checker.
>
> But you're right, we shouldn't really hit a data extent here, as
> previous loops have excluded all data extents.
Then put a comment saying this is done as an extra precaution.
<split>
