On 2020/2/15 上午12:53, David Sterba wrote:
> On Wed, Feb 12, 2020 at 03:46:51PM +0800, Qu Wenruo wrote:
>> This bit is being used in too many locations while there is still no
>> good enough explaination for how this bit is used.
>>
>> Not to mention its name really doesn't make much sense.
>>
>> So this patch will add my explanation on this bit, considering only
>> subvolume trees, along with its reloc trees have this bit, to me it
>> looks like this bit shows whether tree blocks of a root can be shared.
>
> I think there's more tan just sharing, it should say something about
> reference counted sharing. See eg. btrfs_block_can_be_shared:
>
> 864 /*
> 865 * Tree blocks not in reference counted trees and tree roots
> 866 * are never shared. If a block was allocated after the last
> 867 * snapshot and the block was not allocated by tree relocation,
> 868 * we know the block is not shared.
> 869 */
>
> And there can be more specialities found when grepping for REF_COWS. The
> comment explaination should be complete or at least mention what's not
> documenting. The I find the suggested version insufficient but don't
> have a concrete suggestions for improvement. By reading the comment and
> going through code I don't feel any wiser.
>
I see nothing extra conflicting the "shared tree blocks" part from
btrfs_block_can_be_shared().
In fact, reloc tree can only be created for trees with REF_COW bit.
For tree without that bit, we go a completely different way to relocate
them, by just cowing the path (aka the cowonly bit in build_backref_tree()).
if (root) {
if (test_bit(BTRFS_ROOT_REF_COWS, &root->state)) {
BUG_ON(node->new_bytenr);
BUG_ON(!list_empty(&node->list));
btrfs_record_root_in_trans(trans, root);
root = root->reloc_root;
node->new_bytenr = root->node->start;
node->root = root;
list_add_tail(&node->list, &rc->backref_cache.changed);
} else {
path->lowest_level = node->level;
ret = btrfs_search_slot(trans, root, key, path, 0, 1); <<<
btrfs_release_path(path);
if (ret > 0)
ret = 0;
}
So the "REF_COW means tree blocks can be shared" still looks pretty
valid to me.
Thanks,
Qu
Attachment:
signature.asc
Description: OpenPGP digital signature
