[v2] btrfs: make block group flags in balance printks human-readable

Commit Message

Adam Borowski Nov. 7, 2016, 9:40 p.m. UTC

They're not even documented anywhere, letting users with no recourse but
to RTFS.  It's no big burden to output the bitfield as words.

Also, display unknown flags as hex.

Signed-off-by: Adam Borowski <[e-mail]>
---
 fs/btrfs/relocation.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 3 deletions(-)

Comments

Holger Hoffstätte Nov. 8, 2016, 1:42 p.m. UTC | #1

On 11/07/16 22:40, Adam Borowski wrote:
> They're not even documented anywhere, letting users with no recourse but
> to RTFS.  It's no big burden to output the bitfield as words.
> 
> Also, display unknown flags as hex.
> 
> Signed-off-by: Adam Borowski <[e-mail]>
[..]
>  
>  /*
> + * explain bit flags, prefixed by a '|' that'll be dropped
> + */
> +static char *describe_block_group_flags(char *buf, u64 flags)
> +{
> +#define BUF_SIZE 128
> +	char *buf0 = buf = kmalloc(BUF_SIZE, GFP_NOFS);
[..]

Maybe I'm missing some clever (?) trick here, but what's the point of passing
in a potentially uninitialized 'buf' when it's immediately reassigned locally,
and a new value is returned and assigned at the call site?
IMHO you'd probably either want to pass the buffer in or return it, but not
both - and in that case the allocation should probably be hoisted out
into the caller as well, if only to make things a bit more symmetric.

-h

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index c4af0cd..b5d2a00 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4333,6 +4333,42 @@  static struct reloc_control *alloc_reloc_control(struct btrfs_fs_info *fs_info)
 }
 
 /*
+ * explain bit flags, prefixed by a '|' that'll be dropped
+ */
+static char *describe_block_group_flags(char *buf, u64 flags)
+{
+#define BUF_SIZE 128
+	char *buf0 = buf = kmalloc(BUF_SIZE, GFP_NOFS);
+
+	if (!buf)
+		return 0;
+
+	if (!flags) {
+		strcpy(buf, "|NONE");
+		return buf0;
+	}
+#define DESCRIBE_FLAG(f, d) \
+		if (flags & BTRFS_BLOCK_GROUP_##f) { \
+			buf += snprintf(buf, buf0 - buf + BUF_SIZE, "|%s", d); \
+			flags &= ~BTRFS_BLOCK_GROUP_##f; \
+		}
+	DESCRIBE_FLAG(DATA,     "data");
+	DESCRIBE_FLAG(SYSTEM,   "system");
+	DESCRIBE_FLAG(METADATA, "metadata");
+	DESCRIBE_FLAG(RAID0,    "raid0");
+	DESCRIBE_FLAG(RAID1,    "raid1");
+	DESCRIBE_FLAG(DUP,      "dup");
+	DESCRIBE_FLAG(RAID10,   "raid10");
+	DESCRIBE_FLAG(RAID5,    "raid5");
+	DESCRIBE_FLAG(RAID6,    "raid6");
+	if (flags)
+		buf += snprintf(buf, buf0 - buf + BUF_SIZE, "|0x%llx", flags);
+	return buf0;
+#undef DESCRIBE_FLAG
+#undef BUF_SIZE
+}
+
+/*
  * function to relocate all extents in a block group.
  */
 int btrfs_relocate_block_group(struct btrfs_root *extent_root, u64 group_start)
@@ -4344,6 +4380,7 @@  int btrfs_relocate_block_group(struct btrfs_root *extent_root, u64 group_start)
 	int ret;
 	int rw = 0;
 	int err = 0;
+	char *flags_str;
 
 	rc = alloc_reloc_control(fs_info);
 	if (!rc)
@@ -4388,9 +4425,19 @@  int btrfs_relocate_block_group(struct btrfs_root *extent_root, u64 group_start)
 		goto out;
 	}
 
-	btrfs_info(extent_root->fs_info,
-		   "relocating block group %llu flags %llu",
-		   rc->block_group->key.objectid, rc->block_group->flags);
+	if ((flags_str = describe_block_group_flags(flags_str,
+						   rc->block_group->flags))) {
+		btrfs_info(extent_root->fs_info,
+			   "relocating block group %llu flags %s",
+			   rc->block_group->key.objectid,
+			   flags_str+1);
+		kfree(flags_str);
+	} else {
+		btrfs_info(extent_root->fs_info,
+			   "relocating block group %llu flags %llx",
+			   rc->block_group->key.objectid,
+			   rc->block_group->flags);
+	}
 
 	btrfs_wait_block_group_reservations(rc->block_group);
 	btrfs_wait_nocow_writers(rc->block_group);