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

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

 




On 2019/3/28 下午10:25, David Sterba wrote:
> On Thu, Mar 28, 2019 at 10:13:19PM +0800, Qu Wenruo wrote:
>>>>>> That means we have hard link for directories.
>>>>>
>>>>> Yes, hard links of directories are forbidden by VFS but that's not the
>>>>> point here:
>>>>>
>>>>> https://btrfs.wiki.kernel.org/index.php/Project_ideas#Track_link_count_for_directories
>>>>>
>>>>> "The link count for directories is traditionally used to count the number
>>>>> of subdirectores iff the link count is >= 2."
>>>>
>>>> Oh, got it.
>>>>
>>>> But for that case, it would be much better to go for things like nbytes
>>>> or size, as by all means, those two members are less meaningful than
>>>> nlinks for directory.
>>>
>>> size of a directory is currently 2 * sum of all names in the directory,
>>> ie. both files and directories.
>>
>> In fact that's not always the case for older kernels/progs.
>>
>> IIRC it's in recent years that we enhanced btrfs-progs to detect and fix
>> that.
>>
>>> The nlink is used as an optimization
>>> during traversal by existing tools (find), we can't simply change that
>>> but would still like to update btrfs to provide support for that.
>>
>> But current nlinks is persistent against all inodes. It's always the the
>> number of INODE_REF the inode has.
>>
>> While for size/nbytes, it doesn't make anything for directory inode at all.
>> Kernel doesn't care, it's mostly btrfs-check check and fix.
>>
>> Furthermore, only size is fixed to 2 * num_children.
>> nbytes is only instructive and at least in lowmem mode, nbytes is only
>> checked for alignment, even it's unaligned, lowmem check outputs warning
>> only, doesn't count as an error.
>>
>> So at least to me, directory nbytes is a better alternative, and it's
>> back compatible.
>
> The point here is to use nlinks of directories for the directory
> traversal, the size/nbytes is not relevant here for the reasons you
> state above.
>
> I don't see how you think the proposed nbytes alternative would be used,
> eg. by find.


In state() call or whatever syscall find is using, export nbytes instead
of nlink for dir.
Of course, this needs some compatibility check, e.g. nbytes should never
exceed size/2, or the nbytes must be old value (nodesize).

My point is, if we're going to give new meaning of an existing member,
at least choose a member that is unused and doesn't have persistent
meaning among all inodes, not to mention current btrfs check will
definitely complain about nlinks.

Thanks,
Qu





[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