Re: [PATCH] btrfs: Add comment for BTRFS_ROOT_REF_COWS

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2292 bytes --]



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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 520 bytes --]