[PATCH v5 0/3] btrfs: balance: improve kernel logs

[PATCH v5 0/3] btrfs: balance: improve kernel logs

[Anand Jain]

v4->v5:
 Mainly address David review comment [1].
 [1]
  https://patchwork.kernel.org/patch/10425987/
 pls ref to individual patch 2/3 for details.

v3->v4:
 Pls ref to individual patches.

Based on misc-next.

v2->v3:
  Inspried by describe_relocation(), improves it, makes it a helper
  function and use it to log the balance operations.

Kernel logs are very important for the forensic investigations of the
issues, these patchs make balance logs easy to review.

Anand Jain (3):
  btrfs: add helper function describe_block_group()
  btrfs: balance: add args info during start and resume
  btrfs: balance: add kernel log for end or paused

 fs/btrfs/relocation.c |  30 +------
 fs/btrfs/volumes.c    | 222 +++++++++++++++++++++++++++++++++++++++++++++++++-
 fs/btrfs/volumes.h    |   1 +
 3 files changed, 223 insertions(+), 30 deletions(-)

-- 
1.8.3.1

[PATCH v5 1/3] btrfs: add helper function describe_block_group()

[Anand Jain]

Improve on describe_relocation() add a common helper function to describe
the block groups.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
Reviewed-by: David Sterba <dsterba@suse.com>
---
v4.1->v5: Initialize buf[128] to null.
v4->v4.1: Use strcpy(buf, "|NONE"); as in the original
v3->v4: Just pass full flag name in the define DESCRIBE_FLAG(flag,..),
	 so that it can be used at couple of more places.
	Rename describe_block_groups() to btrfs_describe_block_groups().
	Drop useless return u32.
v3:	Born.

 fs/btrfs/relocation.c | 30 +++---------------------------
 fs/btrfs/volumes.c    | 43 +++++++++++++++++++++++++++++++++++++++++++
 fs/btrfs/volumes.h    |  1 +
 3 files changed, 47 insertions(+), 27 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 924116f654a1..0373b3cc1d36 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4184,37 +4184,13 @@ static struct reloc_control *alloc_reloc_control(void)
 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;
+	char buf[128] = {'\0'};
 
-	/* Shouldn't happen */
-	if (!flags) {
-		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(bp, buf - bp + sizeof(buf), "|0x%llx", flags);
-#undef DESCRIBE_FLAG
-	}
+	btrfs_describe_block_groups(block_group->flags, buf, sizeof(buf));
 
 	btrfs_info(fs_info,
 		   "relocating block group %llu flags %s",
-		   block_group->key.objectid, buf + 1);
+		   block_group->key.objectid, buf);
 }
 
 /*
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index f435d397019e..12b3d0625c6a 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -123,6 +123,49 @@ const char *get_raid_name(enum btrfs_raid_types type)
 	return btrfs_raid_array[type].raid_name;
 }
 
+void btrfs_describe_block_groups(u64 bg_flags, char *buf, u32 size_buf)
+{
+	int i;
+	int ret;
+	char *bp = buf;
+	u64 flags = bg_flags;
+	u32 size_bp = size_buf;
+
+	if (!flags) {
+		strcpy(bp, "NONE");
+		return;
+	}
+
+#define DESCRIBE_FLAG(f, d) \
+	do { \
+		if (flags & f) { \
+			ret = snprintf(bp, size_bp, "%s|", d); \
+			if (ret < 0 || ret >= size_bp) \
+				return; \
+			size_bp -= ret; \
+			bp += ret; \
+			flags &= ~f; \
+		} \
+	} while (0)
+
+	DESCRIBE_FLAG(BTRFS_BLOCK_GROUP_DATA, "data");
+	DESCRIBE_FLAG(BTRFS_BLOCK_GROUP_SYSTEM, "system");
+	DESCRIBE_FLAG(BTRFS_BLOCK_GROUP_METADATA, "metadata");
+	DESCRIBE_FLAG(BTRFS_AVAIL_ALLOC_BIT_SINGLE, "single");
+	for (i = 0; i < BTRFS_NR_RAID_TYPES; i++)
+		DESCRIBE_FLAG(btrfs_raid_array[i].bg_flag,
+			      btrfs_raid_array[i].raid_name);
+#undef DESCRIBE_FLAG
+
+	if (flags) {
+		ret = snprintf(bp, size_bp, "0x%llx|", flags);
+		size_bp -= ret;
+	}
+
+	if (size_bp < size_buf)
+		buf[size_buf - size_bp - 1] = '\0'; /* remove last | */
+}
+
 static int init_first_rw_device(struct btrfs_trans_handle *trans,
 				struct btrfs_fs_info *fs_info);
 static int btrfs_relocate_sys_chunks(struct btrfs_fs_info *fs_info);
diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
index aefce895e994..3e914effcdf6 100644
--- a/fs/btrfs/volumes.h
+++ b/fs/btrfs/volumes.h
@@ -430,6 +430,7 @@ struct btrfs_device *btrfs_find_device(struct btrfs_fs_info *fs_info, u64 devid,
 int btrfs_balance(struct btrfs_fs_info *fs_info,
 		  struct btrfs_balance_control *bctl,
 		  struct btrfs_ioctl_balance_args *bargs);
+void btrfs_describe_block_groups(u64 flags, char *buf, u32 size_buf);
 int btrfs_resume_balance_async(struct btrfs_fs_info *fs_info);
 int btrfs_recover_balance(struct btrfs_fs_info *fs_info);
 int btrfs_pause_balance(struct btrfs_fs_info *fs_info);
-- 
1.8.3.1

[PATCH v5 2/3] btrfs: balance: add args info during start and resume

[Anand Jain]

Balance arg info is an important information to be reviewed for the
system audit. So this patch adds them to the kernel log.

Example:
->btrfs bal start -f -mprofiles=raid1,convert=single,soft -dlimit=10..20,usage=50 /btrfs

kernel: BTRFS info (device sdb): balance: start -f -dusage=50,limit=10..20 -msoft,profiles=raid1,convert=single -ssoft,profiles=raid1,convert=single

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v4.1->v5: Per David review comment the code..
           bp += snprintf(bp, buf - bp + size_buf, "soft,");
	  is not safe if 'buf - bp + size_buf' becomes accidentally
	  negative, so fix this by using following snprintf.

 #define CHECK_UPDATE_BP_1(a) \
       do { \
               ret = snprintf(bp, size_bp, a); \
               if (ret < 0 || ret >= size_bp) \
                       goto out; \
               size_bp -= ret; \
               bp += ret; \
       } while (0)

	And initialize the tmp_buf to null.

v4->v4.1: Rename last missed sz to size_buf.

v3->v4: Change log updated with new example.
	Log format is changed to almost match with the cli.
	snprintf drop %s for inline string.
	Print range as =%u..%u instead of min=%umax=%u.
	soft is under the args now.
	force is printed as -f.

v2->v3: Change log updated.
        Make use of describe_block_group() added in 1/4
        Drop using strcat instead use snprintf.
        Logs string format updated as shown in the example above.

v1->v2: Change log update.
        Move adding the logs for balance complete and end to a new patch

 fs/btrfs/volumes.c | 172 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 169 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 12b3d0625c6a..8397f72bdac7 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -3754,6 +3754,173 @@ static inline int validate_convert_profile(struct btrfs_balance_args *bctl_arg,
 		 (bctl_arg->target & ~allowed)));
 }
 
+static void describe_balance_args(struct btrfs_balance_args *bargs, char *buf,
+				 u32 size_buf)
+{
+	int ret;
+	u32 size_bp = size_buf;
+	char *bp = buf;
+	u64 flags = bargs->flags;
+	char tmp_buf[128] = {'\0'};
+
+	if (!flags)
+		return;
+
+#define CHECK_UPDATE_BP_1(a) \
+	do { \
+		ret = snprintf(bp, size_bp, a); \
+		if (ret < 0 || ret >= size_bp) \
+			goto out; \
+		size_bp -= ret; \
+		bp += ret; \
+	} while (0)
+
+#define CHECK_UPDATE_BP_2(a, v1) \
+	do { \
+		ret = snprintf(bp, size_bp, a, v1); \
+		if (ret < 0 || ret >= size_bp) \
+			goto out; \
+		size_bp -= ret; \
+		bp += ret; \
+	} while (0)
+
+#define CHECK_UPDATE_BP_3(a, v1, v2) \
+	do { \
+		ret = snprintf(bp, size_bp, a, v1, v2); \
+		if (ret < 0 || ret >= size_bp) \
+			goto out; \
+		size_bp -= ret; \
+		bp += ret; \
+	} while (0)
+
+	if (flags & BTRFS_BALANCE_ARGS_SOFT) {
+		CHECK_UPDATE_BP_1("soft,");
+	}
+
+	if (flags & BTRFS_BALANCE_ARGS_PROFILES) {
+		btrfs_describe_block_groups(bargs->profiles, tmp_buf,
+					    sizeof(tmp_buf));
+		CHECK_UPDATE_BP_2("profiles=%s,", tmp_buf);
+	}
+
+	if (flags & BTRFS_BALANCE_ARGS_USAGE) {
+		CHECK_UPDATE_BP_2("usage=%llu,", bargs->usage);
+	}
+
+	if (flags & BTRFS_BALANCE_ARGS_USAGE_RANGE) {
+		CHECK_UPDATE_BP_3("usage=%u..%u,",
+				  bargs->usage_min, bargs->usage_max);
+	}
+
+	if (flags & BTRFS_BALANCE_ARGS_DEVID) {
+		CHECK_UPDATE_BP_2("devid=%llu,", bargs->devid);
+	}
+
+	if (flags & BTRFS_BALANCE_ARGS_DRANGE) {
+		CHECK_UPDATE_BP_3("drange=%llu..%llu,",
+				  bargs->pstart, bargs->pend);
+	}
+
+	if (flags & BTRFS_BALANCE_ARGS_VRANGE) {
+		CHECK_UPDATE_BP_3("vrange%llu..%llu,",
+				  bargs->vstart, bargs->vend);
+	}
+
+	if (flags & BTRFS_BALANCE_ARGS_LIMIT) {
+		CHECK_UPDATE_BP_2("limit=%llu,", bargs->limit);
+	}
+
+	if (flags & BTRFS_BALANCE_ARGS_LIMIT_RANGE) {
+		CHECK_UPDATE_BP_3("limit=%u..%u,",
+				bargs->limit_min, bargs->limit_max);
+	}
+
+	if (flags & BTRFS_BALANCE_ARGS_STRIPES_RANGE) {
+		CHECK_UPDATE_BP_3("stripes=%u..%u,",
+				  bargs->stripes_min, bargs->stripes_max);
+	}
+
+	if (flags & BTRFS_BALANCE_ARGS_CONVERT) {
+		int index = btrfs_bg_flags_to_raid_index(bargs->target);
+
+		CHECK_UPDATE_BP_2("convert=%s,", get_raid_name(index));
+	}
+
+out:
+#undef CHECK_UPDATE_BP_3
+#undef CHECK_UPDATE_BP_2
+#undef CHECK_UPDATE_BP_1
+
+	if (size_bp < size_buf)
+		buf[size_buf - size_bp - 1] = '\0'; /* remove last , */
+	else
+		buf[0] = '\0';
+}
+
+static void describe_balance_start_or_resume(struct btrfs_fs_info *fs_info)
+{
+	u32 size_buf = 1024;
+	char tmp_buf[192] = {'\0'};
+	char *buf;
+	char *bp;
+	u32 size_bp = size_buf;
+	int ret;
+	struct btrfs_balance_control *bctl = fs_info->balance_ctl;
+
+	buf = kzalloc(size_buf, GFP_KERNEL);
+	if (!buf)
+		return;
+
+	bp = buf;
+#define CHECK_UPDATE_BP_1(a) \
+	do { \
+		ret = snprintf(bp, size_bp, a); \
+		if (ret < 0 || ret >= size_bp) \
+			return; \
+		size_bp -= ret; \
+		bp += ret; \
+	} while (0)
+
+#define CHECK_UPDATE_BP_2(a, v1) \
+	do { \
+		ret = snprintf(bp, size_bp, a, v1); \
+		if (ret < 0 || ret >= size_bp) \
+			return; \
+		size_bp -= ret; \
+		bp += ret; \
+	} while (0)
+
+	if (bctl->flags & BTRFS_BALANCE_FORCE) {
+		CHECK_UPDATE_BP_1("-f ");
+	}
+
+	if (bctl->flags & BTRFS_BALANCE_DATA) {
+		describe_balance_args(&bctl->data, tmp_buf, sizeof(tmp_buf));
+		CHECK_UPDATE_BP_2("-d%s ", tmp_buf);
+	}
+
+	if (bctl->flags & BTRFS_BALANCE_METADATA) {
+		describe_balance_args(&bctl->meta, tmp_buf, sizeof(tmp_buf));
+		CHECK_UPDATE_BP_2("-m%s ", tmp_buf);
+	}
+
+	if (bctl->flags & BTRFS_BALANCE_SYSTEM) {
+		describe_balance_args(&bctl->sys, tmp_buf, sizeof(tmp_buf));
+		CHECK_UPDATE_BP_2("-s%s ", tmp_buf);
+	}
+
+#undef CHECK_UPDATE_BP_2
+#undef CHECK_UPDATE_BP_1
+
+	if (size_bp < size_buf)
+		buf[size_buf - size_bp - 1] = '\0'; /* remove last " "*/
+	btrfs_info(fs_info, "%s %s",
+		   bctl->flags & BTRFS_BALANCE_RESUME ?
+		   "balance: resume":"balance: start", buf);
+
+	kfree(buf);
+}
+
 /*
  * Should be called with balance mutexe held
  */
