On 2/19/20 10:10 AM, Nikolay Borisov wrote:
On 14.02.20 г. 23:11 ч., Josef Bacik wrote:
There are a few different ways to free roots, either you allocated them
yourself and you just do
free_extent_buffer(root->node);
free_extent_buffer(root->commit_node);
btrfs_put_root(root);
Which is the pattern for log roots. Or for snapshots/subvolumes that
are being dropped you simply call btrfs_free_fs_root() which does all
the cleanup for you.
Unify this all into btrfs_put_root(), so that we don't free up things
associated with the root until the last reference is dropped. This
makes the root freeing code much more significant.
The only caveat is at close_ctree() time we have to free the extent
buffers for all of our main roots (extent_root, chunk_root, etc) because
we have to drop the btree_inode and we'll run into issues if we hold
onto those nodes until ->kill_sb() time. This will be addressed in the
future when we kill the btree_inode.
Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
Nit: This patch obsoleted the last comment in btrfs_init_fs_root, namely:
/* The caller is responsible to call btrfs_free_fs_root */
---
fs/btrfs/disk-io.c | 64 ++++++++++++++++++------------------
fs/btrfs/disk-io.h | 16 +--------
fs/btrfs/extent-tree.c | 7 ++--
fs/btrfs/extent_io.c | 16 +++++++--
fs/btrfs/free-space-tree.c | 2 --
fs/btrfs/qgroup.c | 7 +---
fs/btrfs/relocation.c | 4 ---
fs/btrfs/tests/btrfs-tests.c | 5 +--
fs/btrfs/tree-log.c | 6 ----
9 files changed, 50 insertions(+), 77 deletions(-)
<snip>
@@ -4795,7 +4803,6 @@ int extent_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
static void __free_extent_buffer(struct extent_buffer *eb)
{
- btrfs_leak_debug_del(&eb->fs_info->eb_leak_lock, &eb->leak_list);
kmem_cache_free(extent_buffer_cache, eb);
}
This function becomes a trivial wrapper so should be eliminated altogether.
<snip>
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 034f5f151a74..4fb7e3cc2aca 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -2549,10 +2549,6 @@ void free_reloc_roots(struct list_head *list)
reloc_root = list_entry(list->next, struct btrfs_root,
root_list);
__del_reloc_root(reloc_root);
- free_extent_buffer(reloc_root->node);
- free_extent_buffer(reloc_root->commit_root);
- reloc_root->node = NULL;
- reloc_root->commit_root = NULL;
Shouldn't you do btrfs_put_root(reloc_root) here ?
No, but I can see how this is confusing. The reloc root is actually cleaned up
in clean_dirty_subvols(), so it's final put happens there. Thanks,
Josef
