diff mbox series

[3/4] btrfs: make DUMP_BLOCK_RSV() to have better output

Message ID d0096ee10270e00362471c7a842aeefed20806c5.1657867842.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: output more info for -ENOSPC caused transaction abort and other enhancement | expand

Commit Message

Qu Wenruo July 15, 2022, 6:57 a.m. UTC 
This involves the following change:

- Remove "_block_rsv" suffix

- Better alignment for the numbers

- Better naming distinguish for delayed refs and delayed inode

- Skip "size" and "reserved" output
  Just output the numbers directly

Before:

  BTRFS info (device dm-1: state A): global_block_rsv: size 3670016 reserved 3653632
  BTRFS info (device dm-1: state A): trans_block_rsv: size 0 reserved 0
  BTRFS info (device dm-1: state A): chunk_block_rsv: size 0 reserved 0
  BTRFS info (device dm-1: state A): delayed_block_rsv: size 0 reserved 0
  BTRFS info (device dm-1: state A): delayed_refs_rsv: size 524288 reserved 360448

After:

 BTRFS info (device dm-1: state A): global:          (3670016/3670016)
 BTRFS info (device dm-1: state A): trans:           (0/0)
 BTRFS info (device dm-1: state A): chunk:           (0/0)
 BTRFS info (device dm-1: state A): delayed_inode:   (0/0)
 BTRFS info (device dm-1: state A): delayed_refs:    (524288/524288)

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/space-info.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

Comments

Johannes Thumshirn July 15, 2022, 8:14 a.m. UTC | #1 
On 15.07.22 08:58, Qu Wenruo wrote:
> - Skip "size" and "reserved" output
>   Just output the numbers directly
> 
> Before:
> 
>   BTRFS info (device dm-1: state A): global_block_rsv: size 3670016 reserved 3653632
>   BTRFS info (device dm-1: state A): trans_block_rsv: size 0 reserved 0
>   BTRFS info (device dm-1: state A): chunk_block_rsv: size 0 reserved 0
>   BTRFS info (device dm-1: state A): delayed_block_rsv: size 0 reserved 0
>   BTRFS info (device dm-1: state A): delayed_refs_rsv: size 524288 reserved 360448
> 
> After:
> 
>  BTRFS info (device dm-1: state A): global:          (3670016/3670016)
>  BTRFS info (device dm-1: state A): trans:           (0/0)
>  BTRFS info (device dm-1: state A): chunk:           (0/0)
>  BTRFS info (device dm-1: state A): delayed_inode:   (0/0)
>  BTRFS info (device dm-1: state A): delayed_refs:    (524288/524288)

Pure personal preference, but I find it a tad bit easier to have size and 
reserved printed. So you don't have to look up the code when you encounter
the error.

Maybe:
BTRFS info (device dm-1: state A): global:          size:3670016,res:3670016

But in the end of the day it doesn't matter I guess.
Qu Wenruo July 15, 2022, 8:16 a.m. UTC | #2 
On 2022/7/15 16:14, Johannes Thumshirn wrote:
> On 15.07.22 08:58, Qu Wenruo wrote:
>> - Skip "size" and "reserved" output
>>    Just output the numbers directly
>>
>> Before:
>>
>>    BTRFS info (device dm-1: state A): global_block_rsv: size 3670016 reserved 3653632
>>    BTRFS info (device dm-1: state A): trans_block_rsv: size 0 reserved 0
>>    BTRFS info (device dm-1: state A): chunk_block_rsv: size 0 reserved 0
>>    BTRFS info (device dm-1: state A): delayed_block_rsv: size 0 reserved 0
>>    BTRFS info (device dm-1: state A): delayed_refs_rsv: size 524288 reserved 360448
>>
>> After:
>>
>>   BTRFS info (device dm-1: state A): global:          (3670016/3670016)
>>   BTRFS info (device dm-1: state A): trans:           (0/0)
>>   BTRFS info (device dm-1: state A): chunk:           (0/0)
>>   BTRFS info (device dm-1: state A): delayed_inode:   (0/0)
>>   BTRFS info (device dm-1: state A): delayed_refs:    (524288/524288)
>
> Pure personal preference, but I find it a tad bit easier to have size and
> reserved printed. So you don't have to look up the code when you encounter
> the error.
>
> Maybe:
> BTRFS info (device dm-1: state A): global:          size:3670016,res:3670016
>
> But in the end of the day it doesn't matter I guess.

