On 11.09.19 г. 3:39 ч., Qu Wenruo wrote:
> [...]
>>> - Search for DIR_INDEX/DIR_ITEM
>>> If above search fails, we falls back to locate the DIR_INDEX/DIR_ITEM
>>> just after the INODE_ITEM.
>>> If any can be found, it's definitely a directory.
>>
>> This needs an explicit satement that it will only work for non-empty files
>> and directories
>
> Indeed.
>
>>>
>>> - Search for EXTENT_DATA
>>> If EXTENT_DATA can be found, it's either REG or LNK.
>>> For this case, we default to REG, as user can inspect the file to
>>> determine if it's a file or just a path.
>>>
>>> - Use rdev to detect BLK/CHR
>>> If all above fails, but INODE_ITEM has non-zero rdev, then it's either
>>> a BLK or CHR file. Then we default to BLK.
>>>
>>> - Fail out if none of above methods succeeded
>>> No educated guess to make things worse.
>>>
>>> Signed-off-by: Qu Wenruo <w...@suse.com>
>>> ---
>>> check/mode-common.c | 130 +++++++++++++++++++++++++++++++++++++++-----
>>> 1 file changed, 117 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/check/mode-common.c b/check/mode-common.c
>>> index c0ddc50a1dd0..abea2ceda4c4 100644
>>> --- a/check/mode-common.c
>>> +++ b/check/mode-common.c
>>> @@ -935,6 +935,113 @@ out:
>>> return ret;
>>> }
>>>
>>> +static int detect_imode(struct btrfs_root *root, struct btrfs_path *path,
>>> + u32 *imode_ret)
>>> +{
>>> + struct btrfs_key key;
>>> + struct btrfs_inode_item iitem;
>>> + const u32 priv = 0700;
>>> + bool found = false;
>>> + u64 ino;
>>> + u32 imode;
>>> + int ret = 0;
>>> +
>>> + btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
>>> + ino = key.objectid;
>>> + read_extent_buffer(path->nodes[0], &iitem,
>>> + btrfs_item_ptr_offset(path->nodes[0], path->slots[0]),
>>> + sizeof(iitem));
>>> + /* root inode */
>>> + if (ino == BTRFS_FIRST_FREE_OBJECTID) {
>>> + imode = S_IFDIR;
>>> + found = true;
>>> + goto out;
>>> + }
>>> +
>>> + while (1) {
>>> + struct btrfs_inode_ref *iref;
>>> + struct extent_buffer *leaf;
>>> + unsigned long cur;
>>> + unsigned long end;
>>> + char namebuf[BTRFS_NAME_LEN] = {0};
>>> + u64 index;
>>> + u32 namelen;
>>> + int slot;
>>> +
>>> + ret = btrfs_next_item(root, path);
>>> + if (ret > 0) {
>>> + /* falls back to rdev check */
>>> + ret = 0;
>>> + goto out;
>>> + }
>>
>> In my testing if an inode is the last one in the leaf and it doesn't have
>> an INODE_REF item then it won't be repaired. But e.g. it can have perfectly
>> valid DIR_ITEM/DIR_INDEX entries which describe this inode as being a file.
>> E.g.
>>
>> item 2 key (256 DIR_ITEM 388586943) itemoff 16076 itemsize 35
>> location key (260 INODE_ITEM 0) type FILE
>> transid 7 data_len 0 name_len 5
>> name: file3
>>
>> .....
>> item 15 key (260 INODE_ITEM 0) itemoff 15184 itemsize 160
>> generation 7 transid 7 size 0 nbytes 0
>> block group 0 mode 26772225102 links 1 uid 0 gid 0 rdev 0
>> sequence 1 flags 0x0(none)
>> atime 1568127261.284993602 (2019-09-10 14:54:21)
>> ctime 1568127261.284993602 (2019-09-10 14:54:21)
>> mtime 1568127261.284993602 (2019-09-10 14:54:21)
>> otime 1568127261.284993602 (2019-09-10 14:54:21)
>>
>> I have intentionally deleted INODE_REF too see what's happening. Is this
>> intended?
>
> Yes, completely intended.
>
> For this case, you need to iterate through the whole tree to locate the
> DIR_INDEX to fix, which is not really possible with current code base,
> which only search based on the INODE, not the DIR_INDEX/DIR_ITEM from
> its parent dir.
>
> Furthermore, didn't you mention that if we don't have confident about
> the imode, then we should fail out instead of using REG as default?
I did, what I supposed could happen here is if we can't find an
INODE_REF then do a search for DIR_INDEX/DIR_ITEM since those items also
contain the type of the inode they are pointing to. Fixing really boils
down to exploiting redundancy in the on-disk metadata to repair existing
items. Here comes the question, of course, what to do if we don't have
an INODE_REF and DIR_INDEX/DIR_ITEM are broken. I guess it's a judgement
call, currently you decided that inode_ref will be the source of
information. I'm fine with that I was merely pointing there is more we
can do. Of course we need to weigh the pros/cons between code complexity
and the returns we get.
Just that I will advise to make it explicit in the changelog that you
made a judgement call to utilize INODE_REF as the starting point of
doing imode repair but not necessarily the only one.
<snip>