@@ -3893,6 +4060,7 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
 
 	ASSERT(!test_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags));
 	set_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags);
+	describe_balance_start_or_resume(fs_info);
 	mutex_unlock(&fs_info->balance_mutex);
 
 	ret = __btrfs_balance(fs_info);
@@ -3930,10 +4098,8 @@ static int balance_kthread(void *data)
 	int ret = 0;
 
 	mutex_lock(&fs_info->balance_mutex);
-	if (fs_info->balance_ctl) {
-		btrfs_info(fs_info, "balance: resuming");
+	if (fs_info->balance_ctl)
 		ret = btrfs_balance(fs_info, fs_info->balance_ctl, NULL);
-	}
 	mutex_unlock(&fs_info->balance_mutex);
 
 	return ret;
-- 
1.8.3.1

[PATCH v5 3/3] btrfs: balance: add kernel log for end or paused

[Anand Jain]

Add a kernel log when the balance ends, either for cancel or completed
or if it is paused.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
v4->v5: nothing.
v3->v4: nothing.
v2->v3: nothing.
v1->v2: Moved from 2/3 to 3/3
 fs/btrfs/volumes.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 8397f72bdac7..93f1683f8f5c 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -4066,6 +4066,13 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
 	ret = __btrfs_balance(fs_info);
 
 	mutex_lock(&fs_info->balance_mutex);
