[PATCH 0/5] btrfs: fixes and cleanups around messages.c

[PATCH 0/5] btrfs: fixes and cleanups around messages.c

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Fix a couple bugs when building the fs state string for printing in
messages and a few related cleanups in this area.

Filipe Manana (5):
  btrfs: fix incomplete iteration over fs state when logging messages
  btrfs: fix duplicated code in fs state string in logged messages
  btrfs: add missing code for no delayed iput fs state in log messages
  btrfs: remove unncessary pointer increment in btrfs_state_to_string()
  btrfs: avoid overhead when logging messages if fs state is clean

 fs/btrfs/messages.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

-- 
2.47.2

[PATCH 1/5] btrfs: fix incomplete iteration over fs state when logging messages

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

In btrfs_state_to_string() we iterate over all the bits set in
fs_info->fs_state using for_each_set_bit(), which requires its third
argument to match the number of bits, but we pass it the number of bytes
of the type used by fs_info->fs_state, an unsigned long, which is 8 on
64 bits platforms and 4 on 32 bits platforms. This means we are not
iterating over bits in the ranges [8, 63] or [4, 31].

Fix this by passing BTRFS_FS_STATE_COUNT instead of sizeof(fs_state).
Note that we could pass BITS_PER_TYPE(fs_state), but since
BTRFS_FS_STATE_COUNT is 11 and we have no bits beyond that, it would
cause pointless iterations over fs_state.

This also makes pointless the checks for the bit being inside the limit
of BTRFS_FS_STATE_COUNT inside the loop.

Fixes: c067da878156 ("btrfs: add filesystems state details to error messages")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/messages.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/messages.c b/fs/btrfs/messages.c
index 198d1747c80a..6debf0d4acad 100644
--- a/fs/btrfs/messages.c
+++ b/fs/btrfs/messages.c
@@ -42,9 +42,8 @@ static void btrfs_state_to_string(const struct btrfs_fs_info *info, char *buf)
 		states_printed = true;
 	}
 
-	for_each_set_bit(bit, &fs_state, sizeof(fs_state)) {
-		WARN_ON_ONCE(bit >= BTRFS_FS_STATE_COUNT);
-		if ((bit < BTRFS_FS_STATE_COUNT) && fs_state_chars[bit]) {
+	for_each_set_bit(bit, &fs_state, BTRFS_FS_STATE_COUNT) {
+		if (fs_state_chars[bit]) {
 			*curr++ = fs_state_chars[bit];
 			states_printed = true;
 		}
-- 
2.47.2

[PATCH 2/5] btrfs: fix duplicated code in fs state string in logged messages

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

We are using the code 'E' when the fs is in an error state (transaction
aborted) and when it's in the emergency shutdown state, which is confusing
and not intentional. Fix this by using an unused code such as 'H' for the
emergency shutdown state and add a comment over the fs_state_chars array
to warn to not use 'E' as its use is not defined in the array but instead
in the function btrfs_state_to_string(), since the fs state error is not
defined as a bit but as an error code in fs_info->fs_error.

Fixes: 9b2839451dfc ("btrfs: introduce a new shutdown state")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/messages.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/messages.c b/fs/btrfs/messages.c
index 6debf0d4acad..5fe5d34ab629 100644
--- a/fs/btrfs/messages.c
+++ b/fs/btrfs/messages.c
@@ -13,6 +13,8 @@
 /*
  * Characters to print to indicate error conditions or uncommon filesystem state.
  * RO is not an error.
+ * Don't use 'E' because that's used to signal that there's an error stored in
+ * fs_info->fs_error (see btrfs_state_to_string() below).
  */
 static const char fs_state_chars[] = {
 	[BTRFS_FS_STATE_REMOUNTING]		= 'M',
@@ -24,7 +26,7 @@ static const char fs_state_chars[] = {
 	[BTRFS_FS_STATE_NO_DATA_CSUMS]		= 'C',
 	[BTRFS_FS_STATE_SKIP_META_CSUMS]	= 'S',
 	[BTRFS_FS_STATE_LOG_CLEANUP_ERROR]	= 'L',
-	[BTRFS_FS_STATE_EMERGENCY_SHUTDOWN]	= 'E',
+	[BTRFS_FS_STATE_EMERGENCY_SHUTDOWN]	= 'H',
 };
 
 static void btrfs_state_to_string(const struct btrfs_fs_info *info, char *buf)
-- 
2.47.2

[PATCH 3/5] btrfs: add missing code for no delayed iput fs state in log messages

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

Add a letter code for the BTRFS_FS_STATE_NO_DELAYED_IPUT fs state, which
is currently missing and it's useful to have.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/messages.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/btrfs/messages.c b/fs/btrfs/messages.c
index 5fe5d34ab629..2610b447165e 100644
--- a/fs/btrfs/messages.c
+++ b/fs/btrfs/messages.c
@@ -26,6 +26,7 @@ static const char fs_state_chars[] = {
 	[BTRFS_FS_STATE_NO_DATA_CSUMS]		= 'C',
 	[BTRFS_FS_STATE_SKIP_META_CSUMS]	= 'S',
 	[BTRFS_FS_STATE_LOG_CLEANUP_ERROR]	= 'L',
+	[BTRFS_FS_STATE_NO_DELAYED_IPUT]	= 'I',
 	[BTRFS_FS_STATE_EMERGENCY_SHUTDOWN]	= 'H',
 };
 
-- 
2.47.2

[PATCH 4/5] btrfs: remove unncessary pointer increment in btrfs_state_to_string()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

There's no point in incrementing the local pointer after we set the NUL
terminator, as we don't use the pointer anymore after that.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/messages.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/messages.c b/fs/btrfs/messages.c
index 2610b447165e..c42a12139377 100644
--- a/fs/btrfs/messages.c
+++ b/fs/btrfs/messages.c
@@ -56,7 +56,7 @@ static void btrfs_state_to_string(const struct btrfs_fs_info *info, char *buf)
 	if (!states_printed)
 		curr = buf;
 
-	*curr++ = 0;
+	*curr = 0;
 }
 #endif
 
-- 
2.47.2

[PATCH 5/5] btrfs: avoid overhead when logging messages if fs state is clean

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

If the fs state does not have any errors and is not in some special state
(like emergency shutdown or mounted with rescue options), then in
btrfs_state_to_string() we end up doing a memcpy() into the buffer and
then later just undo that work by setting the buffer's first element to
the NUL terminator, since fs_info->fs_error is 0 and there is no bit set
in fs_info->fs_state.

Improve on this by returning early if there's no error in the fs and the
fs state is clean, which is the expected scenario for a healthy fs that is
not mounted with special options like the rescue options.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/messages.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/messages.c b/fs/btrfs/messages.c
index c42a12139377..0704be7be871 100644
--- a/fs/btrfs/messages.c
+++ b/fs/btrfs/messages.c
@@ -33,27 +33,27 @@ static const char fs_state_chars[] = {
 static void btrfs_state_to_string(const struct btrfs_fs_info *info, char *buf)
 {
 	unsigned int bit;
-	bool states_printed = false;
 	unsigned long fs_state = READ_ONCE(info->fs_state);
 	char *curr = buf;
 
+	if (likely(BTRFS_FS_ERROR(info) == 0 && fs_state == 0)) {
+		*curr = 0;
+		return;
+	}
+
 	memcpy(curr, STATE_STRING_PREFACE, sizeof(STATE_STRING_PREFACE));
 	curr += sizeof(STATE_STRING_PREFACE) - 1;
 
-	if (unlikely(BTRFS_FS_ERROR(info))) {
+	if (BTRFS_FS_ERROR(info))
 		*curr++ = 'E';
-		states_printed = true;
-	}
 
 	for_each_set_bit(bit, &fs_state, BTRFS_FS_STATE_COUNT) {
-		if (fs_state_chars[bit]) {
+		if (fs_state_chars[bit])
 			*curr++ = fs_state_chars[bit];
-			states_printed = true;
-		}
 	}
 
-	/* If no states were printed, reset the buffer */
-	if (!states_printed)
+	/* If nothing printed, ensure we return an empty string. */
+	if (curr == (buf + sizeof(STATE_STRING_PREFACE) - 1))
 		curr = buf;
 
 	*curr = 0;
-- 
2.47.2

Re: [PATCH 0/5] btrfs: fixes and cleanups around messages.c

[Qu Wenruo]

在 2026/9/30 23:55, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
> 
> Fix a couple bugs when building the fs state string for printing in
> messages and a few related cleanups in this area.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> 
> Filipe Manana (5):
>    btrfs: fix incomplete iteration over fs state when logging messages
>    btrfs: fix duplicated code in fs state string in logged messages
>    btrfs: add missing code for no delayed iput fs state in log messages
>    btrfs: remove unncessary pointer increment in btrfs_state_to_string()
>    btrfs: avoid overhead when logging messages if fs state is clean
> 
>   fs/btrfs/messages.c | 28 +++++++++++++++-------------
>   1 file changed, 15 insertions(+), 13 deletions(-)
>