Linux-BTRFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] btrfs: destroy qgroup extent records on transaction abort
@ 2020-02-11  7:25 Qu Wenruo
  2020-02-12 13:06 ` Josef Bacik
  2020-02-14 17:03 ` David Sterba
  0 siblings, 2 replies; 3+ messages in thread
From: Qu Wenruo @ 2020-02-11  7:25 UTC (permalink / raw)
  To: linux-btrfs; +Cc: Jeff Mahoney

From: Jeff Mahoney <jeffm@suse.com>

We clean up the delayed references when we abort a transaction but
we leave the pending qgroup extent records behind, leaking memory.

This patch destroyes the extent records when we destroy the delayed
refs and checks to ensure they're gone before releasing the transaction.

Fixes: 3368d001ba5df (btrfs: qgroup: Record possible quota-related extent for qgroup.)
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
[Rebased to latest upstream, remove to_qgroup() helper, use
 rbtree_postorder_for_each_entry_safe() wrapper]
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
Changelog:
v2:
- Update the commit message to remove to_qgroup()
---
 fs/btrfs/disk-io.c     |  1 +
 fs/btrfs/qgroup.c      | 13 +++++++++++++
 fs/btrfs/qgroup.h      |  1 +
 fs/btrfs/transaction.c |  2 ++
 4 files changed, 17 insertions(+)

diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 7fa9bb79ad08..e8154e2747da 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -4275,6 +4275,7 @@ static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
 		cond_resched();
 		spin_lock(&delayed_refs->lock);
 	}
+	btrfs_qgroup_destroy_extent_records(trans);
 
 	spin_unlock(&delayed_refs->lock);
 
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 98d9a50352d6..184e3f66ee94 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -4002,3 +4002,16 @@ int btrfs_qgroup_trace_subtree_after_cow(struct btrfs_trans_handle *trans,
 	}
 	return ret;
 }
+
+void btrfs_qgroup_destroy_extent_records(struct btrfs_transaction *trans)
+{
+	struct btrfs_delayed_ref_root *delayed_refs = &trans->delayed_refs;
+	struct btrfs_qgroup_extent_record *entry;
+	struct btrfs_qgroup_extent_record *next;
+	struct rb_root *root = &delayed_refs->dirty_extent_root;
+
+	rbtree_postorder_for_each_entry_safe(entry, next, root, node) {
+		ulist_free(entry->old_roots);
+		kfree(entry);
+	}
+}
diff --git a/fs/btrfs/qgroup.h b/fs/btrfs/qgroup.h
index 236f12224d52..1bc654459469 100644
--- a/fs/btrfs/qgroup.h
+++ b/fs/btrfs/qgroup.h
@@ -414,5 +414,6 @@ int btrfs_qgroup_add_swapped_blocks(struct btrfs_trans_handle *trans,
 		u64 last_snapshot);
 int btrfs_qgroup_trace_subtree_after_cow(struct btrfs_trans_handle *trans,
 		struct btrfs_root *root, struct extent_buffer *eb);
+void btrfs_qgroup_destroy_extent_records(struct btrfs_transaction *trans);
 
 #endif
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 33dcc88b428a..beb6c69cd1e5 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -121,6 +121,8 @@ void btrfs_put_transaction(struct btrfs_transaction *transaction)
 		BUG_ON(!list_empty(&transaction->list));
 		WARN_ON(!RB_EMPTY_ROOT(
 				&transaction->delayed_refs.href_root.rb_root));
+		WARN_ON(!RB_EMPTY_ROOT(
+				&transaction->delayed_refs.dirty_extent_root));
 		if (transaction->delayed_refs.pending_csums)
 			btrfs_err(transaction->fs_info,
 				  "pending csums is %llu",
-- 
2.25.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] btrfs: destroy qgroup extent records on transaction abort
  2020-02-11  7:25 [PATCH v2] btrfs: destroy qgroup extent records on transaction abort Qu Wenruo
@ 2020-02-12 13:06 ` Josef Bacik
  2020-02-14 17:03 ` David Sterba
  1 sibling, 0 replies; 3+ messages in thread
From: Josef Bacik @ 2020-02-12 13:06 UTC (permalink / raw)
  To: Qu Wenruo, linux-btrfs; +Cc: Jeff Mahoney

On 2/11/20 2:25 AM, Qu Wenruo wrote:
> From: Jeff Mahoney <jeffm@suse.com>
> 
> We clean up the delayed references when we abort a transaction but
> we leave the pending qgroup extent records behind, leaking memory.
> 
> This patch destroyes the extent records when we destroy the delayed
> refs and checks to ensure they're gone before releasing the transaction.
> 
> Fixes: 3368d001ba5df (btrfs: qgroup: Record possible quota-related extent for qgroup.)
> Signed-off-by: Jeff Mahoney <jeffm@suse.com>
> [Rebased to latest upstream, remove to_qgroup() helper, use
>   rbtree_postorder_for_each_entry_safe() wrapper]
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] btrfs: destroy qgroup extent records on transaction abort
  2020-02-11  7:25 [PATCH v2] btrfs: destroy qgroup extent records on transaction abort Qu Wenruo
  2020-02-12 13:06 ` Josef Bacik
@ 2020-02-14 17:03 ` David Sterba
  1 sibling, 0 replies; 3+ messages in thread
From: David Sterba @ 2020-02-14 17:03 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs, Jeff Mahoney

On Tue, Feb 11, 2020 at 03:25:37PM +0800, Qu Wenruo wrote:
> From: Jeff Mahoney <jeffm@suse.com>
> 
> We clean up the delayed references when we abort a transaction but
> we leave the pending qgroup extent records behind, leaking memory.
> 
> This patch destroyes the extent records when we destroy the delayed
> refs and checks to ensure they're gone before releasing the transaction.
> 
> Fixes: 3368d001ba5df (btrfs: qgroup: Record possible quota-related extent for qgroup.)
> Signed-off-by: Jeff Mahoney <jeffm@suse.com>
> [Rebased to latest upstream, remove to_qgroup() helper, use
>  rbtree_postorder_for_each_entry_safe() wrapper]
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> Changelog:
> v2:
> - Update the commit message to remove to_qgroup()

Added to misc-next, thanks.

> +void btrfs_qgroup_destroy_extent_records(struct btrfs_transaction *trans)
> +{
> +	struct btrfs_delayed_ref_root *delayed_refs = &trans->delayed_refs;
> +	struct btrfs_qgroup_extent_record *entry;
> +	struct btrfs_qgroup_extent_record *next;
> +	struct rb_root *root = &delayed_refs->dirty_extent_root;

I've removed the delayed_refs varaible indirection:

	root = &trans->delayed_refs.dirty_extent_root;

> +	rbtree_postorder_for_each_entry_safe(entry, next, root, node) {
> +		ulist_free(entry->old_roots);
> +		kfree(entry);
> +	}
> +}

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-11  7:25 [PATCH v2] btrfs: destroy qgroup extent records on transaction abort Qu Wenruo
2020-02-12 13:06 ` Josef Bacik
2020-02-14 17:03 ` David Sterba

Linux-BTRFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-btrfs/0 linux-btrfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-btrfs linux-btrfs/ https://lore.kernel.org/linux-btrfs \
		linux-btrfs@vger.kernel.org
	public-inbox-index linux-btrfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-btrfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git