Re: [PATCH 5/7] btrfs: check/lowmem: Check and repair free space cache inode mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2019/3/25 下午10:36, Su Yue wrote:
> 
> 
> On 2019/3/25 4:22 PM, Qu Wenruo wrote:
>> Unlike inodes in fs roots, we don't really check the inode items in root
>> tree, in fact we just skip everything other than ROOT_ITEM and ROOT_REF.
>>
>> This makes invalid inode items sneak into root tree.
>> For example:
>>          item 9 key (256 INODE_ITEM 0) itemoff 13702 itemsize 160
>>                  generation 30 transid 30 size 65536 nbytes 1507328
>>                  block group 0 mode 0 links 1 uid 0 gid 0 rdev 0
>>                    ^ Should be 100600
>>                  sequence 23 flags
>> 0x1b(NODATASUM|NODATACOW|NOCOMPRESS|PREALLOC)
>>                  atime 0.0 (1970-01-01 08:00:00)
>>                  ctime 1553491158.189771625 (2019-03-25 13:19:18)
>>                  mtime 0.0 (1970-01-01 08:00:00)
>>                  otime 0.0 (1970-01-01 08:00:00)
>>
>> There is a report of such problem in the mail list.
>>
>> This patch will check and repair inode items of free space cache
>> inodes in
>> lowmem mode.
>>
>> Since free space cache inodes doesn't have INODE_REF but still has 1
>> link, we can't use check_inode_item() directly.
>> Instead we only check the inode mode, as that's the important part.
>>
>> The check and repair function: check_repair_free_space_inode() is also
>> exported for original mode.
>>
>> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
>> ---
>>   check/mode-common.h |  1 +
>>   check/mode-lowmem.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>   check/mode-lowmem.h |  2 ++
>>   3 files changed, 48 insertions(+)
>>
>> diff --git a/check/mode-common.h b/check/mode-common.h
>> index 8895747adf1d..855f83617854 100644
>> --- a/check/mode-common.h
>> +++ b/check/mode-common.h
>> @@ -24,6 +24,7 @@
>>   #include <sys/stat.h>
>>   #include "ctree.h"
>>
>> +#define FREE_SPACE_CACHE_INODE_MODE    (0100600)
>>   /*
>>    * Use for tree walk to walk through trees whose leaves/nodes can be
>> shared
>>    * between different trees. (Namely subvolume/fs trees)
>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>> index f37c3b42e6b6..cdb9b66a63a3 100644
>> --- a/check/mode-lowmem.c
>> +++ b/check/mode-lowmem.c
>> @@ -5091,6 +5091,40 @@ out:
>>       return err;
>>   }
>>
>> +/*
>> + * For free space inodes, we can't call check_inode_item() as free space
>> + * cache inode doesn't have INODE_REF.
>> + * We just check its inode mode.
>> + */
>> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
>> +                  struct btrfs_path *path)
>> +{
> 
> 
> In personal taste, I prefer check_free_space_inode() to
> follow existed code.
> 
>> +    struct btrfs_inode_item *iitem;
>> +    struct btrfs_key key;
>> +    u32 mode;
>> +    int ret = 0;
>> +
>> +    btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>> +    ASSERT(key.type == BTRFS_INODE_ITEM_KEY && is_fstree(key.objectid));
>> +    iitem = btrfs_item_ptr(path->nodes[0], path->slots[0],
>> +                   struct btrfs_inode_item);
>> +    mode = btrfs_inode_mode(path->nodes[0], iitem);
>> +    if (mode != FREE_SPACE_CACHE_INODE_MODE) {
>> +        error(
>> +    "free space cache inode %llu has invalid mode: has 0%o expect 0%o",
>> +            key.objectid, mode, FREE_SPACE_CACHE_INODE_MODE);
>> +        ret = -EUCLEAN;
>> +        if (repair) {
>> +            ret = repair_imode_lowmem(fs_info->tree_root,
>> +                          path);
>> +            if (ret < 0)
>> +                return ret;
>> +            return ret;
>> +        }
>> +    }
>> +    return ret;
>> +}
>> +
>>   /*
>>    * Check all fs/file tree in low_memory mode.
>>    *
>> @@ -5130,6 +5164,17 @@ int check_fs_roots_lowmem(struct btrfs_fs_info
>> *fs_info)
>>           btrfs_item_key_to_cpu(node, &key, slot);
>>           if (key.objectid > BTRFS_LAST_FREE_OBJECTID)
>>               goto out;
>> +        if (key.type == BTRFS_INODE_ITEM_KEY &&
>> +            is_fstree(key.objectid)) {
>> +            ret = check_repair_free_space_inode(fs_info, &path);
>> +            /* Check if we still have a valid path to continue */
> 
> 'path.nodes[0]' is quite interesting :).
> What about the situation btrfs_search_slot() succeed but failed to
> commit a transaction?

As long as path.nodes[0] exists, we still have a valid path of root
tree, check can continue going.

For that valid path but failed commit case, I believe the most possible
problem would be a corrupted extent tree. And in that case, we can still
continue checking fs tree.

Thanks,
Qu

> 
> 
> ---
> Su
>> +            if (ret < 0 && path.nodes[0]) {
>> +                err |= ret;
>> +                goto next;
>> +            }
>> +            if (ret < 0 && !path.nodes[0])
>> +                goto out;
>> +        }
>>           if (key.type == BTRFS_ROOT_ITEM_KEY &&
>>               fs_root_objectid(key.objectid)) {
>>               if (key.objectid == BTRFS_TREE_RELOC_OBJECTID) {
>> diff --git a/check/mode-lowmem.h b/check/mode-lowmem.h
>> index e0ab30b770d5..d2983fd12eb4 100644
>> --- a/check/mode-lowmem.h
>> +++ b/check/mode-lowmem.h
>> @@ -67,5 +67,7 @@
>>
>>   int check_fs_roots_lowmem(struct btrfs_fs_info *fs_info);
>>   int check_chunks_and_extents_lowmem(struct btrfs_fs_info *fs_info);
>> +int check_repair_free_space_inode(struct btrfs_fs_info *fs_info,
>> +                  struct btrfs_path *path);
>>
>>   #endif
>>
> 

Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux