From patchwork Mon Mar 10 20:07:01 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boris Burkov X-Patchwork-Id: 14010609 Received: from fhigh-b2-smtp.messagingengine.com (fhigh-b2-smtp.messagingengine.com [202.12.124.153]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 40F671D516F for ; Mon, 10 Mar 2025 20:06:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.153 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741637182; cv=none; b=KXWhj0xYrhGjbQa0sTBF55ADsCZIt47+TyiWWpevfu9rvsJFnOom6s+1yLN0HbZnj5OsIcV2jumj30GLQiz2nbCWZ5AqTbRYHz2ZtqcZO29Iza3Gb/g4CX9FL6Fen/9NhMGnFrvO4tWfhbjTlYB1Zc3mK03aPaiDfnhvXx0avLU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741637182; c=relaxed/simple; bh=aBOccwu3njph4qI3KdmxQ73Wn4HiwIA1XiT3mWFDgPE=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=QMryFfl08G+/9J7I/7gLWqx8okxSgMUKKO7sym7Aa0NTPwzAdLGbUNmTIceHkGCbmf5W0k3mJW5f6yMIblMNougMSwGR+s2+6QCt1hvzMDOJkT38xOQ6Qj6zU75ccQL5BqluzpzO0/ruzbt4NJH6SggIo3E9cmDjuRIV6vXTbLU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=bur.io; spf=pass smtp.mailfrom=bur.io; dkim=pass (2048-bit key) header.d=bur.io header.i=@bur.io header.b=oa84xG+K; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=ltfjzk7H; arc=none smtp.client-ip=202.12.124.153 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=bur.io Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bur.io Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bur.io header.i=@bur.io header.b="oa84xG+K"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="ltfjzk7H" Received: from phl-compute-07.internal (phl-compute-07.phl.internal [10.202.2.47]) by mailfhigh.stl.internal (Postfix) with ESMTP id 572EB25401FD; Mon, 10 Mar 2025 16:06:19 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-07.internal (MEProxy); Mon, 10 Mar 2025 16:06:19 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bur.io; h=cc :content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:subject:subject:to:to; s=fm2; t=1741637179; x= 1741723579; bh=XOkQByygIMeEPz4kNTEJu6JOZwkpBTKy+JPLWK2W5T4=; b=o a84xG+K+GlAVq2EwvP0UfGZGRF9zH7Fgw5Ak/iBNZoZjNxScdC2TfBOI8HQQCWSW hqRMznWKUjrdxAK540Bo+YtgiGmZ+FyTzUYxqoDPxVGDvht2JK1MV+jkVUTZq2xk hCRH8iAno9hzqIBIY5N5UcrYMVGnAVkfxjPSJJoA+jDCoz7q2iSxP3j2PJcz9JgH t+wW5N6hzMSxFFULDImFAXhr/CWoy53Z0+G7E4MEmWvG8N07Vn6CGzg/skXrm7mp UmSRqw086Fi338BIhODpxecODg7JGqR9kEU5qw19AxYSKbKBtgorxXScuReuVAGE hMTuVCWBw8Fjb5YxuHKXA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:date:feedback-id:feedback-id:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; t=1741637179; x=1741723579; bh=XOkQByygIMeEPz4kNTEJu6JOZwkp BTKy+JPLWK2W5T4=; b=ltfjzk7HCaPDi5qDzJTTNSGZb3eCWekDTFk9qdt/wtnR kSPo3Rclfbk6rG7sJUNc0rMdNK42MQc1hHCXw65FRVnTMRRhGqvMrjpeEbzdPV1G vBwhCqz20aCMdDYnT0mD2OmYyqIofkj8MS6veuUrBGxyL85Han5U4VK+sClm1SH4 t6M70cA2DDhkhg3K3g8bkuhhmH5O9xcBp9OsvxYBIhZyopVBTEB/Vkc46kXhCt8r UjPpb4JKNebcWDoFm6n1oSdrcjm+1JIb2GcWr9ijUxJkd0oq6yFtrOy1nHdiLEpJ yq+rPr8zeG5BlJrowU3PsLhbLMQRM+vTwegJSEJJcQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvddtvdeiucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucenucfjughrpefhvf fufffkofgjfhgggfestdekredtredttdenucfhrhhomhepuehorhhishcuuehurhhkohhv uceosghorhhishessghurhdrihhoqeenucggtffrrghtthgvrhhnpeeiueffuedvieeuje fhheeigfekvedujeejjeffvedvhedtudefiefhkeegueehleenucevlhhushhtvghrufhi iigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpegsohhrihhssegsuhhrrdhiohdpnh gspghrtghpthhtohepvddpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtoheplhhinhhu gidqsghtrhhfshesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehkvghrnh gvlhdqthgvrghmsehfsgdrtghomh X-ME-Proxy: Feedback-ID: i083147f8:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 10 Mar 2025 16:06:18 -0400 (EDT) From: Boris Burkov To: linux-btrfs@vger.kernel.org, kernel-team@fb.com Subject: [PATCH v2 1/5] btrfs: fix bg refcount race in btrfs_create_pending_block_groups Date: Mon, 10 Mar 2025 13:07:01 -0700 Message-ID: <498b58b794722c70eca58bf3fe46003c43e60aff.1741636986.git.boris@bur.io> X-Mailer: git-send-email 2.48.1 In-Reply-To: References: Precedence: bulk X-Mailing-List: linux-btrfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Block group creation is done in two phases, which results in a slightly unintiuitive property: a block group can be allocated/deallocated from after btrfs_make_block_group adds it to the space_info with btrfs_add_bg_to_space_info, but before creation is completely completed in btrfs_create_pending_block_groups. As a result, it is possible for a block group to go unused and have 'btrfs_mark_bg_unused' called on it concurrently with 'btrfs_create_pending_block_groups'. This causes a number of issues, which were fixed with the block group flag 'BLOCK_GROUP_FLAG_NEW'. However, this fix is not quite complete. Since it does not use the unused_bg_lock, it is possible for the following race to occur: btrfs_create_pending_block_groups btrfs_mark_bg_unused if list_empty // false list_del_init clear_bit else if (test_bit) // true list_move_tail And we get into the exact same broken ref count and invalid new_bgs state for transaction cleanup that BLOCK_GROUP_FLAG_NEW was designed to prevent. The broken refcount aspect will result in a warning like: [ 1272.943113] ------------[ cut here ]------------ [ 1272.943527] refcount_t: underflow; use-after-free. [ 1272.943967] WARNING: CPU: 1 PID: 61 at lib/refcount.c:28 refcount_warn_saturate+0xba/0x110 [ 1272.944731] Modules linked in: btrfs virtio_net xor zstd_compress raid6_pq null_blk [last unloaded: btrfs] [ 1272.945550] CPU: 1 UID: 0 PID: 61 Comm: kworker/u32:1 Kdump: loaded Tainted: G W 6.14.0-rc5+ #108 [ 1272.946368] Tainted: [W]=WARN [ 1272.946585] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux 1.16.3-1-1 04/01/2014 [ 1272.947273] Workqueue: btrfs_discard btrfs_discard_workfn [btrfs] [ 1272.947788] RIP: 0010:refcount_warn_saturate+0xba/0x110 [ 1272.948180] Code: 01 01 e8 e9 c7 a9 ff 0f 0b c3 cc cc cc cc 80 3d 3f 4a de 01 00 75 85 48 c7 c7 00 9b 9f 8f c6 05 2f 4a de 01 01 e8 c6 c7 a9 ff <0f> 0b c3 cc cc cc cc 80 3d 1d 4a de 01 00 0f 85 5e ff ff ff 48 c7 [ 1272.949532] RSP: 0018:ffffbf1200247df0 EFLAGS: 00010282 [ 1272.949901] RAX: 0000000000000000 RBX: ffffa14b00e3f800 RCX: 0000000000000000 [ 1272.950437] RDX: 0000000000000000 RSI: ffffbf1200247c78 RDI: 00000000ffffdfff [ 1272.950986] RBP: ffffa14b00dc2860 R08: 00000000ffffdfff R09: ffffffff90526268 [ 1272.951512] R10: ffffffff904762c0 R11: 0000000063666572 R12: ffffa14b00dc28c0 [ 1272.952024] R13: 0000000000000000 R14: ffffa14b00dc2868 R15: 000001285dcd12c0 [ 1272.952850] FS: 0000000000000000(0000) GS:ffffa14d33c40000(0000) knlGS:0000000000000000 [ 1272.953458] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 1272.953931] CR2: 00007f838cbda000 CR3: 000000010104e000 CR4: 00000000000006f0 [ 1272.954474] Call Trace: [ 1272.954655] [ 1272.954812] ? refcount_warn_saturate+0xba/0x110 [ 1272.955173] ? __warn.cold+0x93/0xd7 [ 1272.955487] ? refcount_warn_saturate+0xba/0x110 [ 1272.955816] ? report_bug+0xe7/0x120 [ 1272.956103] ? handle_bug+0x53/0x90 [ 1272.956424] ? exc_invalid_op+0x13/0x60 [ 1272.956700] ? asm_exc_invalid_op+0x16/0x20 [ 1272.957011] ? refcount_warn_saturate+0xba/0x110 [ 1272.957399] btrfs_discard_cancel_work.cold+0x26/0x2b [btrfs] [ 1272.957853] btrfs_put_block_group.cold+0x5d/0x8e [btrfs] [ 1272.958289] btrfs_discard_workfn+0x194/0x380 [btrfs] [ 1272.958729] process_one_work+0x130/0x290 [ 1272.959026] worker_thread+0x2ea/0x420 [ 1272.959335] ? __pfx_worker_thread+0x10/0x10 [ 1272.959644] kthread+0xd7/0x1c0 [ 1272.959872] ? __pfx_kthread+0x10/0x10 [ 1272.960172] ret_from_fork+0x30/0x50 [ 1272.960474] ? __pfx_kthread+0x10/0x10 [ 1272.960745] ret_from_fork_asm+0x1a/0x30 [ 1272.961035] [ 1272.961238] ---[ end trace 0000000000000000 ]--- Though we have seen them in the async discard workfn as well. It is most likely to happen after a relocation finishes which cancels discard, tears down the block group, etc. Fix this fully by taking the lock around the list_del_init + clear_bit so that the two are done atomically. Fixes: 0657b20c5a76 ("btrfs: fix use-after-free of new block group that became unused") Reviewed-by: Qu Wenruo Reviewed-by: Filipe Manana Signed-off-by: Boris Burkov --- fs/btrfs/block-group.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c index 64f0268dcf02..2db1497b58d9 100644 --- a/fs/btrfs/block-group.c +++ b/fs/btrfs/block-group.c @@ -2797,8 +2797,11 @@ void btrfs_create_pending_block_groups(struct btrfs_trans_handle *trans) /* Already aborted the transaction if it failed. */ next: btrfs_dec_delayed_refs_rsv_bg_inserts(fs_info); + + spin_lock(&fs_info->unused_bgs_lock); list_del_init(&block_group->bg_list); clear_bit(BLOCK_GROUP_FLAG_NEW, &block_group->runtime_flags); + spin_unlock(&fs_info->unused_bgs_lock); /* * If the block group is still unused, add it to the list of From patchwork Mon Mar 10 20:07:02 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boris Burkov X-Patchwork-Id: 14010610 Received: from fhigh-b2-smtp.messagingengine.com (fhigh-b2-smtp.messagingengine.com [202.12.124.153]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5DCC71DE2A6 for ; Mon, 10 Mar 2025 20:06:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.153 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741637184; cv=none; b=EBPDF4nNI2m6lcLLs0aj+DYftZZvzGLAnnDKbRpGHkBRDsKXjfPr1vbjir5O182qbVI3/vxIzK3qnUn8VNKiSOiaOAdaZVknFARp22lIG5t7cRq8tugY4/H7g40vXIKLuR9AmxVMwF9VGoADCHb5m37uSCE1qcvnhJ/08EOPHW8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741637184; c=relaxed/simple; bh=xGGT14VPhh/Uo+9JTKMT1PMatod+wRauSDI6CdGptjk=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=owWH8Tz2Cja8GhGkS871riDlMKnBr43jXEuhlBuYkhSxUshLPBdnEkVkZfnqXPNu47uaCjNFb7f4NgTsqd5tIBXiltYO7Hkjdhi4VKM2n56crdvC2R8hh5shKjCUzIeHlwy3xDNU3fYNrHGPXxxtEHSM2vz3WC0mnt7B+0QFhPc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=bur.io; spf=pass smtp.mailfrom=bur.io; dkim=pass (2048-bit key) header.d=bur.io header.i=@bur.io header.b=EjMc7Po7; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=LxzKDDun; arc=none smtp.client-ip=202.12.124.153 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=bur.io Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bur.io Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bur.io header.i=@bur.io header.b="EjMc7Po7"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="LxzKDDun" Received: from phl-compute-12.internal (phl-compute-12.phl.internal [10.202.2.52]) by mailfhigh.stl.internal (Postfix) with ESMTP id 5DA1A25401E9; Mon, 10 Mar 2025 16:06:21 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-12.internal (MEProxy); Mon, 10 Mar 2025 16:06:21 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bur.io; h=cc :content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:subject:subject:to:to; s=fm2; t=1741637181; x= 1741723581; bh=KHgs5nOLPLlVN3QM8ii9OZ6nLIRyyuZX2L3xIG4reOE=; b=E jMc7Po7xu+h4TFfOmEH5RnMUY6ilNyeI55nek6ezW+wLRV/RNQ5ijKsJyLNLxECQ F7AjrqHwHXy6jioBIRpNYcoEgfO7v3aSdO/V7M+9JgNCQ+fQhzQ2ZMzVo1XRm+N9 M7pdmEuhzsae/d9KBeTJQ/dMMwa3/Oh0jsa5GECjsulBfN9xHmx6zXAawre3kJTl sF3x0AVdKQzkyXr2Mw/T2TkqnEmaojNWtVdFN5tvfLuWtaCpsyXVjR1AMCR/k63m IgJEQEePDZ8/nsNci09cyru201nC+zdNVq87rUWazCHDrC3dXmbFBvOka1lNRI39 eZMt4OOSXjdd2anfqDExQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:date:feedback-id:feedback-id:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; t=1741637181; x=1741723581; bh=KHgs5nOLPLlVN3QM8ii9OZ6nLIRy yuZX2L3xIG4reOE=; b=LxzKDDunMI7JM0/yfxtsn3mvK/XfHuZsI0QvpV0/cUmA Lgfb+n6O0HM0pxsaUdm30GLfI2EpXl7xBOB7vldqNjQxC/zpI6LP8t+j7oBFkDrD KIGOIPAF3o5EoTENrFBaUBVzVzx+Mu6to7k8w1eGrJ9YIunZndV/0RB7DCp3msGX +MVNH4Ql6G2IICOOeB3x37OXj1lxlYwhl5hVayuROLn21TaeOaqqEkbVAmaA1Ypc SiVhV8sIMWCPTyAOHlrQYaDsFMKTXWsvqd/djA67TDrCgF1ZH4iae91TFjdxJXP8 rp5N/Hh2mvuBTbCZoihA0ZGbCBEDiaTMnO3JERq54w== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvddtvdeiucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucenucfjughrpefhvf fufffkofgjfhgggfestdekredtredttdenucfhrhhomhepuehorhhishcuuehurhhkohhv uceosghorhhishessghurhdrihhoqeenucggtffrrghtthgvrhhnpeeiueffuedvieeuje fhheeigfekvedujeejjeffvedvhedtudefiefhkeegueehleenucevlhhushhtvghrufhi iigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpegsohhrihhssegsuhhrrdhiohdpnh gspghrtghpthhtohepvddpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtoheplhhinhhu gidqsghtrhhfshesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehkvghrnh gvlhdqthgvrghmsehfsgdrtghomh X-ME-Proxy: Feedback-ID: i083147f8:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 10 Mar 2025 16:06:20 -0400 (EDT) From: Boris Burkov To: linux-btrfs@vger.kernel.org, kernel-team@fb.com Subject: [PATCH v2 2/5] btrfs: harden bg->bg_list against list_del races Date: Mon, 10 Mar 2025 13:07:02 -0700 Message-ID: <35f34b992427ea8a8c888d3e183b9ea024d1dfcc.1741636986.git.boris@bur.io> X-Mailer: git-send-email 2.48.1 In-Reply-To: References: Precedence: bulk X-Mailing-List: linux-btrfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 As far as I can tell, these calls of list_del_init on bg_list can not run concurrently with btrfs_mark_bg_unused or btrfs_mark_bg_to_reclaim, as they are in transaction error paths and situations where the block group is readonly. However, if there is any chance at all of racing with mark_bg_unused, or a different future user of bg_list, better to be safe than sorry. Otherwise we risk the following interleaving (bg_list refcount in parens) T1 (some random op) T2 (btrfs_mark_bg_unused) !list_empty(&bg->bg_list); (1) list_del_init(&bg->bg_list); (1) list_move_tail (1) btrfs_put_block_group (0) btrfs_delete_unused_bgs bg = list_first_entry list_del_init(&bg->bg_list); btrfs_put_block_group(bg); (-1) Ultimately, this results in a broken ref count that hits zero one deref early and the real final deref underflows the refcount, resulting in a WARNING. Reviewed-by: Qu Wenruo Signed-off-by: Boris Burkov Reviewed-by: Filipe Manana --- fs/btrfs/extent-tree.c | 8 ++++++++ fs/btrfs/transaction.c | 12 ++++++++++++ 2 files changed, 20 insertions(+) diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 5de1a1293c93..5ead2f4976e4 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -2868,7 +2868,15 @@ int btrfs_finish_extent_commit(struct btrfs_trans_handle *trans) block_group->length, &trimmed); + /* + * Not strictly necessary to lock, as the block_group should be + * read-only from btrfs_delete_unused_bgs. + */ + ASSERT(block_group->ro); + spin_lock(&fs_info->unused_bgs_lock); list_del_init(&block_group->bg_list); + spin_unlock(&fs_info->unused_bgs_lock); + btrfs_unfreeze_block_group(block_group); btrfs_put_block_group(block_group); diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c index db8fe291d010..470dfc3a1a5c 100644 --- a/fs/btrfs/transaction.c +++ b/fs/btrfs/transaction.c @@ -160,7 +160,13 @@ void btrfs_put_transaction(struct btrfs_transaction *transaction) cache = list_first_entry(&transaction->deleted_bgs, struct btrfs_block_group, bg_list); + /* + * Not strictly necessary to lock, as no other task will be using a + * block_group on the deleted_bgs list during a transaction abort. + */ + spin_lock(&transaction->fs_info->unused_bgs_lock); list_del_init(&cache->bg_list); + spin_unlock(&transaction->fs_info->unused_bgs_lock); btrfs_unfreeze_block_group(cache); btrfs_put_block_group(cache); } @@ -2096,7 +2102,13 @@ static void btrfs_cleanup_pending_block_groups(struct btrfs_trans_handle *trans) list_for_each_entry_safe(block_group, tmp, &trans->new_bgs, bg_list) { btrfs_dec_delayed_refs_rsv_bg_inserts(fs_info); + /* + * Not strictly necessary to lock, as no other task will be using a + * block_group on the new_bgs list during a transaction abort. + */ + spin_lock(&fs_info->unused_bgs_lock); list_del_init(&block_group->bg_list); + spin_unlock(&fs_info->unused_bgs_lock); } } From patchwork Mon Mar 10 20:07:03 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boris Burkov X-Patchwork-Id: 14010611 Received: from fhigh-b2-smtp.messagingengine.com (fhigh-b2-smtp.messagingengine.com [202.12.124.153]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9F4271DDA09 for ; Mon, 10 Mar 2025 20:06:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.153 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741637186; cv=none; b=iM+F35jNnQaVGFWlaDN2pButBoRq/59DhBfEoGUec/qGrPSw774ddGk/bvoCjtl7JUzfOlvIEaJz2t5Fv8RczBNnOUHblQin7bqvbtMF+rTKZjpoFovGSW669l/jTPUKl+LSaVD6VWXElF8eodJ4UJtjM5eUe6GU1Y+ZwElmQ0o= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741637186; c=relaxed/simple; bh=a8GliFKuwJozEkX8lQ9ghqkc/DOzPnF+sas7xSJdBmw=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=nYFrOWn1VLOV5Lce5xAwcZfl4dfvQ+DmlzBGyeOl2UjKKLMtqJYhhYd63GA4bl69Q+q7F3UaSVMpmBZ4Xoibq9qeOft5UFm85KBVVLQYirT2fKIv2sN5mV2YPo+ToZT3V5yfwIA/mr/RZUZMsjkL9S9t1PSgkv+jxYEjiPjJErM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=bur.io; spf=pass smtp.mailfrom=bur.io; dkim=pass (2048-bit key) header.d=bur.io header.i=@bur.io header.b=ULKLHpf8; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=d2Fnv2F3; arc=none smtp.client-ip=202.12.124.153 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=bur.io Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bur.io Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bur.io header.i=@bur.io header.b="ULKLHpf8"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="d2Fnv2F3" Received: from phl-compute-08.internal (phl-compute-08.phl.internal [10.202.2.48]) by mailfhigh.stl.internal (Postfix) with ESMTP id BAB922540100; Mon, 10 Mar 2025 16:06:23 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-08.internal (MEProxy); Mon, 10 Mar 2025 16:06:23 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bur.io; h=cc :content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:subject:subject:to:to; s=fm2; t=1741637183; x= 1741723583; bh=DVXts/98tHZ2vxZE+3Wgaw+MggJ8UwczYg8DvQBKv2M=; b=U LKLHpf8g1gJGgToFOKITgbMICb1Vm3pmIVkSlq38Jeg3IsDdcR82DUbTwHnCLw/y XA3/kiWkRhOg0wt1SGA/DVCzG9MBtsPpi4IAjCEk0NsKUD6v41uEdpnDyjeDYcod K79KjpEbrkz95XkUucre6V5EPZQIJJq/dnRVLBr7LtXqPsGUYemdGxhST2e8a//6 UZ+RFUHf9BauqCtbdlPC5s1PdaqRgPu7g6bG2TE0og41fVAy9FJWl+6eSVZFRFSc 0KHYvT9yVN0e5Fqxhx6Q8Jw2S/dq2xHUF3t8MjSZRrIh7Kd+ujyR5yScb/ThdJ/3 pjAOKnLv7ps3gQ/EqvD8Q== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:date:feedback-id:feedback-id:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; t=1741637183; x=1741723583; bh=DVXts/98tHZ2vxZE+3Wgaw+MggJ8 UwczYg8DvQBKv2M=; b=d2Fnv2F3MYhzwn0oQvIhS/941GL1fcFVIf7zGOmikpE7 VEizjKS1WWdqgocsfxSTRryitqIXrowBl8CpfdpNNkuM42Hc++5ad/ufIHzLTxhL rIzy4nd8QFMIXnOdroAKVWKnItZK3eZgTIPRSK/FC9vh4H6hXBa+zP4S+wIDVdmB ZpwdctETjQZxOmugC8D3CGrmoh9RL0N+3X19HqlGnpBYjJuC99BFzjv2GJJkFStj bMWHN3lBT3S3uVZkDwF2hVrFm82bjFOpnPMRndKlUpmjD2eYNLrToY7bXmlpseFF y4pqCVsCwNuxyjT8puwoTLi7LdwBRHIwyuijyEp+PA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvddtvdeiucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucenucfjughrpefhvf fufffkofgjfhgggfestdekredtredttdenucfhrhhomhepuehorhhishcuuehurhhkohhv uceosghorhhishessghurhdrihhoqeenucggtffrrghtthgvrhhnpeeiueffuedvieeuje fhheeigfekvedujeejjeffvedvhedtudefiefhkeegueehleenucevlhhushhtvghrufhi iigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpegsohhrihhssegsuhhrrdhiohdpnh gspghrtghpthhtohepvddpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtoheplhhinhhu gidqsghtrhhfshesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehkvghrnh gvlhdqthgvrghmsehfsgdrtghomh X-ME-Proxy: Feedback-ID: i083147f8:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 10 Mar 2025 16:06:23 -0400 (EDT) From: Boris Burkov To: linux-btrfs@vger.kernel.org, kernel-team@fb.com Subject: [PATCH v2 3/5] btrfs: make discard_workfn block_group ref explicit Date: Mon, 10 Mar 2025 13:07:03 -0700 Message-ID: X-Mailer: git-send-email 2.48.1 In-Reply-To: References: Precedence: bulk X-Mailing-List: linux-btrfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Currently, the async discard machinery owns a ref to the block_group when the block_group is queued on a discard list. However, to handle races with discard cancellation and the discard workfn, we have some cute logic to detect that the block_group is *currently* running in the workfn, to protect the workfn's usage amidst cancellation. As far as I can tell, this doesn't have any overt bugs (though finish_discard_pass and remove_from_discard_list racing can have a surprising outcome for the caller of remove_from_discard_list in that it is again added at the end) But it is needlessly complicated to rely on locking and the nullity of discard_ctl->block_group. Simplify this significantly by just taking a refcount while we are in the workfn and uncondtionally drop it in both the remove and workfn paths, regardless of if they race. Reviewed-by: Filipe Manana Signed-off-by: Boris Burkov --- fs/btrfs/discard.c | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/fs/btrfs/discard.c b/fs/btrfs/discard.c index e815d165cccc..d6eef4bd9e9d 100644 --- a/fs/btrfs/discard.c +++ b/fs/btrfs/discard.c @@ -167,13 +167,7 @@ static bool remove_from_discard_list(struct btrfs_discard_ctl *discard_ctl, block_group->discard_eligible_time = 0; queued = !list_empty(&block_group->discard_list); list_del_init(&block_group->discard_list); - /* - * If the block group is currently running in the discard workfn, we - * don't want to deref it, since it's still being used by the workfn. - * The workfn will notice this case and deref the block group when it is - * finished. - */ - if (queued && !running) + if (queued) btrfs_put_block_group(block_group); spin_unlock(&discard_ctl->lock); @@ -260,9 +254,10 @@ static struct btrfs_block_group *peek_discard_list( block_group->discard_cursor = block_group->start; block_group->discard_state = BTRFS_DISCARD_EXTENTS; } - discard_ctl->block_group = block_group; } if (block_group) { + btrfs_get_block_group(block_group); + discard_ctl->block_group = block_group; *discard_state = block_group->discard_state; *discard_index = block_group->discard_index; } @@ -493,9 +488,20 @@ static void btrfs_discard_workfn(struct work_struct *work) block_group = peek_discard_list(discard_ctl, &discard_state, &discard_index, now); - if (!block_group || !btrfs_run_discard_work(discard_ctl)) + if (!block_group) return; + if (!btrfs_run_discard_work(discard_ctl)) { + spin_lock(&discard_ctl->lock); + btrfs_put_block_group(block_group); + discard_ctl->block_group = NULL; + spin_unlock(&discard_ctl->lock); + return; + } if (now < block_group->discard_eligible_time) { + spin_lock(&discard_ctl->lock); + btrfs_put_block_group(block_group); + discard_ctl->block_group = NULL; + spin_unlock(&discard_ctl->lock); btrfs_discard_schedule_work(discard_ctl, false); return; } @@ -547,15 +553,7 @@ static void btrfs_discard_workfn(struct work_struct *work) spin_lock(&discard_ctl->lock); discard_ctl->prev_discard = trimmed; discard_ctl->prev_discard_time = now; - /* - * If the block group was removed from the discard list while it was - * running in this workfn, then we didn't deref it, since this function - * still owned that reference. But we set the discard_ctl->block_group - * back to NULL, so we can use that condition to know that now we need - * to deref the block_group. - */ - if (discard_ctl->block_group == NULL) - btrfs_put_block_group(block_group); + btrfs_put_block_group(block_group); discard_ctl->block_group = NULL; __btrfs_discard_schedule_work(discard_ctl, now, false); spin_unlock(&discard_ctl->lock); From patchwork Mon Mar 10 20:07:04 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boris Burkov X-Patchwork-Id: 14010612 Received: from fhigh-b2-smtp.messagingengine.com (fhigh-b2-smtp.messagingengine.com [202.12.124.153]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AF00B1DE4EF for ; Mon, 10 Mar 2025 20:06:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.153 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741637188; cv=none; b=tGzCJFykvpmVqcV4D4tJ8HAGPsoHq3eY3787gab9OrHE5j+Ovu7tkF12GTNsEMLkZzj1Ac6ugIyMaqTXxFKFUk6obimneQSrKAbyWYe6GJQphsiMetCVwd6B/4lcs1TRWSz+QC/nFG2so/XNJ52SJgWprfqAPg36Ye9M5NoGp10= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741637188; c=relaxed/simple; bh=60UpVM4h3IBiIqIK12f59Twv61/aMzVeCnf8i+hF2Hc=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=drtr75eMlTEB20+b8k8Fylmle9v61ay8/VSiN4OhYVgk6u9a+q7K6V0Iul+w3cq/+3ZApDsdJ/+e2+SXrVcx4aqOIYcAm8vNG5vKxkRXES1Hn5PE1xiPI2+jOGXZ8pARAIk5QWy+92JtRnIQIRJIAurna7D5ws3oLdenN69hYWo= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=bur.io; spf=pass smtp.mailfrom=bur.io; dkim=pass (2048-bit key) header.d=bur.io header.i=@bur.io header.b=nXrWrPuL; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=aydbTMs+; arc=none smtp.client-ip=202.12.124.153 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=bur.io Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bur.io Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bur.io header.i=@bur.io header.b="nXrWrPuL"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="aydbTMs+" Received: from phl-compute-03.internal (phl-compute-03.phl.internal [10.202.2.43]) by mailfhigh.stl.internal (Postfix) with ESMTP id B4B4325401FB; Mon, 10 Mar 2025 16:06:25 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-03.internal (MEProxy); Mon, 10 Mar 2025 16:06:25 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bur.io; h=cc :content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:subject:subject:to:to; s=fm2; t=1741637185; x= 1741723585; bh=ZvYkrBp8FBTjhczrjsDx3itZexfG8oeS+u5mj27MUQo=; b=n XrWrPuLiSoPiM6a9aSDDsIjLk+oxHID3jGx2bq8j4v/RaLQF7QWVXub8WQz9sycE o/StsoQc0XluYTMI6Q9O8Q4U1uSjBJSxBEuFv34tVUVLuJkAsBjEdrTVt/jeEjvI BG+yKgDuok9+iDMA+LRFtJ8TEYODIA3+lZvAQSWqmpTRc8kTBbxp01pWg1jgbebK fJsmvufpzrlgEHBEHKzuJDdJDn+Qy1C99e/1CAPezGCv/2TAwYOGDJOQjsWAMZ72 0CK8f7ejs1TynfAKX66v0XjhqgtK/3ArtGea1j2yIPTKUCfqfqx/ozikQA6rw1Uv qbTz2cT7JPfS/Eb6xhGrw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:date:feedback-id:feedback-id:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; t=1741637185; x=1741723585; bh=ZvYkrBp8FBTjhczrjsDx3itZexfG 8oeS+u5mj27MUQo=; b=aydbTMs++EeXR6SVcADSlJQItaCGNmTr55JQSLvAtMJb PxLsfqRKK6Z3WL472GD65QY8N+/c5iV834E5KOplfnqy9xzq2SmPUKf9FU/PmVXe 4u8KppDXVYDOOmyBjM+dleVtCs5fz0KoXwS8eenyPMVetniAX/Wl3HEGMJr6q9t/ EEG5eVx1ujMCFQXK7pFj+vu16I/JuKfmz6Kt4Ne8TLslDicUlaOk3lgFWjcZ+KTS 2C05iyfXWh9i8ajCBtVgoGqUoI1q/NlV9A/ZSCUf2WKFNl86b0OAEJEBKs3taICb CJQ01I/RG+jEXzmASQXJ6XaAx3rt4vPY1LnJgvi/ug== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvddtvdejucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucenucfjughrpefhvf fufffkofgjfhgggfestdekredtredttdenucfhrhhomhepuehorhhishcuuehurhhkohhv uceosghorhhishessghurhdrihhoqeenucggtffrrghtthgvrhhnpeeiueffuedvieeuje fhheeigfekvedujeejjeffvedvhedtudefiefhkeegueehleenucevlhhushhtvghrufhi iigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpegsohhrihhssegsuhhrrdhiohdpnh gspghrtghpthhtohepvddpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtoheplhhinhhu gidqsghtrhhfshesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehkvghrnh gvlhdqthgvrghmsehfsgdrtghomh X-ME-Proxy: Feedback-ID: i083147f8:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 10 Mar 2025 16:06:25 -0400 (EDT) From: Boris Burkov To: linux-btrfs@vger.kernel.org, kernel-team@fb.com Subject: [PATCH v2 4/5] btrfs: explicitly ref count block_group on new_bgs list Date: Mon, 10 Mar 2025 13:07:04 -0700 Message-ID: <5ea6c280fbe91b772802313847967ee4c2e89201.1741636986.git.boris@bur.io> X-Mailer: git-send-email 2.48.1 In-Reply-To: References: Precedence: bulk X-Mailing-List: linux-btrfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 All other users of the bg_list list_head inc the refcount when adding to a list and dec it when deleting from the list. Just for the sake of uniformity and to try to avoid refcounting bugs, do it for this list as well. This does not fix any known ref-counting bug, as the reference belongs to a single task (trans_handle is not shared and this represents trans_handle->new_bgs linkage) and will not lose its original refcount while that thread is running. And BLOCK_GROUP_FLAG_NEW protects against ref-counting errors "moving" the block group to the unused list without taking a ref. With that said, I still believe it is simpler to just hold the extra ref count for this list user as well. Reviewed-by: Filipe Manana Signed-off-by: Boris Burkov --- fs/btrfs/block-group.c | 2 ++ fs/btrfs/transaction.c | 1 + 2 files changed, 3 insertions(+) diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c index 2db1497b58d9..e4071897c9a8 100644 --- a/fs/btrfs/block-group.c +++ b/fs/btrfs/block-group.c @@ -2801,6 +2801,7 @@ void btrfs_create_pending_block_groups(struct btrfs_trans_handle *trans) spin_lock(&fs_info->unused_bgs_lock); list_del_init(&block_group->bg_list); clear_bit(BLOCK_GROUP_FLAG_NEW, &block_group->runtime_flags); + btrfs_put_block_group(block_group); spin_unlock(&fs_info->unused_bgs_lock); /* @@ -2939,6 +2940,7 @@ struct btrfs_block_group *btrfs_make_block_group(struct btrfs_trans_handle *tran } #endif + btrfs_get_block_group(cache); list_add_tail(&cache->bg_list, &trans->new_bgs); btrfs_inc_delayed_refs_rsv_bg_inserts(fs_info); diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c index 470dfc3a1a5c..f26a394a9ec5 100644 --- a/fs/btrfs/transaction.c +++ b/fs/btrfs/transaction.c @@ -2108,6 +2108,7 @@ static void btrfs_cleanup_pending_block_groups(struct btrfs_trans_handle *trans) */ spin_lock(&fs_info->unused_bgs_lock); list_del_init(&block_group->bg_list); + btrfs_put_block_group(block_group); spin_unlock(&fs_info->unused_bgs_lock); } } From patchwork Mon Mar 10 20:07:05 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boris Burkov X-Patchwork-Id: 14010613 Received: from fout-b1-smtp.messagingengine.com (fout-b1-smtp.messagingengine.com [202.12.124.144]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A10861DE895 for ; Mon, 10 Mar 2025 20:06:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=202.12.124.144 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741637190; cv=none; b=p7sGJCPCGcL+0o5WWqF0d4ZCQavW6ATeNFoSStPx3HDetZGY10b7z0WsDkdkIW4rZv1jZiAGm0W2UNuca37+IKjYZK5Lqm3+e3O2Rw11O0/KxQjnx8g5VAH+h8ImjQ5hGBWLAzaOLfkOR1svc3edde11JzYckb4wn06NxYqTSEg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741637190; c=relaxed/simple; bh=CCfC5MH4NejjmcRyQRtXMPbVgbLZQbv489BcXUtgoXA=; h=From:To:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=aYup2V05ikfjZCQNlvKwAtUuwlLFeK77UTmQMZTK8WnEc117Y56UHZf/pFqk4SR7cQkK/8PUTNPG/Vxgf47zqo/iZwXTTYIpvKLYnlywkEhM+/YhPjUsAbJvJg0iir+yU6c1Zs7dkqOnOikdnpS10t7ME/FinItR0YfVOpi9+ZM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=bur.io; spf=pass smtp.mailfrom=bur.io; dkim=pass (2048-bit key) header.d=bur.io header.i=@bur.io header.b=me/TJ6ww; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=fFi0aKAX; arc=none smtp.client-ip=202.12.124.144 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=bur.io Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bur.io Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bur.io header.i=@bur.io header.b="me/TJ6ww"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="fFi0aKAX" Received: from phl-compute-02.internal (phl-compute-02.phl.internal [10.202.2.42]) by mailfout.stl.internal (Postfix) with ESMTP id A59521140199; Mon, 10 Mar 2025 16:06:27 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-02.internal (MEProxy); Mon, 10 Mar 2025 16:06:27 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bur.io; h=cc :content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:subject:subject:to:to; s=fm2; t=1741637187; x= 1741723587; bh=uXeQn6WnNuRKv4Z8OM5zT0iFk4eHXSDsJY9qT48cFUc=; b=m e/TJ6wwUFMn4rCMyw+OHkhjBlx7yqIX2vdesm9Ow8WgzYgqKO2Sjwzuzq7EmKKdO XyO8UDXfHlP7gGjNdq1yTgSxoowx4sQ8Su/cdm6QffWvN0QZ/ALL/11bZiTq8btX BHiTkZ0RbBhCgpKEtvsC45YETNvnUVRsZaZV8EPKorLFXmOkge6OXzNFa0/vZnQJ v8fLVUiB0KG36nPxHTAs3MCq5Y+YMKUmiuD/GZhyAk/ptewgMkiRV7+Q1PNzEY5+ jo1amaVMjODBBzX5LUutLmuA0bxLGHsrTWzdd8GdvgUsQiT/BondCF0alm8ki2w0 sezvx1epRMeRe/vzeZ2Bg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:date:feedback-id:feedback-id:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; t=1741637187; x=1741723587; bh=uXeQn6WnNuRKv4Z8OM5zT0iFk4eH XSDsJY9qT48cFUc=; b=fFi0aKAXCPMaPpeHnu1JQGbcXqrKgocPtZ9ZWwpTLyFY nUMj2+NM21yN9WW5pGQ6WvhdzwUnN8NagOO9bxMATlVaoyFpb6rhcv+kUOruCG5Z Ud9f/jhjAY9XGiJzwwI/BxCHxHUI6ugggtQcGACjxIBDDzW+bh0CglHo7wjA/U7w SOGf6+2nt/AUnFiZ1iTDQqkkAOW+7p1conPia/PChOQL6WLCxAlE46ysGVebSZOA vDHmQ8PHDJZ3spzBKyR4KMhX9iMRZCY1Ej0AwWGKgzNkDyrLo6cEpYl4PMCKXPfV W/p/QFsBJUHY+MMZk10fq5MGD7wRhp9Jia22yfXGGA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgdduvddtvdejucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggv pdfurfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucenucfjughrpefhvf fufffkofgjfhgggfestdekredtredttdenucfhrhhomhepuehorhhishcuuehurhhkohhv uceosghorhhishessghurhdrihhoqeenucggtffrrghtthgvrhhnpeeiueffuedvieeuje fhheeigfekvedujeejjeffvedvhedtudefiefhkeegueehleenucevlhhushhtvghrufhi iigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpegsohhrihhssegsuhhrrdhiohdpnh gspghrtghpthhtohepvddpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtoheplhhinhhu gidqsghtrhhfshesvhhgvghrrdhkvghrnhgvlhdrohhrghdprhgtphhtthhopehkvghrnh gvlhdqthgvrghmsehfsgdrtghomh X-ME-Proxy: Feedback-ID: i083147f8:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 10 Mar 2025 16:06:27 -0400 (EDT) From: Boris Burkov To: linux-btrfs@vger.kernel.org, kernel-team@fb.com Subject: [PATCH v2 5/5] btrfs: codify pattern for adding block_group to bg_list Date: Mon, 10 Mar 2025 13:07:05 -0700 Message-ID: X-Mailer: git-send-email 2.48.1 In-Reply-To: References: Precedence: bulk X-Mailing-List: linux-btrfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Similar to mark_bg_unused and mark_bg_to_reclaim, we have a few places that use bg_list with refcounting, mostly for retrying failures to reclaim/delete unused. These have custom logic for handling locking and refcounting the bg_list properly, but they actually all want to do the same thing, so pull that logic out into a helper. Unfortunately, mark_bg_unused does still need the NEW flag to avoid prematurely marking stuff unused (even if refcount is fine, we don't want to mess with bg creation), so it cannot use the new helper. Reviewed-by: Filipe Manana Signed-off-by: Boris Burkov --- fs/btrfs/block-group.c | 54 +++++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c index e4071897c9a8..5b13e086c7a8 100644 --- a/fs/btrfs/block-group.c +++ b/fs/btrfs/block-group.c @@ -1455,6 +1455,31 @@ static bool clean_pinned_extents(struct btrfs_trans_handle *trans, return ret == 0; } +/* + * Link the block_group to a list via bg_list. + * + * Use this rather than list_add_tail directly to ensure proper respect + * to locking and refcounting. + * + * @bg: the block_group to link to the list. + * @list: the list to link it to. + * Returns: true if the bg was linked with a refcount bump and false otherwise. + */ +static bool btrfs_link_bg_list(struct btrfs_block_group *bg, struct list_head *list) +{ + struct btrfs_fs_info *fs_info = bg->fs_info; + bool added = false; + + spin_lock(&fs_info->unused_bgs_lock); + if (list_empty(&bg->bg_list)) { + btrfs_get_block_group(bg); + list_add_tail(&bg->bg_list, list); + added = true; + } + spin_unlock(&fs_info->unused_bgs_lock); + return added; +} + /* * Process the unused_bgs list and remove any that don't have any allocated * space inside of them. @@ -1570,8 +1595,7 @@ void btrfs_delete_unused_bgs(struct btrfs_fs_info *fs_info) * drop under the "next" label for the * fs_info->unused_bgs list. */ - btrfs_get_block_group(block_group); - list_add_tail(&block_group->bg_list, &retry_list); + btrfs_link_bg_list(block_group, &retry_list); trace_btrfs_skip_unused_block_group(block_group); spin_unlock(&block_group->lock); @@ -1968,20 +1992,8 @@ void btrfs_reclaim_bgs_work(struct work_struct *work) spin_unlock(&space_info->lock); next: - if (ret && !READ_ONCE(space_info->periodic_reclaim)) { - /* Refcount held by the reclaim_bgs list after splice. */ - spin_lock(&fs_info->unused_bgs_lock); - /* - * This block group might be added to the unused list - * during the above process. Move it back to the - * reclaim list otherwise. - */ - if (list_empty(&bg->bg_list)) { - btrfs_get_block_group(bg); - list_add_tail(&bg->bg_list, &retry_list); - } - spin_unlock(&fs_info->unused_bgs_lock); - } + if (ret && !READ_ONCE(space_info->periodic_reclaim)) + btrfs_link_bg_list(bg, &retry_list); btrfs_put_block_group(bg); mutex_unlock(&fs_info->reclaim_bgs_lock); @@ -2021,13 +2033,8 @@ void btrfs_mark_bg_to_reclaim(struct btrfs_block_group *bg) { struct btrfs_fs_info *fs_info = bg->fs_info; - spin_lock(&fs_info->unused_bgs_lock); - if (list_empty(&bg->bg_list)) { - btrfs_get_block_group(bg); + if (btrfs_link_bg_list(bg, &fs_info->reclaim_bgs)) trace_btrfs_add_reclaim_block_group(bg); - list_add_tail(&bg->bg_list, &fs_info->reclaim_bgs); - } - spin_unlock(&fs_info->unused_bgs_lock); } static int read_bg_from_eb(struct btrfs_fs_info *fs_info, const struct btrfs_key *key, @@ -2940,8 +2947,7 @@ struct btrfs_block_group *btrfs_make_block_group(struct btrfs_trans_handle *tran } #endif - btrfs_get_block_group(cache); - list_add_tail(&cache->bg_list, &trans->new_bgs); + btrfs_link_bg_list(cache, &trans->new_bgs); btrfs_inc_delayed_refs_rsv_bg_inserts(fs_info); set_avail_alloc_bits(fs_info, type);