linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] btrfs: relocation: Output current relocation stage at btrfs_relocate_block_group()
@ 2019-11-28  7:54 Qu Wenruo
  2019-11-28  7:54 ` Qu Wenruo
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Qu Wenruo @ 2019-11-28  7:54 UTC (permalink / raw)
  To: linux-btrfs

There are several reports of hanging relocation, populating the dmesg
with things like:
  BTRFS info (device dm-5): found 1 extents

The investigation is still on going, but will never hurt to output a
little more info.

This patch will also output the current relocation stage, making that
output something like:

  BTRFS info (device dm-5): balance: start -d -m -s
  BTRFS info (device dm-5): relocating block group 30408704 flags metadata|dup
  BTRFS info (device dm-5): found 2 extents at MOVE_DATA_EXTENT stage
  BTRFS info (device dm-5): relocating block group 22020096 flags system|dup
  BTRFS info (device dm-5): found 1 extents at MOVE_DATA_EXTENT stage
  BTRFS info (device dm-5): relocating block group 13631488 flags data
  BTRFS info (device dm-5): found 1 extents at MOVE_DATA_EXTENT stage
  BTRFS info (device dm-5): found 1 extents at UPDATE_DATA_PTRS stage
  BTRFS info (device dm-5): balance: ended with status: 0

The string "MOVE_DATA_EXTENT" and "UPDATE_DATA_PTRS" is mostly from the
macro MOVE_DATA_EXTENTS and UPDATE_DATA_PTRS, but the 'S' from
MOVE_DATA_EXTENTS is removed in the output string to make the alignment
better.

This patch will not increase the number of lines, but with extra info
for us to debug the reported problem.
(Although it's very likely the bug is sticking at UPDATE_DATA_PTRS
stage, even without the patch)

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/relocation.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index d897a8e5e430..88fd9182852d 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4291,6 +4291,15 @@ static void describe_relocation(struct btrfs_fs_info *fs_info,
 		   block_group->start, buf);
 }
 
+static const char *stage_to_string(int stage)
+{
+	if (stage == MOVE_DATA_EXTENTS)
+		return "MOVE_DATA_EXTENT";
+	if (stage == UPDATE_DATA_PTRS)
+		return "UPDATE_DATA_PTRS";
+	return "UNKNOWN";
+}
+
 /*
  * function to relocate all extents in a block group.
  */
@@ -4365,12 +4374,15 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start)
 				 rc->block_group->length);
 
 	while (1) {
+		int finishes_stage;
+
 		mutex_lock(&fs_info->cleaner_mutex);
 		ret = relocate_block_group(rc);
 		mutex_unlock(&fs_info->cleaner_mutex);
 		if (ret < 0)
 			err = ret;
 
+		finishes_stage = rc->stage;
 		/*
 		 * We may have gotten ENOSPC after we already dirtied some
 		 * extents.  If writeout happens while we're relocating a
@@ -4396,8 +4408,8 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start)
 		if (rc->extents_found == 0)
 			break;
 
-		btrfs_info(fs_info, "found %llu extents", rc->extents_found);
-
+		btrfs_info(fs_info, "found %llu extents at %s stage",
+			   rc->extents_found, stage_to_string(finishes_stage));
 	}
 
 	WARN_ON(rc->block_group->pinned > 0);
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] btrfs: relocation: Output current relocation stage at btrfs_relocate_block_group()
  2019-11-28  7:54 [PATCH] btrfs: relocation: Output current relocation stage at btrfs_relocate_block_group() Qu Wenruo
@ 2019-11-28  7:54 ` Qu Wenruo
  2019-11-28 10:50 ` Su Yue
  2019-11-28 15:40 ` David Sterba
  2 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2019-11-28  7:54 UTC (permalink / raw)
  To: linux-btrfs

There are several reports of hanging relocation, populating the dmesg
with things like:
  BTRFS info (device dm-5): found 1 extents

The investigation is still on going, but will never hurt to output a
little more info.

This patch will also output the current relocation stage, making that
output something like:

  BTRFS info (device dm-5): balance: start -d -m -s
  BTRFS info (device dm-5): relocating block group 30408704 flags metadata|dup
  BTRFS info (device dm-5): found 2 extents at MOVE_DATA_EXTENTS stage
  BTRFS info (device dm-5): relocating block group 22020096 flags system|dup
  BTRFS info (device dm-5): found 1 extents at MOVE_DATA_EXTENTS stage
  BTRFS info (device dm-5): relocating block group 13631488 flags data
  BTRFS info (device dm-5): found 1 extents at MOVE_DATA_EXTENTS stage
  BTRFS info (device dm-5): found 1 extents at UPDATE_DATA_PTRS stage
  BTRFS info (device dm-5): balance: ended with status: 0

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/relocation.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index d897a8e5e430..b3bc5b571820 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -4291,6 +4291,15 @@ static void describe_relocation(struct btrfs_fs_info *fs_info,
 		   block_group->start, buf);
 }
 
+static const char *stage_to_string(int stage)
+{
+	if (stage == MOVE_DATA_EXTENTS)
+		return "MOVE_DATA_EXTENTS";
+	if (stage == UPDATE_DATA_PTRS)
+		return "UPDATE_DATA_PTRS";
+	return "UNKNOWN";
+}
+
 /*
  * function to relocate all extents in a block group.
  */