+	if (ret == -ECANCELED && atomic_read(&fs_info->balance_pause_req))
+		btrfs_info(fs_info, "balance: paused");
+	else if (ret == -ECANCELED && atomic_read(&fs_info->balance_cancel_req))
+		btrfs_info(fs_info, "balance: canceled");
+	else
+		btrfs_info(fs_info, "balance: ended with status: %d", ret);
+
 	clear_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags);
 
 	if (bargs) {
-- 
1.8.3.1

Re: [PATCH v5 1/3] btrfs: add helper function describe_block_group()

[David Sterba]

On Wed, Nov 14, 2018 at 09:17:10PM +0800, Anand Jain wrote:
> Improve on describe_relocation() add a common helper function to describe
> the block groups.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> Reviewed-by: David Sterba <dsterba@suse.com>
> ---
> v4.1->v5: Initialize buf[128] to null.
> v4->v4.1: Use strcpy(buf, "|NONE"); as in the original
> v3->v4: Just pass full flag name in the define DESCRIBE_FLAG(flag,..),
> 	 so that it can be used at couple of more places.
> 	Rename describe_block_groups() to btrfs_describe_block_groups().
> 	Drop useless return u32.
> v3:	Born.
> 
>  fs/btrfs/relocation.c | 30 +++---------------------------
>  fs/btrfs/volumes.c    | 43 +++++++++++++++++++++++++++++++++++++++++++
>  fs/btrfs/volumes.h    |  1 +
>  3 files changed, 47 insertions(+), 27 deletions(-)
> 
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 924116f654a1..0373b3cc1d36 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -4184,37 +4184,13 @@ static struct reloc_control *alloc_reloc_control(void)
>  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;
> +	char buf[128] = {'\0'};
>  
> -	/* Shouldn't happen */
> -	if (!flags) {
> -		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(bp, buf - bp + sizeof(buf), "|0x%llx", flags);
> -#undef DESCRIBE_FLAG
> -	}
> +	btrfs_describe_block_groups(block_group->flags, buf, sizeof(buf));
>  
>  	btrfs_info(fs_info,
>  		   "relocating block group %llu flags %s",
> -		   block_group->key.objectid, buf + 1);
> +		   block_group->key.objectid, buf);
>  }
>  
>  /*
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index f435d397019e..12b3d0625c6a 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -123,6 +123,49 @@ const char *get_raid_name(enum btrfs_raid_types type)
>  	return btrfs_raid_array[type].raid_name;
>  }
>  
> +void btrfs_describe_block_groups(u64 bg_flags, char *buf, u32 size_buf)
> +{
> +	int i;
> +	int ret;
> +	char *bp = buf;
> +	u64 flags = bg_flags;
> +	u32 size_bp = size_buf;
> +
> +	if (!flags) {
> +		strcpy(bp, "NONE");
> +		return;
> +	}
> +
> +#define DESCRIBE_FLAG(f, d) \

Macro arguments should be in ( ) in the body

> +	do { \
> +		if (flags & f) { \
> +			ret = snprintf(bp, size_bp, "%s|", d); \
> +			if (ret < 0 || ret >= size_bp) \
> +				return; \

Ok, this seems to be safe. snprintf will not write more than size_bp and
this will be caught if it overflows.

The return is obscured by the macro, I'd rather make it more visible
that there is a potential change in the code flow. The proposed change:

				goto out_overflow;
				...

and define out_overflow at the end of function. Same for the other
helper macros.

Also please align the backslashes of the macro a few tabs to the right.

> +			size_bp -= ret; \
> +			bp += ret; \
> +			flags &= ~f; \
> +		} \
> +	} while (0)

Re: [PATCH v5 2/3] btrfs: balance: add args info during start and resume

[David Sterba]

On Wed, Nov 14, 2018 at 09:17:11PM +0800, Anand Jain wrote:
> Balance arg info is an important information to be reviewed for the
> system audit. So this patch adds them to the kernel log.
> 
> Example:
> ->btrfs bal start -f -mprofiles=raid1,convert=single,soft -dlimit=10..20,usage=50 /btrfs
> 
> kernel: BTRFS info (device sdb): balance: start -f -dusage=50,limit=10..20 -msoft,profiles=raid1,convert=single -ssoft,profiles=raid1,convert=single
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
> v4.1->v5: Per David review comment the code..
>            bp += snprintf(bp, buf - bp + size_buf, "soft,");
> 	  is not safe if 'buf - bp + size_buf' becomes accidentally
> 	  negative, so fix this by using following snprintf.
> 
>  #define CHECK_UPDATE_BP_1(a) \
>        do { \
>                ret = snprintf(bp, size_bp, a); \
>                if (ret < 0 || ret >= size_bp) \
>                        goto out; \
>                size_bp -= ret; \
>                bp += ret; \
>        } while (0)
> 
> 	And initialize the tmp_buf to null.
> 
> v4->v4.1: Rename last missed sz to size_buf.
> 
> v3->v4: Change log updated with new example.
> 	Log format is changed to almost match with the cli.
> 	snprintf drop %s for inline string.
> 	Print range as =%u..%u instead of min=%umax=%u.
> 	soft is under the args now.
> 	force is printed as -f.
> 
> v2->v3: Change log updated.
>         Make use of describe_block_group() added in 1/4
>         Drop using strcat instead use snprintf.
>         Logs string format updated as shown in the example above.
> 
> v1->v2: Change log update.
>         Move adding the logs for balance complete and end to a new patch
> 
>  fs/btrfs/volumes.c | 172 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 169 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 12b3d0625c6a..8397f72bdac7 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -3754,6 +3754,173 @@ static inline int validate_convert_profile(struct btrfs_balance_args *bctl_arg,
>  		 (bctl_arg->target & ~allowed)));
>  }
>  
> +static void describe_balance_args(struct btrfs_balance_args *bargs, char *buf,
> +				 u32 size_buf)
> +{
> +	int ret;
> +	u32 size_bp = size_buf;
> +	char *bp = buf;
> +	u64 flags = bargs->flags;
> +	char tmp_buf[128] = {'\0'};
> +
> +	if (!flags)
> +		return;
> +
> +#define CHECK_UPDATE_BP_1(a) \

CHECK_UPDATE_BP_NOARG and the others CHECK_UPDATE_BP_1ARG, though I'm
thinking about picking another name for the macro like CHECK_APPEND_2ARG
etc.

> +	do { \
> +		ret = snprintf(bp, size_bp, a); \
> +		if (ret < 0 || ret >= size_bp) \
> +			goto out; \

out_overflow and align \ to the right, please.

> +		size_bp -= ret; \
> +		bp += ret; \
> +	} while (0)
> +
> +#define CHECK_UPDATE_BP_2(a, v1) \
> +	do { \
> +		ret = snprintf(bp, size_bp, a, v1); \
> +		if (ret < 0 || ret >= size_bp) \
> +			goto out; \
> +		size_bp -= ret; \
> +		bp += ret; \
> +	} while (0)
> +
> +#define CHECK_UPDATE_BP_3(a, v1, v2) \
> +	do { \
> +		ret = snprintf(bp, size_bp, a, v1, v2); \
> +		if (ret < 0 || ret >= size_bp) \
> +			goto out; \
> +		size_bp -= ret; \
> +		bp += ret; \
> +	} while (0)
> +
> +	if (flags & BTRFS_BALANCE_ARGS_SOFT) {
> +		CHECK_UPDATE_BP_1("soft,");
> +	}

no { } around single statement

> +
> +	if (flags & BTRFS_BALANCE_ARGS_PROFILES) {
> +		btrfs_describe_block_groups(bargs->profiles, tmp_buf,
> +					    sizeof(tmp_buf));
> +		CHECK_UPDATE_BP_2("profiles=%s,", tmp_buf);
> +	}
> +
> +	if (flags & BTRFS_BALANCE_ARGS_USAGE) {
> +		CHECK_UPDATE_BP_2("usage=%llu,", bargs->usage);
> +	}
> +
> +	if (flags & BTRFS_BALANCE_ARGS_USAGE_RANGE) {
> +		CHECK_UPDATE_BP_3("usage=%u..%u,",
> +				  bargs->usage_min, bargs->usage_max);
> +	}
> +
> +	if (flags & BTRFS_BALANCE_ARGS_DEVID) {
> +		CHECK_UPDATE_BP_2("devid=%llu,", bargs->devid);
> +	}
> +
> +	if (flags & BTRFS_BALANCE_ARGS_DRANGE) {
> +		CHECK_UPDATE_BP_3("drange=%llu..%llu,",
> +				  bargs->pstart, bargs->pend);
> +	}
> +
> +	if (flags & BTRFS_BALANCE_ARGS_VRANGE) {
> +		CHECK_UPDATE_BP_3("vrange%llu..%llu,",
> +				  bargs->vstart, bargs->vend);
> +	}
> +
> +	if (flags & BTRFS_BALANCE_ARGS_LIMIT) {
> +		CHECK_UPDATE_BP_2("limit=%llu,", bargs->limit);
> +	}
> +
> +	if (flags & BTRFS_BALANCE_ARGS_LIMIT_RANGE) {
> +		CHECK_UPDATE_BP_3("limit=%u..%u,",
> +				bargs->limit_min, bargs->limit_max);
> +	}
> +
> +	if (flags & BTRFS_BALANCE_ARGS_STRIPES_RANGE) {
> +		CHECK_UPDATE_BP_3("stripes=%u..%u,",
> +				  bargs->stripes_min, bargs->stripes_max);
> +	}
> +
> +	if (flags & BTRFS_BALANCE_ARGS_CONVERT) {
> +		int index = btrfs_bg_flags_to_raid_index(bargs->target);
> +
> +		CHECK_UPDATE_BP_2("convert=%s,", get_raid_name(index));
> +	}
> +
> +out:
> +#undef CHECK_UPDATE_BP_3
> +#undef CHECK_UPDATE_BP_2
> +#undef CHECK_UPDATE_BP_1
> +
> +	if (size_bp < size_buf)
> +		buf[size_buf - size_bp - 1] = '\0'; /* remove last , */
> +	else
> +		buf[0] = '\0';
> +}
> +
> +static void describe_balance_start_or_resume(struct btrfs_fs_info *fs_info)
> +{
> +	u32 size_buf = 1024;
> +	char tmp_buf[192] = {'\0'};
> +	char *buf;
> +	char *bp;
> +	u32 size_bp = size_buf;
> +	int ret;
> +	struct btrfs_balance_control *bctl = fs_info->balance_ctl;
> +
> +	buf = kzalloc(size_buf, GFP_KERNEL);
> +	if (!buf)
> +		return;
> +
> +	bp = buf;
> +#define CHECK_UPDATE_BP_1(a) \
> +	do { \
> +		ret = snprintf(bp, size_bp, a); \
> +		if (ret < 0 || ret >= size_bp) \
> +			return; \
> +		size_bp -= ret; \
> +		bp += ret; \
> +	} while (0)
> +
> +#define CHECK_UPDATE_BP_2(a, v1) \
> +	do { \
> +		ret = snprintf(bp, size_bp, a, v1); \
> +		if (ret < 0 || ret >= size_bp) \
> +			return; \
> +		size_bp -= ret; \
> +		bp += ret; \
> +	} while (0)
> +
> +	if (bctl->flags & BTRFS_BALANCE_FORCE) {
> +		CHECK_UPDATE_BP_1("-f ");
> +	}
> +
> +	if (bctl->flags & BTRFS_BALANCE_DATA) {
> +		describe_balance_args(&bctl->data, tmp_buf, sizeof(tmp_buf));
> +		CHECK_UPDATE_BP_2("-d%s ", tmp_buf);
> +	}
> +
> +	if (bctl->flags & BTRFS_BALANCE_METADATA) {
> +		describe_balance_args(&bctl->meta, tmp_buf, sizeof(tmp_buf));
> +		CHECK_UPDATE_BP_2("-m%s ", tmp_buf);
> +	}
> +
> +	if (bctl->flags & BTRFS_BALANCE_SYSTEM) {
> +		describe_balance_args(&bctl->sys, tmp_buf, sizeof(tmp_buf));
> +		CHECK_UPDATE_BP_2("-s%s ", tmp_buf);
> +	}
> +
> +#undef CHECK_UPDATE_BP_2
> +#undef CHECK_UPDATE_BP_1
> +
> +	if (size_bp < size_buf)
> +		buf[size_buf - size_bp - 1] = '\0'; /* remove last " "*/
> +	btrfs_info(fs_info, "%s %s",
> +		   bctl->flags & BTRFS_BALANCE_RESUME ?
> +		   "balance: resume":"balance: start", buf);
> +
> +	kfree(buf);
> +}
> +
>  /*
>   * Should be called with balance mutexe held
>   */
> @@ -3893,6 +4060,7 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
>  
>  	ASSERT(!test_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags));
>  	set_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags);
> +	describe_balance_start_or_resume(fs_info);
>  	mutex_unlock(&fs_info->balance_mutex);
>  
>  	ret = __btrfs_balance(fs_info);
> @@ -3930,10 +4098,8 @@ static int balance_kthread(void *data)
>  	int ret = 0;
>  
>  	mutex_lock(&fs_info->balance_mutex);
> -	if (fs_info->balance_ctl) {
> -		btrfs_info(fs_info, "balance: resuming");
> +	if (fs_info->balance_ctl)
>  		ret = btrfs_balance(fs_info, fs_info->balance_ctl, NULL);
> -	}
>  	mutex_unlock(&fs_info->balance_mutex);
>  
>  	return ret;
> -- 
> 1.8.3.1

Re: [PATCH v5 1/3] btrfs: add helper function describe_block_group()

[Anand Jain]

Thanks for the review.. more below.

On 11/20/2018 01:02 AM, David Sterba wrote:
> On Wed, Nov 14, 2018 at 09:17:10PM +0800, Anand Jain wrote:
>> Improve on describe_relocation() add a common helper function to describe
>> the block groups.
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> Reviewed-by: David Sterba <dsterba@suse.com>
>> ---
>> v4.1->v5: Initialize buf[128] to null.
>> v4->v4.1: Use strcpy(buf, "|NONE"); as in the original
>> v3->v4: Just pass full flag name in the define DESCRIBE_FLAG(flag,..),
>> 	 so that it can be used at couple of more places.
>> 	Rename describe_block_groups() to btrfs_describe_block_groups().
>> 	Drop useless return u32.
>> v3:	Born.
>>
>>   fs/btrfs/relocation.c | 30 +++---------------------------
>>   fs/btrfs/volumes.c    | 43 +++++++++++++++++++++++++++++++++++++++++++
>>   fs/btrfs/volumes.h    |  1 +
>>   3 files changed, 47 insertions(+), 27 deletions(-)
>>
>> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
>> index 924116f654a1..0373b3cc1d36 100644
>> --- a/fs/btrfs/relocation.c
>> +++ b/fs/btrfs/relocation.c
>> @@ -4184,37 +4184,13 @@ static struct reloc_control *alloc_reloc_control(void)
>>   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;
>> +	char buf[128] = {'\0'};
>>   
>> -	/* Shouldn't happen */
>> -	if (!flags) {
>> -		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(bp, buf - bp + sizeof(buf), "|0x%llx", flags);
>> -#undef DESCRIBE_FLAG
>> -	}
>> +	btrfs_describe_block_groups(block_group->flags, buf, sizeof(buf));
>>   
>>   	btrfs_info(fs_info,
>>   		   "relocating block group %llu flags %s",
>> -		   block_group->key.objectid, buf + 1);
>> +		   block_group->key.objectid, buf);
>>   }
>>   
>>   /*
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index f435d397019e..12b3d0625c6a 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -123,6 +123,49 @@ const char *get_raid_name(enum btrfs_raid_types type)
>>   	return btrfs_raid_array[type].raid_name;
>>   }
>>   
>> +void btrfs_describe_block_groups(u64 bg_flags, char *buf, u32 size_buf)
>> +{
>> +	int i;
>> +	int ret;
>> +	char *bp = buf;
>> +	u64 flags = bg_flags;
>> +	u32 size_bp = size_buf;
>> +
>> +	if (!flags) {
>> +		strcpy(bp, "NONE");
>> +		return;
>> +	}
>> +
>> +#define DESCRIBE_FLAG(f, d) \
> 
> Macro arguments should be in ( ) in the body

  Fixed in v6.

>> +	do { \
>> +		if (flags & f) { \
>> +			ret = snprintf(bp, size_bp, "%s|", d); \
>> +			if (ret < 0 || ret >= size_bp) \
>> +				return; \
> 
> Ok, this seems to be safe. snprintf will not write more than size_bp and
> this will be caught if it overflows.
> 
> The return is obscured by the macro, I'd rather make it more visible
> that there is a potential change in the code flow. The proposed change:
> 
> 				goto out_overflow;
> 				...
> 
> and define out_overflow at the end of function. Same for the other
> helper macros.

  makes sense. Thanks for explaining. Fixed in v6.

> Also please align the backslashes of the macro a few tabs to the right.

  Yep done in v6.

Thanks, Anand


>> +			size_bp -= ret; \
>> +			bp += ret; \
>> +			flags &= ~f; \
>> +		} \
>> +	} while (0)

Re: [PATCH v5 2/3] btrfs: balance: add args info during start and resume

[Anand Jain]

On 11/20/2018 01:07 AM, David Sterba wrote:
> On Wed, Nov 14, 2018 at 09:17:11PM +0800, Anand Jain wrote:
>> Balance arg info is an important information to be reviewed for the
>> system audit. So this patch adds them to the kernel log.
>>
>> Example:
>> ->btrfs bal start -f -mprofiles=raid1,convert=single,soft -dlimit=10..20,usage=50 /btrfs
>>
>> kernel: BTRFS info (device sdb): balance: start -f -dusage=50,limit=10..20 -msoft,profiles=raid1,convert=single -ssoft,profiles=raid1,convert=single
>>
>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>> ---
>> v4.1->v5: Per David review comment the code..
>>             bp += snprintf(bp, buf - bp + size_buf, "soft,");
>> 	  is not safe if 'buf - bp + size_buf' becomes accidentally
>> 	  negative, so fix this by using following snprintf.
>>
>>   #define CHECK_UPDATE_BP_1(a) \
>>         do { \
>>                 ret = snprintf(bp, size_bp, a); \
>>                 if (ret < 0 || ret >= size_bp) \
>>                         goto out; \
>>                 size_bp -= ret; \
>>                 bp += ret; \
>>         } while (0)
>>
>> 	And initialize the tmp_buf to null.
>>
>> v4->v4.1: Rename last missed sz to size_buf.
>>
>> v3->v4: Change log updated with new example.
>> 	Log format is changed to almost match with the cli.
>> 	snprintf drop %s for inline string.
>> 	Print range as =%u..%u instead of min=%umax=%u.
>> 	soft is under the args now.
>> 	force is printed as -f.
>>
>> v2->v3: Change log updated.
>>          Make use of describe_block_group() added in 1/4
>>          Drop using strcat instead use snprintf.
>>          Logs string format updated as shown in the example above.
>>
>> v1->v2: Change log update.
>>          Move adding the logs for balance complete and end to a new patch
>>
>>   fs/btrfs/volumes.c | 172 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 169 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index 12b3d0625c6a..8397f72bdac7 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -3754,6 +3754,173 @@ static inline int validate_convert_profile(struct btrfs_balance_args *bctl_arg,
>>   		 (bctl_arg->target & ~allowed)));
>>   }
>>   
>> +static void describe_balance_args(struct btrfs_balance_args *bargs, char *buf,
>> +				 u32 size_buf)
>> +{
>> +	int ret;
>> +	u32 size_bp = size_buf;
>> +	char *bp = buf;
>> +	u64 flags = bargs->flags;
>> +	char tmp_buf[128] = {'\0'};
>> +
>> +	if (!flags)
>> +		return;
>> +
>> +#define CHECK_UPDATE_BP_1(a) \
> 
> CHECK_UPDATE_BP_NOARG and the others CHECK_UPDATE_BP_1ARG, though I'm
> thinking about picking another name for the macro like CHECK_APPEND_2ARG
> etc.

  I liked CHECK_APPEND_XXX I have renamed
   CHECK_UPDATE_BP_1 to CHECK_APPEND_NOARG
   CHECK_UPDATE_BP_2 to CHECK_APPEND_1ARG
   CHECK_UPDATE_BP_3 to CHECK_APPEND_2ARG

Hope its better now.

>> +	do { \
>> +		ret = snprintf(bp, size_bp, a); \
>> +		if (ret < 0 || ret >= size_bp) \
>> +			goto out; \
> 
> out_overflow and align \ to the right, please.

  Yep done.

>> +		size_bp -= ret; \
>> +		bp += ret; \
>> +	} while (0)
>> +
>> +#define CHECK_UPDATE_BP_2(a, v1) \
>> +	do { \
>> +		ret = snprintf(bp, size_bp, a, v1); \
>> +		if (ret < 0 || ret >= size_bp) \
>> +			goto out; \
>> +		size_bp -= ret; \
>> +		bp += ret; \
>> +	} while (0)
>> +
>> +#define CHECK_UPDATE_BP_3(a, v1, v2) \
>> +	do { \
>> +		ret = snprintf(bp, size_bp, a, v1, v2); \
>> +		if (ret < 0 || ret >= size_bp) \
>> +			goto out; \
>> +		size_bp -= ret; \
>> +		bp += ret; \
>> +	} while (0)
>> +
>> +	if (flags & BTRFS_BALANCE_ARGS_SOFT) {
>> +		CHECK_UPDATE_BP_1("soft,");
>> +	}
> 
> no { } around single statement

  fixed.

>> +
>> +	if (flags & BTRFS_BALANCE_ARGS_PROFILES) {
>> +		btrfs_describe_block_groups(bargs->profiles, tmp_buf,
>> +					    sizeof(tmp_buf));
>> +		CHECK_UPDATE_BP_2("profiles=%s,", tmp_buf);
>> +	}
>> +
>> +	if (flags & BTRFS_BALANCE_ARGS_USAGE) {
>> +		CHECK_UPDATE_BP_2("usage=%llu,", bargs->usage);
>> +	}
>> +
>> +	if (flags & BTRFS_BALANCE_ARGS_USAGE_RANGE) {
>> +		CHECK_UPDATE_BP_3("usage=%u..%u,",
>> +				  bargs->usage_min, bargs->usage_max);
>> +	}
>> +
>> +	if (flags & BTRFS_BALANCE_ARGS_DEVID) {
>> +		CHECK_UPDATE_BP_2("devid=%llu,", bargs->devid);
>> +	}
>> +
>> +	if (flags & BTRFS_BALANCE_ARGS_DRANGE) {
>> +		CHECK_UPDATE_BP_3("drange=%llu..%llu,",
>> +				  bargs->pstart, bargs->pend);
>> +	}
>> +
>> +	if (flags & BTRFS_BALANCE_ARGS_VRANGE) {
>> +		CHECK_UPDATE_BP_3("vrange%llu..%llu,",
>> +				  bargs->vstart, bargs->vend);
>> +	}
>> +
>> +	if (flags & BTRFS_BALANCE_ARGS_LIMIT) {
>> +		CHECK_UPDATE_BP_2("limit=%llu,", bargs->limit);
>> +	}
>> +
>> +	if (flags & BTRFS_BALANCE_ARGS_LIMIT_RANGE) {
>> +		CHECK_UPDATE_BP_3("limit=%u..%u,",
>> +				bargs->limit_min, bargs->limit_max);
>> +	}
>> +
>> +	if (flags & BTRFS_BALANCE_ARGS_STRIPES_RANGE) {
>> +		CHECK_UPDATE_BP_3("stripes=%u..%u,",
>> +				  bargs->stripes_min, bargs->stripes_max);
>> +	}
>> +
>> +	if (flags & BTRFS_BALANCE_ARGS_CONVERT) {
>> +		int index = btrfs_bg_flags_to_raid_index(bargs->target);
>> +
>> +		CHECK_UPDATE_BP_2("convert=%s,", get_raid_name(index));
>> +	}
>> +
>> +out:
>> +#undef CHECK_UPDATE_BP_3
>> +#undef CHECK_UPDATE_BP_2
>> +#undef CHECK_UPDATE_BP_1
>> +
>> +	if (size_bp < size_buf)
>> +		buf[size_buf - size_bp - 1] = '\0'; /* remove last , */
>> +	else
>> +		buf[0] = '\0';
>> +}
>> +
>> +static void describe_balance_start_or_resume(struct btrfs_fs_info *fs_info)
>> +{
>> +	u32 size_buf = 1024;
>> +	char tmp_buf[192] = {'\0'};
>> +	char *buf;
>> +	char *bp;
>> +	u32 size_bp = size_buf;
>> +	int ret;
>> +	struct btrfs_balance_control *bctl = fs_info->balance_ctl;
>> +
>> +	buf = kzalloc(size_buf, GFP_KERNEL);
>> +	if (!buf)
>> +		return;
>> +
>> +	bp = buf;
>> +#define CHECK_UPDATE_BP_1(a) \
>> +	do { \
>> +		ret = snprintf(bp, size_bp, a); \
>> +		if (ret < 0 || ret >= size_bp) \
>> +			return; \

  Leaking buf above. By using goto out_overflow also fixes this.

Thanks, Anand


>> +		size_bp -= ret; \
>> +		bp += ret; \
>> +	} while (0)
>> +
>> +#define CHECK_UPDATE_BP_2(a, v1) \
>> +	do { \
>> +		ret = snprintf(bp, size_bp, a, v1); \
>> +		if (ret < 0 || ret >= size_bp) \
>> +			return; \
>> +		size_bp -= ret; \
>> +		bp += ret; \
>> +	} while (0)
>> +
>> +	if (bctl->flags & BTRFS_BALANCE_FORCE) {
>> +		CHECK_UPDATE_BP_1("-f ");
>> +	}
>> +
>> +	if (bctl->flags & BTRFS_BALANCE_DATA) {
>> +		describe_balance_args(&bctl->data, tmp_buf, sizeof(tmp_buf));
>> +		CHECK_UPDATE_BP_2("-d%s ", tmp_buf);
>> +	}
>> +
>> +	if (bctl->flags & BTRFS_BALANCE_METADATA) {
>> +		describe_balance_args(&bctl->meta, tmp_buf, sizeof(tmp_buf));
>> +		CHECK_UPDATE_BP_2("-m%s ", tmp_buf);
>> +	}
>> +
>> +	if (bctl->flags & BTRFS_BALANCE_SYSTEM) {
>> +		describe_balance_args(&bctl->sys, tmp_buf, sizeof(tmp_buf));
>> +		CHECK_UPDATE_BP_2("-s%s ", tmp_buf);
>> +	}
>> +
>> +#undef CHECK_UPDATE_BP_2
>> +#undef CHECK_UPDATE_BP_1
>> +
>> +	if (size_bp < size_buf)
>> +		buf[size_buf - size_bp - 1] = '\0'; /* remove last " "*/
>> +	btrfs_info(fs_info, "%s %s",
>> +		   bctl->flags & BTRFS_BALANCE_RESUME ?
>> +		   "balance: resume":"balance: start", buf);
>> +
>> +	kfree(buf);
>> +}
>> +
>>   /*
>>    * Should be called with balance mutexe held
>>    */
>> @@ -3893,6 +4060,7 @@ int btrfs_balance(struct btrfs_fs_info *fs_info,
>>   
>>   	ASSERT(!test_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags));
>>   	set_bit(BTRFS_FS_BALANCE_RUNNING, &fs_info->flags);
>> +	describe_balance_start_or_resume(fs_info);
>>   	mutex_unlock(&fs_info->balance_mutex);
>>   
>>   	ret = __btrfs_balance(fs_info);
>> @@ -3930,10 +4098,8 @@ static int balance_kthread(void *data)
>>   	int ret = 0;
>>   
>>   	mutex_lock(&fs_info->balance_mutex);
>> -	if (fs_info->balance_ctl) {
>> -		btrfs_info(fs_info, "balance: resuming");
>> +	if (fs_info->balance_ctl)
>>   		ret = btrfs_balance(fs_info, fs_info->balance_ctl, NULL);
>> -	}
>>   	mutex_unlock(&fs_info->balance_mutex);
>>   
>>   	return ret;
>> -- 
>> 1.8.3.1