public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Adarsh Das <adarshdas950@gmail.com>, Qu Wenruo <wqu@suse.com>,
	David Sterba <dsterba@suse.com>, Sasha Levin <sashal@kernel.org>,
	clm@fb.com, linux-btrfs@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 6.19-5.15] btrfs: replace BUG() with error handling in __btrfs_balance()
Date: Mon, 23 Feb 2026 07:37:09 -0500	[thread overview]
Message-ID: <20260223123738.1532940-4-sashal@kernel.org> (raw)
In-Reply-To: <20260223123738.1532940-1-sashal@kernel.org>

From: Adarsh Das <adarshdas950@gmail.com>

[ Upstream commit be6324a809dbda76d5fdb23720ad9b20e5c1905c ]

We search with offset (u64)-1 which should never match exactly.
Previously this was handled with BUG(). Now logs an error
and return -EUCLEAN.

Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Adarsh Das <adarshdas950@gmail.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis

### What the commit does

This commit replaces a `BUG()` call with proper error handling in
`__btrfs_balance()`. The `BUG()` is triggered when `btrfs_search_slot()`
returns an exact match (ret == 0) for key offset `(u64)-1`, which should
theoretically never happen. The existing code has a `/* FIXME break ?
*/` comment from 2012, indicating the developers always knew `BUG()` was
wrong here.

The fix:
1. Replaces `BUG()` with `btrfs_err()` logging + return `-EUCLEAN`
2. Properly releases `reclaim_bgs_lock` mutex before `goto error`
   (fixing what would have been a mutex held across a panic)
3. Uses `unlikely()` to indicate this is an exceptional path

### Bug severity assessment

**The existing `BUG()` crashes the kernel** (panic/oops). While the
condition "should never happen," if it does occur (e.g., due to
filesystem corruption, a prior failed relocate as the comment says, or a
metadata inconsistency), the result is a full kernel crash instead of a
graceful error return. This is in the btrfs balance path, which is user-
triggered via `btrfs balance start`.

Key points:
- **BUG() = kernel crash** - This is a real fix that prevents a kernel
  panic
- **User-triggerable**: The balance operation is initiated by userspace,
  so a corrupted filesystem could trigger this crash
- **The fix is small and surgical**: Only changes the error handling for
  one condition
- **Properly handles mutex**: The new code correctly unlocks
  `reclaim_bgs_lock` before the error path
- **Well-reviewed**: Reviewed by Qu Wenruo and David Sterba (btrfs
  maintainer)
- **BUG() has existed since 2012** (commit c9e9f97bdfb64d), affecting
  all stable trees

### Stable criteria evaluation

- **Obviously correct**: Yes - replacing BUG() with error handling is
  well-understood
- **Fixes a real bug**: Yes - a kernel crash/panic on a theoretically-
  impossible-but-not-actually-impossible condition
- **Small and contained**: Yes - one file, simple logic change in a
  single function
- **No new features**: Correct - pure error handling improvement
- **Risk**: Very low - the only change is what happens when ret == 0,
  and the new behavior (return error) is strictly better than crashing

### Verification

- `git blame` confirmed the BUG() has been present since commit
  c9e9f97bdfb64d (2012, "Btrfs: add basic restriper infrastructure")
- Read the code at lines 4104-4116: confirmed `reclaim_bgs_lock` is held
  when BUG() fires, so the panic would also leave a mutex locked
- The `/* FIXME break ? */` comment confirms this was a known issue
- The new code properly calls `mutex_unlock()` before `goto error`,
  matching the pattern used at line 4107 for `ret < 0`
- Reviewed-by from Qu Wenruo (btrfs developer) and David Sterba (btrfs
  maintainer)
- The commit exists as be6324a809dbd in the tree, dated 2026-02-03
- The affected function `__btrfs_balance()` has existed for many years
  and is present in all stable trees

### Risk vs Benefit

- **Benefit**: Prevents kernel crash (BUG/panic) on a condition that
  could occur with corrupted filesystems
- **Risk**: Near-zero - the condition was previously a crash; now it's a
  graceful error return. No behavioral change for the normal (ret != 0)
  path.

This is a textbook stable candidate: a small, well-reviewed fix that
replaces a kernel crash with proper error handling in a long-standing
code path. BUG() removal in favor of error handling is one of the most
common and safest types of stable backports.

**YES**

 fs/btrfs/volumes.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 8a08412f3529a..14d988c3ef4f3 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4112,8 +4112,14 @@ static int __btrfs_balance(struct btrfs_fs_info *fs_info)
 		 * this shouldn't happen, it means the last relocate
 		 * failed
 		 */
-		if (ret == 0)
-			BUG(); /* FIXME break ? */
+		if (unlikely(ret == 0)) {
+			btrfs_err(fs_info,
+				  "unexpected exact match of CHUNK_ITEM in chunk tree, offset 0x%llx",
+				  key.offset);
+			mutex_unlock(&fs_info->reclaim_bgs_lock);
+			ret = -EUCLEAN;
+			goto error;
+		}
 
 		ret = btrfs_previous_item(chunk_root, path, 0,
 					  BTRFS_CHUNK_ITEM_KEY);
-- 
2.51.0


       reply	other threads:[~2026-02-23 12:37 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260223123738.1532940-1-sashal@kernel.org>
2026-02-23 12:37 ` Sasha Levin [this message]
2026-02-23 12:37 ` [PATCH AUTOSEL 6.19] btrfs: do not ASSERT() when the fs flips RO inside btrfs_repair_io_failure() Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260223123738.1532940-4-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=adarshdas950@gmail.com \
    --cc=clm@fb.com \
    --cc=dsterba@suse.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=wqu@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox