On 07/20/2016 08:44 PM, Liu Bo wrote:
While processing delayed refs, we may update block group's statistics
and attach it to cur_trans->dirty_bgs, and later writing dirty block
groups will process the list, which happens during
btrfs_commit_transaction().

For whatever reason, the transaction is aborted and dirty_bgs
is not processed in cleanup_transaction(), we end up with memory leak
of these dirty block group cache.

Since btrfs_start_dirty_block_groups() doesn't make it go to the commit
critical section, this also adds the cleanup work inside it.

It's the start_drity_block_groups() hunt that worries me a bit:

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 50bd683..7a35c9d 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3698,6 +3698,8 @@ again:
                        goto again;
                }
                spin_unlock(&cur_trans->dirty_bgs_lock);
+       } else if (ret < 0) {
+               btrfs_cleanup_dirty_bgs(cur_trans, root);
        }

        btrfs_free_path(path);


We have checks in here to make sure only one process runs btrfs_start_dirty_block_groups() but that doesn't mean that only one process its messing around with the cache inode. Is there any reason we can't let this cleanup wait for the cleanup_transaction code?

btrfs_run_delayed_refs() already aborts when it fails.

-chris


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to