Re: [PATCH v2 4/6] btrfs-progs: check/lowmem: Repair bad imode early

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2019/9/9 下午10:55, Nikolay Borisov wrote:
>
>
> On 5.09.19 г. 10:57 ч., Qu Wenruo wrote:
>> For lowmem mode, if we hit a bad inode mode, normally it is reported
>> when we checking the DIR_INDEX/DIR_ITEM of the parent inode.
>>
>> If we didn't repair at that timing, the error will be recorded even we
>> fixed it later.
>>
>> So this patch will check for INODE_ITEM_MISMATCH error type, and if it's
>> really caused by invalid imode, repair it and clear the error.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  check/mode-lowmem.c | 39 +++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 39 insertions(+)
>>
>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>> index 5f7f101daab1..5d0c520217fa 100644
>> --- a/check/mode-lowmem.c
>> +++ b/check/mode-lowmem.c
>> @@ -1550,6 +1550,35 @@ static int lowmem_delete_corrupted_dir_item(struct btrfs_root *root,
>>  	return ret;
>>  }
>>
>> +static int try_repair_imode(struct btrfs_root *root, u64 ino)
>> +{
>> +	struct btrfs_inode_item *iitem;
>> +	struct btrfs_path path;
>> +	struct btrfs_key key;
>> +	int ret;
>> +
>> +	key.objectid = ino;
>> +	key.type = BTRFS_INODE_ITEM_KEY;
>> +	key.offset = 0;
>> +	btrfs_init_path(&path);
>> +
>> +	ret = btrfs_search_slot(NULL, root, &key, &path, 0, 0);
>> +	if (ret > 0)
>> +		ret = -ENOENT;
>> +	if (ret < 0)
>> +		goto out;
>> +	iitem = btrfs_item_ptr(path.nodes[0], path.slots[0],
>> +			       struct btrfs_inode_item);
>> +	if (!is_valid_imode(btrfs_inode_mode(path.nodes[0], iitem))) {
>
> INODE_ITEM_MISMATCH is only set if:
>
> 1. The first inode item is not a directory (it should always be)
> 2. There is a mismatch between the filetype in the inode item and in the
> dir/index item pointing to it.
>
> By using this check you could possibly miss case 1 above, if the first
> inode has a valid type e.g. regular whereas it should really be a
> directory.

That's indeed a special rare case.

>
> I'm not entirely sure whether it makes sense to handle this situation,
> since admittedly, it would require a perfect storm to get such a
> corruption.

Let's wait to see if this could happen.
If it happened, we can easily modify the code to handle it anyway.

On the other hand, let's just focus on the real corruption where some
old 2014 kernel screwed up the imode of some inodes.

Thanks,
Qu

>
>
> <snip>
>