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