On 2020/5/13 下午10:01, David Sterba wrote:
> On Wed, May 13, 2020 at 02:16:11PM +0800, Qu Wenruo wrote:
>> - if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID) {
>> + if (root->root_key.objectid != BTRFS_TREE_LOG_OBJECTID &&
>> + root->root_key.objectid != BTRFS_DATA_RELOC_TREE_OBJECTID) {
>
>> if (test_bit(BTRFS_ROOT_SHAREABLE, &root->state) ||
>> - root == fs_info->tree_root)
>> + root == fs_info->tree_root || root == fs_info->dreloc_root)
>
>> if (found_extent &&
>> (test_bit(BTRFS_ROOT_SHAREABLE, &root->state) ||
>> - root == fs_info->tree_root)) {
>> + root == fs_info->tree_root ||
>> + root == fs_info->dreloc_root)) {
>
> Lots of repeated conditions, though not all of them exactly the same. I
> was thinking about adding some helpers but don't have a good suggestion.
The good news is, only inode truncating cares that much.
All other locations won't bother at all.
The concern here is, INODE_ITEM can exist in trees without SHAREABLE
(REF_COWS).
The original exception is root tree (v1 space cache uses INODE_ITEM).
As we don't set SHAREABLE flag for data reloc tree now, it's adding the
exception.
I could find a way to make it more readable, by separating the regular
SHAREABLE check and two exceptions and wrap it into a function.
Thanks,
Qu
>
> The concern is about too much special casing, it's eg tree_root +
> data_reloc_tree, or SHAREABLE + tree_reloc + data_reloc, etc. The
> helpers should capture the common semantics of the condition and also
> reduce the surface for future errors.
>
Attachment:
signature.asc
Description: OpenPGP digital signature