@@ -4365,12 +4374,15 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start)
 				 rc->block_group->length);
 
 	while (1) {
+		int finishes_stage;
+
 		mutex_lock(&fs_info->cleaner_mutex);
 		ret = relocate_block_group(rc);
 		mutex_unlock(&fs_info->cleaner_mutex);
 		if (ret < 0)
 			err = ret;
 
+		finishes_stage = rc->stage;
 		/*
 		 * We may have gotten ENOSPC after we already dirtied some
 		 * extents.  If writeout happens while we're relocating a
@@ -4396,8 +4408,8 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start)
 		if (rc->extents_found == 0)
 			break;
 
-		btrfs_info(fs_info, "found %llu extents", rc->extents_found);
-
+		btrfs_info(fs_info, "found %llu extents at %s stage",
+			   rc->extents_found, stage_to_string(finishes_stage));
 	}
 
 	WARN_ON(rc->block_group->pinned > 0);
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] btrfs: relocation: Output current relocation stage at btrfs_relocate_block_group()
  2019-11-28  7:54 [PATCH] btrfs: relocation: Output current relocation stage at btrfs_relocate_block_group() Qu Wenruo
  2019-11-28  7:54 ` Qu Wenruo
@ 2019-11-28 10:50 ` Su Yue
  2019-11-28 10:58   ` Qu WenRuo
  2019-11-28 15:40 ` David Sterba
  2 siblings, 1 reply; 6+ messages in thread
From: Su Yue @ 2019-11-28 10:50 UTC (permalink / raw)
  To: Qu Wenruo, linux-btrfs



On 2019/11/28 3:54 PM, Qu Wenruo wrote:
> There are several reports of hanging relocation, populating the dmesg
> with things like:
>    BTRFS info (device dm-5): found 1 extents
>
> The investigation is still on going, but will never hurt to output a
> little more info.
>
> This patch will also output the current relocation stage, making that
> output something like:
>
>    BTRFS info (device dm-5): balance: start -d -m -s
>    BTRFS info (device dm-5): relocating block group 30408704 flags metadata|dup
>    BTRFS info (device dm-5): found 2 extents at MOVE_DATA_EXTENT stage
>    BTRFS info (device dm-5): relocating block group 22020096 flags system|dup
>    BTRFS info (device dm-5): found 1 extents at MOVE_DATA_EXTENT stage
>    BTRFS info (device dm-5): relocating block group 13631488 flags data
>    BTRFS info (device dm-5): found 1 extents at MOVE_DATA_EXTENT stage
>    BTRFS info (device dm-5): found 1 extents at UPDATE_DATA_PTRS stage
>    BTRFS info (device dm-5): balance: ended with status: 0
>
> The string "MOVE_DATA_EXTENT" and "UPDATE_DATA_PTRS" is mostly from the
> macro MOVE_DATA_EXTENTS and UPDATE_DATA_PTRS, but the 'S' from
> MOVE_DATA_EXTENTS is removed in the output string to make the alignment
> better.
>
> This patch will not increase the number of lines, but with extra info
> for us to debug the reported problem.
> (Although it's very likely the bug is sticking at UPDATE_DATA_PTRS
> stage, even without the patch)
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>   fs/btrfs/relocation.c | 16 ++++++++++++++--
>   1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index d897a8e5e430..88fd9182852d 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -4291,6 +4291,15 @@ static void describe_relocation(struct btrfs_fs_info *fs_info,
>   		   block_group->start, buf);
>   }
>
> +static const char *stage_to_string(int stage)
> +{
> +	if (stage == MOVE_DATA_EXTENTS)
> +		return "MOVE_DATA_EXTENT";
> +	if (stage == UPDATE_DATA_PTRS)
> +		return "UPDATE_DATA_PTRS";
> +	return "UNKNOWN";
> +}
> +
>   /*
>    * function to relocate all extents in a block group.
>    */
> @@ -4365,12 +4374,15 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start)
>   				 rc->block_group->length);
>
>   	while (1) {
> +		int finishes_stage;
> +

NIT: the rc::stage is an unsigned integer.


>   		mutex_lock(&fs_info->cleaner_mutex);
>   		ret = relocate_block_group(rc);
>   		mutex_unlock(&fs_info->cleaner_mutex);
>   		if (ret < 0)
>   			err = ret;
>
> +		finishes_stage = rc->stage;
>   		/*
>   		 * We may have gotten ENOSPC after we already dirtied some
>   		 * extents.  If writeout happens while we're relocating a
> @@ -4396,8 +4408,8 @@ int btrfs_relocate_block_group(struct btrfs_fs_info *fs_info, u64 group_start)
>   		if (rc->extents_found == 0)
>   			break;
>
> -		btrfs_info(fs_info, "found %llu extents", rc->extents_found);
> -
> +		btrfs_info(fs_info, "found %llu extents at %s stage",
> +			   rc->extents_found, stage_to_string(finishes_stage));
>   	}
>
>   	WARN_ON(rc->block_group->pinned > 0);
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] btrfs: relocation: Output current relocation stage at btrfs_relocate_block_group()
  2019-11-28 10:50 ` Su Yue
@ 2019-11-28 10:58   ` Qu WenRuo
  0 siblings, 0 replies; 6+ messages in thread
From: Qu WenRuo @ 2019-11-28 10:58 UTC (permalink / raw)
  To: Su Yue, linux-btrfs



On 2019/11/28 下午6:50, Su Yue wrote:
> 
> 
> On 2019/11/28 3:54 PM, Qu Wenruo wrote:
>> There are several reports of hanging relocation, populating the dmesg
>> with things like:
>>    BTRFS info (device dm-5): found 1 extents
>>
>> The investigation is still on going, but will never hurt to output a
>> little more info.
>>
>> This patch will also output the current relocation stage, making that
>> output something like:
>>
>>    BTRFS info (device dm-5): balance: start -d -m -s
>>    BTRFS info (device dm-5): relocating block group 30408704 flags
>> metadata|dup
>>    BTRFS info (device dm-5): found 2 extents at MOVE_DATA_EXTENT stage
>>    BTRFS info (device dm-5): relocating block group 22020096 flags
>> system|dup
>>    BTRFS info (device dm-5): found 1 extents at MOVE_DATA_EXTENT stage
>>    BTRFS info (device dm-5): relocating block group 13631488 flags data
>>    BTRFS info (device dm-5): found 1 extents at MOVE_DATA_EXTENT stage
>>    BTRFS info (device dm-5): found 1 extents at UPDATE_DATA_PTRS stage
>>    BTRFS info (device dm-5): balance: ended with status: 0
>>
>> The string "MOVE_DATA_EXTENT" and "UPDATE_DATA_PTRS" is mostly from the
>> macro MOVE_DATA_EXTENTS and UPDATE_DATA_PTRS, but the 'S' from
>> MOVE_DATA_EXTENTS is removed in the output string to make the alignment
>> better.
>>
>> This patch will not increase the number of lines, but with extra info
>> for us to debug the reported problem.
>> (Although it's very likely the bug is sticking at UPDATE_DATA_PTRS
>> stage, even without the patch)
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   fs/btrfs/relocation.c | 16 ++++++++++++++--
>>   1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
>> index d897a8e5e430..88fd9182852d 100644
>> --- a/fs/btrfs/relocation.c
>> +++ b/fs/btrfs/relocation.c
>> @@ -4291,6 +4291,15 @@ static void describe_relocation(struct
>> btrfs_fs_info *fs_info,
>>              block_group->start, buf);
>>   }
>>
>> +static const char *stage_to_string(int stage)
>> +{
>> +    if (stage == MOVE_DATA_EXTENTS)
>> +        return "MOVE_DATA_EXTENT";
>> +    if (stage == UPDATE_DATA_PTRS)
>> +        return "UPDATE_DATA_PTRS";
>> +    return "UNKNOWN";
>> +}
>> +
>>   /*
>>    * function to relocate all extents in a block group.
>>    */
>> @@ -4365,12 +4374,15 @@ int btrfs_relocate_block_group(struct
>> btrfs_fs_info *fs_info, u64 group_start)
>>                    rc->block_group->length);
>>
>>       while (1) {
>> +        int finishes_stage;
>> +
> 
> NIT: the rc::stage is an unsigned integer.

It doesn't matter. rc::stage is only a 8bit value.

Both unsigned int/int can handle it without any problem.
And the strage_to_string() function can handle any value, so no problem
at all.

Thanks,
Qu
> 
> 
>>           mutex_lock(&fs_info->cleaner_mutex);
>>           ret = relocate_block_group(rc);
>>           mutex_unlock(&fs_info->cleaner_mutex);
>>           if (ret < 0)
>>               err = ret;
>>
>> +        finishes_stage = rc->stage;
>>           /*
>>            * We may have gotten ENOSPC after we already dirtied some
>>            * extents.  If writeout happens while we're relocating a
>> @@ -4396,8 +4408,8 @@ int btrfs_relocate_block_group(struct
>> btrfs_fs_info *fs_info, u64 group_start)
>>           if (rc->extents_found == 0)
>>               break;
>>
>> -        btrfs_info(fs_info, "found %llu extents", rc->extents_found);
>> -
>> +        btrfs_info(fs_info, "found %llu extents at %s stage",
>> +               rc->extents_found, stage_to_string(finishes_stage));
>>       }
>>
>>       WARN_ON(rc->block_group->pinned > 0);
>>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] btrfs: relocation: Output current relocation stage at btrfs_relocate_block_group()
  2019-11-28  7:54 [PATCH] btrfs: relocation: Output current relocation stage at btrfs_relocate_block_group() Qu Wenruo
  2019-11-28  7:54 ` Qu Wenruo
  2019-11-28 10:50 ` Su Yue
@ 2019-11-28 15:40 ` David Sterba
  2019-11-29  0:35   ` Qu Wenruo
  2 siblings, 1 reply; 6+ messages in thread
From: David Sterba @ 2019-11-28 15:40 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Thu, Nov 28, 2019 at 03:54:36PM +0800, Qu Wenruo wrote:
> There are several reports of hanging relocation, populating the dmesg
> with things like:
>   BTRFS info (device dm-5): found 1 extents
> 
> The investigation is still on going, but will never hurt to output a
> little more info.
> 
> This patch will also output the current relocation stage, making that
> output something like:
> 
>   BTRFS info (device dm-5): balance: start -d -m -s
>   BTRFS info (device dm-5): relocating block group 30408704 flags metadata|dup
>   BTRFS info (device dm-5): found 2 extents at MOVE_DATA_EXTENT stage
>   BTRFS info (device dm-5): relocating block group 22020096 flags system|dup
>   BTRFS info (device dm-5): found 1 extents at MOVE_DATA_EXTENT stage
>   BTRFS info (device dm-5): relocating block group 13631488 flags data
>   BTRFS info (device dm-5): found 1 extents at MOVE_DATA_EXTENT stage
>   BTRFS info (device dm-5): found 1 extents at UPDATE_DATA_PTRS stage
>   BTRFS info (device dm-5): balance: ended with status: 0
> 
> The string "MOVE_DATA_EXTENT" and "UPDATE_DATA_PTRS" is mostly from the
> macro MOVE_DATA_EXTENTS and UPDATE_DATA_PTRS, but the 'S' from
> MOVE_DATA_EXTENTS is removed in the output string to make the alignment
> better.
> 
> This patch will not increase the number of lines, but with extra info
> for us to debug the reported problem.

Nice. I'd suggest to make it more user friendly

	relocation: found 111 extents, stage: move data blocks
	relocation: found 111 extents, stage: update data pointers

The identifier can be understood what it means but it's IMHO not
important to copy it to the message verbatim.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] btrfs: relocation: Output current relocation stage at btrfs_relocate_block_group()
  2019-11-28 15:40 ` David Sterba
@ 2019-11-29  0:35   ` Qu Wenruo
  0 siblings, 0 replies; 6+ messages in thread
From: Qu Wenruo @ 2019-11-29  0:35 UTC (permalink / raw)
  To: dsterba, Qu Wenruo, linux-btrfs


[-- Attachment #1.1: Type: text/plain, Size: 1913 bytes --]



On 2019/11/28 下午11:40, David Sterba wrote:
> On Thu, Nov 28, 2019 at 03:54:36PM +0800, Qu Wenruo wrote:
>> There are several reports of hanging relocation, populating the dmesg
>> with things like:
>>   BTRFS info (device dm-5): found 1 extents
>>
>> The investigation is still on going, but will never hurt to output a
>> little more info.
>>
>> This patch will also output the current relocation stage, making that
>> output something like:
>>
>>   BTRFS info (device dm-5): balance: start -d -m -s
>>   BTRFS info (device dm-5): relocating block group 30408704 flags metadata|dup
>>   BTRFS info (device dm-5): found 2 extents at MOVE_DATA_EXTENT stage
>>   BTRFS info (device dm-5): relocating block group 22020096 flags system|dup
>>   BTRFS info (device dm-5): found 1 extents at MOVE_DATA_EXTENT stage
>>   BTRFS info (device dm-5): relocating block group 13631488 flags data
>>   BTRFS info (device dm-5): found 1 extents at MOVE_DATA_EXTENT stage
>>   BTRFS info (device dm-5): found 1 extents at UPDATE_DATA_PTRS stage
>>   BTRFS info (device dm-5): balance: ended with status: 0
>>
>> The string "MOVE_DATA_EXTENT" and "UPDATE_DATA_PTRS" is mostly from the
>> macro MOVE_DATA_EXTENTS and UPDATE_DATA_PTRS, but the 'S' from
>> MOVE_DATA_EXTENTS is removed in the output string to make the alignment
>> better.
>>
>> This patch will not increase the number of lines, but with extra info
>> for us to debug the reported problem.
> 
> Nice. I'd suggest to make it more user friendly
> 
> 	relocation: found 111 extents, stage: move data blocks
> 	relocation: found 111 extents, stage: update data pointers

This is much better, keeps the indent while provide better readability.

I'll go this way in the next version.

Thanks,
Qu
> 
> The identifier can be understood what it means but it's IMHO not
> important to copy it to the message verbatim.
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-11-29  0:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-28  7:54 [PATCH] btrfs: relocation: Output current relocation stage at btrfs_relocate_block_group() Qu Wenruo
2019-11-28  7:54 ` Qu Wenruo
2019-11-28 10:50 ` Su Yue
2019-11-28 10:58   ` Qu WenRuo
2019-11-28 15:40 ` David Sterba
2019-11-29  0:35   ` Qu Wenruo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).