On Fri, May 15, 2020 at 02:01:42PM +0800, Qu Wenruo wrote:
> @@ -1525,6 +1526,7 @@ void btrfs_free_fs_info(struct btrfs_fs_info *fs_info)
> btrfs_put_root(fs_info->uuid_root);
> btrfs_put_root(fs_info->free_space_root);
> btrfs_put_root(fs_info->fs_root);
> + btrfs_put_root(fs_info->data_reloc_root);
> btrfs_check_leaked_roots(fs_info);
> btrfs_extent_buffer_leak_debug_check(fs_info);
> kfree(fs_info->super_copy);
> + location.objectid = BTRFS_DATA_RELOC_TREE_OBJECTID;
> + root = btrfs_get_fs_root(fs_info, &location, true);
> + if (IS_ERR(root)) {
> + ret = PTR_ERR(root);
> + goto out;
> + }
> + set_bit(BTRFS_ROOT_TRACK_DIRTY, &root->state);
> + fs_info->data_reloc_root = root;
I've read the code more carefully, the data reloc tree needs to be read
as the others, like fs_tree as you said before. A new tree has the
reference count set to 1, which is what I missed before, so calling
btrfs_get_fs_root would set it to 2 and then btrfs_free_fs_info won't
free it as it expects refs == 1. Sorry for misleading you.
> @@ -3470,14 +3470,12 @@ struct inode *create_reloc_inode(struct btrfs_fs_info *fs_info,
> {
> struct inode *inode = NULL;
> struct btrfs_trans_handle *trans;
> - struct btrfs_root *root;
> + struct btrfs_root *root = btrfs_grab_root(fs_info->data_reloc_root);
Which means you can use the pointer directly as you had in previous
versions.
> struct btrfs_key key;
> u64 objectid;
> int err = 0;
>
> - root = read_fs_root(fs_info, BTRFS_DATA_RELOC_TREE_OBJECTID);
> - if (IS_ERR(root))
> - return ERR_CAST(root);
> + ASSERT(root);
>
> trans = btrfs_start_transaction(root, 6);
> if (IS_ERR(trans)) {
> @@ -3870,13 +3868,10 @@ int btrfs_recover_relocation(struct btrfs_root *root)
>
> if (err == 0) {
> /* cleanup orphan inode in data relocation tree */
> - fs_root = read_fs_root(fs_info, BTRFS_DATA_RELOC_TREE_OBJECTID);
> - if (IS_ERR(fs_root)) {
> - err = PTR_ERR(fs_root);
> - } else {
> - err = btrfs_orphan_cleanup(fs_root);
> - btrfs_put_root(fs_root);
> - }
> + fs_root = btrfs_grab_root(fs_info->data_reloc_root);
> + ASSERT(fs_root);
> + err = btrfs_orphan_cleanup(fs_root);
> + btrfs_put_root(fs_root);
Here it's fine to grab/put so it's clear the tree is in use but it's
merely for clarity.
