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

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

[Adam Borowski]

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 <kilobyte@angband.pl>
---
 fs/btrfs/relocation.c | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 0ec8ffa..388216f 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4326,6 +4326,34 @@ static struct reloc_control *alloc_reloc_control(struct btrfs_fs_info *fs_info)
 }
 
 /*
+ * explain bit flags, prefixed by a '|' that'll be dropped
+ */
+static void describe_block_group_flags(char *buf, u64 flags)
+{
+	if (!flags)
+		*buf += sprintf(buf, "|NONE");
+	else {
+#define DESCRIBE_FLAG(f, d) \
+			if (flags & BTRFS_BLOCK_GROUP_##f) { \
+				buf += sprintf(buf, "|%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 += sprintf(buf, "|0x%llx", flags);
+	}
+	*buf = 0;
+}
+
+/*
  * function to relocate all extents in a block group.
  */
 int btrfs_relocate_block_group(struct btrfs_root *extent_root, u64 group_start)
@@ -4337,6 +4365,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[128];
 
 	rc = alloc_reloc_control(fs_info);
 	if (!rc)
@@ -4381,9 +4410,10 @@ int btrfs_relocate_block_group(struct btrfs_root *extent_root, u64 group_start)
 		goto out;
 	}
 
+	describe_block_group_flags(flags_str, rc->block_group->flags);
 	btrfs_info(extent_root->fs_info,
-		   "relocating block group %llu flags %llu",
-		   rc->block_group->key.objectid, rc->block_group->flags);
+		   "relocating block group %llu flags %s",
+		   rc->block_group->key.objectid, flags_str+1);
 
 	btrfs_wait_block_group_reservations(rc->block_group);
 	btrfs_wait_nocow_writers(rc->block_group);
-- 
2.10.2

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

[Holger Hoffstätte]

On 11/04/16 08:26, 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 <kilobyte@angband.pl>

Very helpful and works (for me) as advertised.

Tested-by: Holger Hoffstätte <holger@applied-asynchrony.com>

> ---
>  fs/btrfs/relocation.c | 34 ++++++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 0ec8ffa..388216f 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -4326,6 +4326,34 @@ static struct reloc_control *alloc_reloc_control(struct btrfs_fs_info *fs_info)
>  }
>  
>  /*
> + * explain bit flags, prefixed by a '|' that'll be dropped
> + */
> +static void describe_block_group_flags(char *buf, u64 flags)
> +{
> +	if (!flags)
> +		*buf += sprintf(buf, "|NONE");
> +	else {
> +#define DESCRIBE_FLAG(f, d) \
> +			if (flags & BTRFS_BLOCK_GROUP_##f) { \
> +				buf += sprintf(buf, "|%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 += sprintf(buf, "|0x%llx", flags);
> +	}
> +	*buf = 0;
> +}
> +
> +/*
>   * function to relocate all extents in a block group.
>   */
>  int btrfs_relocate_block_group(struct btrfs_root *extent_root, u64 group_start)
> @@ -4337,6 +4365,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[128];
>  
>  	rc = alloc_reloc_control(fs_info);
>  	if (!rc)
> @@ -4381,9 +4410,10 @@ int btrfs_relocate_block_group(struct btrfs_root *extent_root, u64 group_start)
>  		goto out;
>  	}
>  
> +	describe_block_group_flags(flags_str, rc->block_group->flags);
>  	btrfs_info(extent_root->fs_info,
> -		   "relocating block group %llu flags %llu",
> -		   rc->block_group->key.objectid, rc->block_group->flags);
> +		   "relocating block group %llu flags %s",
> +		   rc->block_group->key.objectid, flags_str+1);
>  
>  	btrfs_wait_block_group_reservations(rc->block_group);
>  	btrfs_wait_nocow_writers(rc->block_group);
> 

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

[David Sterba]

On Fri, Nov 04, 2016 at 08:26:54AM +0100, 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.

Ok. Below are some comments (and style nitpicks).