The reason here is I want to avoid size/res affecting the number output.

But your concern is also valid.

What about a new line before everything, showing something like:
Dumping global metadata reservations (reserved/size) :

So that with that line, we know what is reserved and what is size?

Thanks,
Qu
Johannes Thumshirn July 15, 2022, 8:18 a.m. UTC | #3 
On 15.07.22 10:17, Qu Wenruo wrote:
> 
> 
> On 2022/7/15 16:14, Johannes Thumshirn wrote:
>> On 15.07.22 08:58, Qu Wenruo wrote:
>>> - Skip "size" and "reserved" output
>>>    Just output the numbers directly
>>>
>>> Before:
>>>
>>>    BTRFS info (device dm-1: state A): global_block_rsv: size 3670016 reserved 3653632
>>>    BTRFS info (device dm-1: state A): trans_block_rsv: size 0 reserved 0
>>>    BTRFS info (device dm-1: state A): chunk_block_rsv: size 0 reserved 0
>>>    BTRFS info (device dm-1: state A): delayed_block_rsv: size 0 reserved 0
>>>    BTRFS info (device dm-1: state A): delayed_refs_rsv: size 524288 reserved 360448
>>>
>>> After:
>>>
>>>   BTRFS info (device dm-1: state A): global:          (3670016/3670016)
>>>   BTRFS info (device dm-1: state A): trans:           (0/0)
>>>   BTRFS info (device dm-1: state A): chunk:           (0/0)
>>>   BTRFS info (device dm-1: state A): delayed_inode:   (0/0)
>>>   BTRFS info (device dm-1: state A): delayed_refs:    (524288/524288)
>>
>> Pure personal preference, but I find it a tad bit easier to have size and
>> reserved printed. So you don't have to look up the code when you encounter
>> the error.
>>
>> Maybe:
>> BTRFS info (device dm-1: state A): global:          size:3670016,res:3670016
>>
>> But in the end of the day it doesn't matter I guess.
> 
> The reason here is I want to avoid size/res affecting the number output.
> 
> But your concern is also valid.
> 
> What about a new line before everything, showing something like:
> Dumping global metadata reservations (reserved/size) :
> 
> So that with that line, we know what is reserved and what is size?

Yeah that sounds good to me
diff mbox series

Patch

diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index 623fa0488545..6bbf95e8e4f7 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -441,12 +441,12 @@  void btrfs_try_granting_tickets(struct btrfs_fs_info *fs_info,
 	}
 }
 
-#define DUMP_BLOCK_RSV(fs_info, rsv_name)				\
+#define DUMP_BLOCK_RSV(fs_info, member, name)				\
 do {									\
-	struct btrfs_block_rsv *__rsv = &(fs_info)->rsv_name;		\
+	struct btrfs_block_rsv *__rsv = &(fs_info)->member;		\
 	spin_lock(&__rsv->lock);					\
-	btrfs_info(fs_info, #rsv_name ": size %llu reserved %llu",	\
-		   __rsv->size, __rsv->reserved);			\
+	btrfs_info(fs_info, "%-16s (%llu/%llu)",			\
+		   name, __rsv->reserved, __rsv->size);			\
 	spin_unlock(&__rsv->lock);					\
 } while (0)
 
@@ -485,12 +485,11 @@  static void __btrfs_dump_space_info(struct btrfs_fs_info *fs_info,
 		btrfs_info(fs_info,
 			    "  zone_unusable: %llu", info->bytes_zone_unusable);
 
-	DUMP_BLOCK_RSV(fs_info, global_block_rsv);
-	DUMP_BLOCK_RSV(fs_info, trans_block_rsv);
-	DUMP_BLOCK_RSV(fs_info, chunk_block_rsv);
-	DUMP_BLOCK_RSV(fs_info, delayed_block_rsv);
-	DUMP_BLOCK_RSV(fs_info, delayed_refs_rsv);
-
+	DUMP_BLOCK_RSV(fs_info, global_block_rsv, "global:");
+	DUMP_BLOCK_RSV(fs_info, trans_block_rsv, "trans:");
+	DUMP_BLOCK_RSV(fs_info, chunk_block_rsv, "chunk:");
+	DUMP_BLOCK_RSV(fs_info, delayed_block_rsv, "delayed_inode:");
+	DUMP_BLOCK_RSV(fs_info, delayed_refs_rsv, "delayed_refs:");
 }
 
 void btrfs_dump_space_info(struct btrfs_fs_info *fs_info,