Re: [PATCh v2 8/9] btrfs: tree-checker: Verify inode item

[Qu Wenruo <wqu@suse.de>]

On 2019/3/28 下午9:38, David Sterba wrote:
> On Wed, Mar 20, 2019 at 02:37:16PM +0800, Qu Wenruo wrote:
>> +	if (S_ISDIR(mode) && btrfs_inode_nlink(leaf, iitem) > 1) {
>> +		inode_item_err(fs_info, leaf, slot,
>> +			       "invalid nlink: has %u expect no more than 1 for dir",
>> +			btrfs_inode_nlink(leaf, iitem));
>> +		goto error;
>> +	}
> 
> I'm not sure about this check, the number of links for a directory is 1,
> but the exact count could be implemented (there's a project idea for
> that). I don't know if this will require an incompat bit or not.

That means we have hard link for directories.

That's definitely something current VFS can't handle, things like path
resolve loop is definitely going to happen.

> 
> As long as the number is not authoritative (ie. can be verified and
> fixed from the actual directory items), then I'd say don't check it at
> all, or perhaps do only weak check if it's > 0.

Current the number is authoritative already.
It means the number of hard links, both for directory and regular
inodes, also indicates the number of INODE_REF.

Btrfs check will verify that.

> 
> Out of all the verified inode data, this is the least important, I think
> we're not losing some significant piece of information.

That's true, nlinks is not that important, but just as always,
tree-checker is checking every meaningful member and try to catch
anything uncommon.
Although that restrict behavior has already caused a lot of false alerts
in the past, I still prefer to do such restrict check.

Thanks,
Qu