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