> Also, display unknown flags as hex.
> 
> Signed-off-by: Adam Borowski <kilobyte@angband.pl>
> ---
>  fs/btrfs/relocation.c | 34 ++++++++++++++++++++++++++++++++--
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 0ec8ffa..388216f 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -4326,6 +4326,34 @@ static struct reloc_control *alloc_reloc_control(struct btrfs_fs_info *fs_info)
>  }
>  
>  /*
> + * explain bit flags, prefixed by a '|' that'll be dropped
> + */
> +static void describe_block_group_flags(char *buf, u64 flags)
> +{
> +	if (!flags)

	if (!flags) {

> +		*buf += sprintf(buf, "|NONE");

Should it be just 'buf'? Otherwise this stores 5 after 'E', which gets
overwritten by 0 at the end but still. Unless I'm missing something.

	} else {

> +	else {
> +#define DESCRIBE_FLAG(f, d) \
> +			if (flags & BTRFS_BLOCK_GROUP_##f) { \
> +				buf += sprintf(buf, "|%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");

An #undef should go here.

> +		if (flags)
> +			buf += sprintf(buf, "|0x%llx", flags);
> +	}
> +	*buf = 0;
> +}
> +
> +/*
>   * function to relocate all extents in a block group.
>   */
>  int btrfs_relocate_block_group(struct btrfs_root *extent_root, u64 group_start)
> @@ -4337,6 +4365,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[128];

This could be a problem. The relocation can trigger deep call chains
when doing IO, that need stack. I'd rather avoid the static buffer,
please switch it to kmalloc. As this is not a critical allocation, the
fallback would be to print just the raw value as now.

Also, please use snprintf. This would need extra variable to track
number of already printed chars, but should be easy, schematically like:

	remaining = BUFFERSIZE;

	ret = snprintf(buf, remaining, "...");
	remaining -= ret;
	buf += ret;

>  
>  	rc = alloc_reloc_control(fs_info);
>  	if (!rc)
> @@ -4381,9 +4410,10 @@ int btrfs_relocate_block_group(struct btrfs_root *extent_root, u64 group_start)
>  		goto out;
>  	}
>  
> +	describe_block_group_flags(flags_str, rc->block_group->flags);
>  	btrfs_info(extent_root->fs_info,
> -		   "relocating block group %llu flags %llu",
> -		   rc->block_group->key.objectid, rc->block_group->flags);
> +		   "relocating block group %llu flags %s",
> +		   rc->block_group->key.objectid, flags_str+1);
>  
>  	btrfs_wait_block_group_reservations(rc->block_group);
>  	btrfs_wait_nocow_writers(rc->block_group);
> -- 
> 2.10.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Adam Borowski]

On Mon, Nov 07, 2016 at 05:58:41PM +0100, David Sterba wrote:
> On Fri, Nov 04, 2016 at 08:26:54AM +0100, 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.
> 
> Ok. Below are some comments (and style nitpicks).

[...]

> > +	char flags_str[128];
> 
> This could be a problem. The relocation can trigger deep call chains
> when doing IO, that need stack. I'd rather avoid the static buffer,
> please switch it to kmalloc.

Are the chains callers or callees of relocation?  I believe exclusively the
latter, as balance is a long process that wouldn't make sense inside a deep
chain -- am I right?  In that case, moving the printk to the display
function would localize the buffer, avoiding all complications of an alloc.

(v2 does alloc)

> As this is not a critical allocation, the fallback would be to print just
> the raw value as now.

If the kernel is that low on memory, is it reasonable to print an
unimportant message?  As I see, lots of btrfs code would just fall over and
die horribly, with BUG_ON or aborting; perhaps skipping the message would
be safer?

(v2 has a fallback message)

> Also, please use snprintf. This would need extra variable to track
> number of already printed chars, but should be easy

M'kay.  Even with all flags on (most are mutually exclusive) we won't get
anywhere close to 128 characters, but paranoia can't hurt.


Meow!
-- 
A MAP07 (Dead Simple) raspberry tincture recipe: 0.5l 95% alcohol, 1kg
raspberries, 0.4kg sugar; put into a big jar for 1 month.  Filter out and
throw away the fruits (can dump them into a cake, etc), let the drink age
at least 3-6 months.

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

[Adam Borowski]

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 <kilobyte@angband.pl>
---
 fs/btrfs/relocation.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 3 deletions(-)

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);
-- 
2.10.2

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

