Hi David,
Any extra comment on this?
From my recent dig into relocation code, the REF_COW really only means
if the tree blocks of one tree can be shared by multiple roots.
(Just like subvolume trees).
It mostly affects:
- How the root is updated at commit transaction
This bit is conflicting with TRACK_DIRTY, thus we need to manually
call record_root_in_trans() to mark one REF_COW root dirty.
- How relocation handles its tree blocks
For non-REF_COW trees, relocation just uses COW to do the relocation,
while for tree with such bit, it goes the path replace way.
Finally maybe a little off-topic, tree without REF_COW bit can still
contain inodes/extent data, like root tree and data reloc tree.
Root tree is the only exception where we have no REF_COW bit but still
allow file extents to be created in it.
(And I'm working on clear the REF_COW bit for data reloc tree, as we
can't create snapshot for that root).
Any more comment?
Thanks,
Qu
On 2020/2/17 下午3:06, Qu Wenruo wrote:
>
>
> 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