[Holger Hoffstätte]

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 <kilobyte@angband.pl>
[..]
>  
>  /*
> + * 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

[PATCH v3-onstack] btrfs: make block group flags in balance printks human-readable

[Adam Borowski]

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 <kilobyte@angband.pl>
---
 fs/btrfs/relocation.c | 41 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 38 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index c4af0cd..57d867d 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4333,6 +4333,43 @@ static struct reloc_control *alloc_reloc_control(struct btrfs_fs_info *fs_info)
 }
 
 /*
+ * printk the block group being relocated
+ */
+static void describe_relocation(struct btrfs_fs_info *fs_info,
+				struct btrfs_block_group_cache *block_group)
+{
+	char buf[128]; // prefixed by a '|' that'll be dropped
+	u64 flags = block_group->flags;
+
+	if (unlikely(!flags)) // shouldn't happen
+		strcpy(buf, "|NONE");
+	else {
+		char *bp = buf;
+#define DESCRIBE_FLAG(f, d) \
+		if (flags & BTRFS_BLOCK_GROUP_##f) { \
+			bp += snprintf(bp, buf - bp + sizeof(buf), "|%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 (unlikely(flags))
+			snprintf(buf, buf - bp + sizeof(buf), "|0x%llx", flags);
+#undef DESCRIBE_FLAG
+	}
+
+	btrfs_info(fs_info,
+		   "relocating block group %llu flags %s",
+		   block_group->key.objectid, buf+1);
+}
+
+/*
  * function to relocate all extents in a block group.
  */
 int btrfs_relocate_block_group(struct btrfs_root *extent_root, u64 group_start)
@@ -4388,9 +4425,7 @@ 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);
+	describe_relocation(extent_root->fs_info, rc->block_group);
 
 	btrfs_wait_block_group_reservations(rc->block_group);
 	btrfs_wait_nocow_writers(rc->block_group);
-- 
2.10.2

This is a version that uses a temp buffer on the stack, but does it in a
separate function so it doesn't cost us anything when deep call chains are
involved.  While balance that can trigger deep call chain, it's not called
deeply itself.

This approach is simpler than mucking with allocs and avoids code
duplication that would be needed for handling failed alloc.

Re: [PATCH v3-onstack] btrfs: make block group flags in balance printks human-readable

[David Sterba]

On Sat, Nov 12, 2016 at 12:59:59AM +0100, 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 <kilobyte@angband.pl>
> ---
>  fs/btrfs/relocation.c | 41 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index c4af0cd..57d867d 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -4333,6 +4333,43 @@ static struct reloc_control *alloc_reloc_control(struct btrfs_fs_info *fs_info)
>  }
>  
>  /*
> + * printk the block group being relocated
> + */
> +static void describe_relocation(struct btrfs_fs_info *fs_info,
> +				struct btrfs_block_group_cache *block_group)
> +{
> +	char buf[128]; // prefixed by a '|' that'll be dropped

Oh right, moving the buffer to the function is the right way.

As my main objection is addressed, we can proceed to the codinstyle.
Please don't use the // comments.

> +	u64 flags = block_group->flags;
> +
> +	if (unlikely(!flags)) // shouldn't happen

add explicit { ... } if there's an multi-statement else block, drop
'unlikely'

> +		strcpy(buf, "|NONE");
> +	else {
> +		char *bp = buf;

newline between declarations and code

> +#define DESCRIBE_FLAG(f, d) \
> +		if (flags & BTRFS_BLOCK_GROUP_##f) { \
> +			bp += snprintf(bp, buf - bp + sizeof(buf), "|%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 (unlikely(flags))

drop 'unlikely'

> +			snprintf(buf, buf - bp + sizeof(buf), "|0x%llx", flags);
> +#undef DESCRIBE_FLAG
> +	}
> +
> +	btrfs_info(fs_info,
> +		   "relocating block group %llu flags %s",
> +		   block_group->key.objectid, buf+1);

		   block_group->key.objectid, buf + 1);

space around a binary operator

> +}
> +
> +/*
>   * function to relocate all extents in a block group.
>   */
>  int btrfs_relocate_block_group(struct btrfs_root *extent_root, u64 group_start)
> @@ -4388,9 +4425,7 @@ 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);
> +	describe_relocation(extent_root->fs_info, rc->block_group);
>  
>  	btrfs_wait_block_group_reservations(rc->block_group);
>  	btrfs_wait_nocow_writers(rc->block_group);
> -- 
> 2.10.2
> 
> This is a version that uses a temp buffer on the stack, but does it in a
> separate function so it doesn't cost us anything when deep call chains are
> involved.  While balance that can trigger deep call chain, it's not called
> deeply itself.
> 
> This approach is simpler than mucking with allocs and avoids code
> duplication that would be needed for handling failed alloc.

Agreed.

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

[Adam Borowski]

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 <kilobyte@angband.pl>
---
 fs/btrfs/relocation.c | 42 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 39 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index c4af0cd..9a3abf3 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4333,6 +4333,44 @@ static struct reloc_control *alloc_reloc_control(struct btrfs_fs_info *fs_info)
 }
 
 /*
+ * printk the block group being relocated
+ */
+static void describe_relocation(struct btrfs_fs_info *fs_info,
+				struct btrfs_block_group_cache *block_group)
+{
+	char buf[128]; /* prefixed by a '|' that'll be dropped */
+	u64 flags = block_group->flags;
+
+	if (!flags) { /* shouldn't happen */
+		strcpy(buf, "|NONE");
+	} else {
+		char *bp = buf;
+
+#define DESCRIBE_FLAG(f, d) \
+		if (flags & BTRFS_BLOCK_GROUP_##f) { \
+			bp += snprintf(bp, buf - bp + sizeof(buf), "|%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)
+			snprintf(buf, buf - bp + sizeof(buf), "|0x%llx", flags);
+#undef DESCRIBE_FLAG
+	}
+
+	btrfs_info(fs_info,
+		   "relocating block group %llu flags %s",
+		   block_group->key.objectid, buf + 1);
+}
+
+/*
  * function to relocate all extents in a block group.
  */
 int btrfs_relocate_block_group(struct btrfs_root *extent_root, u64 group_start)
@@ -4388,9 +4426,7 @@ 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);
+	describe_relocation(extent_root->fs_info, rc->block_group);
 
 	btrfs_wait_block_group_reservations(rc->block_group);
 	btrfs_wait_nocow_writers(rc->block_group);
-- 
2.10.2

Re: [PATCH v4] btrfs: make block group flags in balance printks human-readable

[David Sterba]

On Mon, Nov 14, 2016 at 06:44:34PM +0100, 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 <kilobyte@angband.pl>

Added to 4.10 queue, thanks.