diff mbox series

[f2fs-dev] dump.f2fs: add checkpoint version to dump_nat

Message ID 20240724103543.2666271-1-bo.wu@vivo.com (mailing list archive)
State Superseded
Headers show
Series [f2fs-dev] dump.f2fs: add checkpoint version to dump_nat | expand

Commit Message

Wu Bo July 24, 2024, 10:35 a.m. UTC
The cp_ver of node footer is useful when analyzing data corruption
issues.

Signed-off-by: Wu Bo <bo.wu@vivo.com>
---
 fsck/dump.c | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

Comments

Chao Yu July 25, 2024, 2:33 a.m. UTC | #1
On 2024/7/24 18:35, Wu Bo wrote:
> The cp_ver of node footer is useful when analyzing data corruption
> issues.
> 
> Signed-off-by: Wu Bo <bo.wu@vivo.com>
> ---
>   fsck/dump.c | 33 ++++++++++++++++++---------------
>   1 file changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/fsck/dump.c b/fsck/dump.c
> index 8d5613e..ca38101 100644
> --- a/fsck/dump.c
> +++ b/fsck/dump.c
> @@ -21,7 +21,7 @@
>   #endif
>   #include <locale.h>
>   
> -#define BUF_SZ	80
> +#define BUF_SZ	256

128 is fine?

>   
>   /* current extent info */
>   struct extent_info dump_extent;
> @@ -38,6 +38,7 @@ void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
>   {
>   	struct f2fs_nat_block *nat_block;
>   	struct f2fs_node *node_block;
> +	struct node_footer *footer;
>   	nid_t nid;
>   	pgoff_t block_addr;
>   	char buf[BUF_SZ];
> @@ -47,6 +48,7 @@ void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
>   	ASSERT(nat_block);
>   	node_block = (struct f2fs_node *)calloc(F2FS_BLKSIZE, 1);
>   	ASSERT(node_block);
> +	footer = F2FS_NODE_FOOTER(node_block);
>   
>   	fd = open("dump_nat", O_CREAT|O_WRONLY|O_TRUNC, 0666);
>   	ASSERT(fd >= 0);
> @@ -54,6 +56,7 @@ void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
>   	for (nid = start_nat; nid < end_nat; nid++) {
>   		struct f2fs_nat_entry raw_nat;
>   		struct node_info ni;
> +		int len;
>   		if(nid == 0 || nid == F2FS_NODE_INO(sbi) ||
>   					nid == F2FS_META_INO(sbi))
>   			continue;
> @@ -66,15 +69,15 @@ void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
>   			ret = dev_read_block(node_block, ni.blk_addr);
>   			ASSERT(ret >= 0);
>   			if (ni.blk_addr != 0x0) {
> -				memset(buf, 0, BUF_SZ);
> -				snprintf(buf, BUF_SZ,
> +				len = snprintf(buf, BUF_SZ,
>   					"nid:%5u\tino:%5u\toffset:%5u"
> -					"\tblkaddr:%10u\tpack:%d\n",
> +					"\tblkaddr:%10u\tpack:%d"
> +					"\tcp_ver:0x%08x\n",
>   					ni.nid, ni.ino,
> -					le32_to_cpu(F2FS_NODE_FOOTER(node_block)->flag) >>
> -						OFFSET_BIT_SHIFT,
> -					ni.blk_addr, pack);
> -				ret = write(fd, buf, strlen(buf));
> +					le32_to_cpu(footer->flag) >> OFFSET_BIT_SHIFT,
> +					ni.blk_addr, pack,
> +					(uint32_t)le64_to_cpu(footer->cp_ver));

(uint64_t)le64_to_cpu(footer->cp_ver) ?

> +				ret = write(fd, buf, len);
>   				ASSERT(ret >= 0);
>   			}
>   		} else {
> @@ -87,15 +90,15 @@ void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
>   
>   			ret = dev_read_block(node_block, ni.blk_addr);
>   			ASSERT(ret >= 0);
> -			memset(buf, 0, BUF_SZ);
> -			snprintf(buf, BUF_SZ,
> +			len = snprintf(buf, BUF_SZ,
>   				"nid:%5u\tino:%5u\toffset:%5u"
> -				"\tblkaddr:%10u\tpack:%d\n",
> +				"\tblkaddr:%10u\tpack:%d"
> +				"\tcp_ver:0x%08x\n",
>   				ni.nid, ni.ino,
> -				le32_to_cpu(F2FS_NODE_FOOTER(node_block)->flag) >>
> -					OFFSET_BIT_SHIFT,
> -				ni.blk_addr, pack);
> -			ret = write(fd, buf, strlen(buf));
> +				le32_to_cpu(footer->flag) >> OFFSET_BIT_SHIFT,
> +				ni.blk_addr, pack,
> +				(uint32_t)le64_to_cpu(footer->cp_ver));

Ditto,

Thanks,

> +			ret = write(fd, buf, len);
>   			ASSERT(ret >= 0);
>   		}
>   	}
Wu Bo July 25, 2024, 3:51 a.m. UTC | #2
On Thu, Jul 25, 2024 at 10:33:33AM +0800, Chao Yu wrote:
> On 2024/7/24 18:35, Wu Bo wrote:
> > The cp_ver of node footer is useful when analyzing data corruption
> > issues.
> > 
> > Signed-off-by: Wu Bo <bo.wu@vivo.com>
> > ---
> >   fsck/dump.c | 33 ++++++++++++++++++---------------
> >   1 file changed, 18 insertions(+), 15 deletions(-)
> > 
> > diff --git a/fsck/dump.c b/fsck/dump.c
> > index 8d5613e..ca38101 100644
> > --- a/fsck/dump.c
> > +++ b/fsck/dump.c
> > @@ -21,7 +21,7 @@
> >   #endif
> >   #include <locale.h>
> > -#define BUF_SZ	80
> > +#define BUF_SZ	256
> 
> 128 is fine?

This buffer is located in the stack. Making it a little bigger shouldn't cause a
performance drop, right?
128 seems prone to overflow if additional information is added later.

> 
> >   /* current extent info */
> >   struct extent_info dump_extent;
> > @@ -38,6 +38,7 @@ void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
> >   {
> >   	struct f2fs_nat_block *nat_block;
> >   	struct f2fs_node *node_block;
> > +	struct node_footer *footer;
> >   	nid_t nid;
> >   	pgoff_t block_addr;
> >   	char buf[BUF_SZ];
> > @@ -47,6 +48,7 @@ void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
> >   	ASSERT(nat_block);
> >   	node_block = (struct f2fs_node *)calloc(F2FS_BLKSIZE, 1);
> >   	ASSERT(node_block);
> > +	footer = F2FS_NODE_FOOTER(node_block);
> >   	fd = open("dump_nat", O_CREAT|O_WRONLY|O_TRUNC, 0666);
> >   	ASSERT(fd >= 0);
> > @@ -54,6 +56,7 @@ void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
> >   	for (nid = start_nat; nid < end_nat; nid++) {
> >   		struct f2fs_nat_entry raw_nat;
> >   		struct node_info ni;
> > +		int len;
> >   		if(nid == 0 || nid == F2FS_NODE_INO(sbi) ||
> >   					nid == F2FS_META_INO(sbi))
> >   			continue;
> > @@ -66,15 +69,15 @@ void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
> >   			ret = dev_read_block(node_block, ni.blk_addr);
> >   			ASSERT(ret >= 0);
> >   			if (ni.blk_addr != 0x0) {
> > -				memset(buf, 0, BUF_SZ);
> > -				snprintf(buf, BUF_SZ,
> > +				len = snprintf(buf, BUF_SZ,
> >   					"nid:%5u\tino:%5u\toffset:%5u"
> > -					"\tblkaddr:%10u\tpack:%d\n",
> > +					"\tblkaddr:%10u\tpack:%d"
> > +					"\tcp_ver:0x%08x\n",
> >   					ni.nid, ni.ino,
> > -					le32_to_cpu(F2FS_NODE_FOOTER(node_block)->flag) >>
> > -						OFFSET_BIT_SHIFT,
> > -					ni.blk_addr, pack);
> > -				ret = write(fd, buf, strlen(buf));
> > +					le32_to_cpu(footer->flag) >> OFFSET_BIT_SHIFT,
> > +					ni.blk_addr, pack,
> > +					(uint32_t)le64_to_cpu(footer->cp_ver));
> 
> (uint64_t)le64_to_cpu(footer->cp_ver) ?

Is the upper 32 bits used for CRC?
I've noticed that the checkpoint version dumped is always 32 bits long.
To better compare with the current checkpoint, I only print the lower 32 bits here.

> 
> > +				ret = write(fd, buf, len);
> >   				ASSERT(ret >= 0);
> >   			}
> >   		} else {
> > @@ -87,15 +90,15 @@ void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
> >   			ret = dev_read_block(node_block, ni.blk_addr);
> >   			ASSERT(ret >= 0);
> > -			memset(buf, 0, BUF_SZ);
> > -			snprintf(buf, BUF_SZ,
> > +			len = snprintf(buf, BUF_SZ,
> >   				"nid:%5u\tino:%5u\toffset:%5u"
> > -				"\tblkaddr:%10u\tpack:%d\n",
> > +				"\tblkaddr:%10u\tpack:%d"
> > +				"\tcp_ver:0x%08x\n",
> >   				ni.nid, ni.ino,
> > -				le32_to_cpu(F2FS_NODE_FOOTER(node_block)->flag) >>
> > -					OFFSET_BIT_SHIFT,
> > -				ni.blk_addr, pack);
> > -			ret = write(fd, buf, strlen(buf));
> > +				le32_to_cpu(footer->flag) >> OFFSET_BIT_SHIFT,
> > +				ni.blk_addr, pack,
> > +				(uint32_t)le64_to_cpu(footer->cp_ver));
> 
> Ditto,
> 
> Thanks,
> 
> > +			ret = write(fd, buf, len);
> >   			ASSERT(ret >= 0);
> >   		}
> >   	}
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Chao Yu July 25, 2024, 7:13 a.m. UTC | #3
On 2024/7/25 11:51, Wu Bo wrote:
> On Thu, Jul 25, 2024 at 10:33:33AM +0800, Chao Yu wrote:
>> On 2024/7/24 18:35, Wu Bo wrote:
>>> The cp_ver of node footer is useful when analyzing data corruption
>>> issues.
>>>
>>> Signed-off-by: Wu Bo <bo.wu@vivo.com>
>>> ---
>>>    fsck/dump.c | 33 ++++++++++++++++++---------------
>>>    1 file changed, 18 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/fsck/dump.c b/fsck/dump.c
>>> index 8d5613e..ca38101 100644
>>> --- a/fsck/dump.c
>>> +++ b/fsck/dump.c
>>> @@ -21,7 +21,7 @@
>>>    #endif
>>>    #include <locale.h>
>>> -#define BUF_SZ	80
>>> +#define BUF_SZ	256
>>
>> 128 is fine?
> 
> This buffer is located in the stack. Making it a little bigger shouldn't cause a
> performance drop, right?

Yup,

> 128 seems prone to overflow if additional information is added later.

The message will be truncated rather than it will causing overflow and
overwriting random stack size, so, it's safe now?

How about expanding it once it's not enough?

> 
>>
>>>    /* current extent info */
>>>    struct extent_info dump_extent;
>>> @@ -38,6 +38,7 @@ void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
>>>    {
>>>    	struct f2fs_nat_block *nat_block;
>>>    	struct f2fs_node *node_block;
>>> +	struct node_footer *footer;
>>>    	nid_t nid;
>>>    	pgoff_t block_addr;
>>>    	char buf[BUF_SZ];
>>> @@ -47,6 +48,7 @@ void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
>>>    	ASSERT(nat_block);
>>>    	node_block = (struct f2fs_node *)calloc(F2FS_BLKSIZE, 1);
>>>    	ASSERT(node_block);
>>> +	footer = F2FS_NODE_FOOTER(node_block);
>>>    	fd = open("dump_nat", O_CREAT|O_WRONLY|O_TRUNC, 0666);
>>>    	ASSERT(fd >= 0);
>>> @@ -54,6 +56,7 @@ void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
>>>    	for (nid = start_nat; nid < end_nat; nid++) {
>>>    		struct f2fs_nat_entry raw_nat;
>>>    		struct node_info ni;
>>> +		int len;
>>>    		if(nid == 0 || nid == F2FS_NODE_INO(sbi) ||
>>>    					nid == F2FS_META_INO(sbi))
>>>    			continue;
>>> @@ -66,15 +69,15 @@ void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
>>>    			ret = dev_read_block(node_block, ni.blk_addr);
>>>    			ASSERT(ret >= 0);
>>>    			if (ni.blk_addr != 0x0) {
>>> -				memset(buf, 0, BUF_SZ);
>>> -				snprintf(buf, BUF_SZ,
>>> +				len = snprintf(buf, BUF_SZ,
>>>    					"nid:%5u\tino:%5u\toffset:%5u"
>>> -					"\tblkaddr:%10u\tpack:%d\n",
>>> +					"\tblkaddr:%10u\tpack:%d"
>>> +					"\tcp_ver:0x%08x\n",
>>>    					ni.nid, ni.ino,
>>> -					le32_to_cpu(F2FS_NODE_FOOTER(node_block)->flag) >>
>>> -						OFFSET_BIT_SHIFT,
>>> -					ni.blk_addr, pack);
>>> -				ret = write(fd, buf, strlen(buf));
>>> +					le32_to_cpu(footer->flag) >> OFFSET_BIT_SHIFT,
>>> +					ni.blk_addr, pack,
>>> +					(uint32_t)le64_to_cpu(footer->cp_ver));
>>
>> (uint64_t)le64_to_cpu(footer->cp_ver) ?
> 
> Is the upper 32 bits used for CRC?
> I've noticed that the checkpoint version dumped is always 32 bits long.
> To better compare with the current checkpoint, I only print the lower 32 bits here.

Do you want to compare high 32-bits crc value in cp_ver w/ crc value
in CP? maybe you can dump them to two hexadecimal numbers?

Thanks,

> 
>>
>>> +				ret = write(fd, buf, len);
>>>    				ASSERT(ret >= 0);
>>>    			}
>>>    		} else {
>>> @@ -87,15 +90,15 @@ void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
>>>    			ret = dev_read_block(node_block, ni.blk_addr);
>>>    			ASSERT(ret >= 0);
>>> -			memset(buf, 0, BUF_SZ);
>>> -			snprintf(buf, BUF_SZ,
>>> +			len = snprintf(buf, BUF_SZ,
>>>    				"nid:%5u\tino:%5u\toffset:%5u"
>>> -				"\tblkaddr:%10u\tpack:%d\n",
>>> +				"\tblkaddr:%10u\tpack:%d"
>>> +				"\tcp_ver:0x%08x\n",
>>>    				ni.nid, ni.ino,
>>> -				le32_to_cpu(F2FS_NODE_FOOTER(node_block)->flag) >>
>>> -					OFFSET_BIT_SHIFT,
>>> -				ni.blk_addr, pack);
>>> -			ret = write(fd, buf, strlen(buf));
>>> +				le32_to_cpu(footer->flag) >> OFFSET_BIT_SHIFT,
>>> +				ni.blk_addr, pack,
>>> +				(uint32_t)le64_to_cpu(footer->cp_ver));
>>
>> Ditto,
>>
>> Thanks,
>>
>>> +			ret = write(fd, buf, len);
>>>    			ASSERT(ret >= 0);
>>>    		}
>>>    	}
>>
>>
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
diff mbox series

Patch

diff --git a/fsck/dump.c b/fsck/dump.c
index 8d5613e..ca38101 100644
--- a/fsck/dump.c
+++ b/fsck/dump.c
@@ -21,7 +21,7 @@ 
 #endif
 #include <locale.h>
 
-#define BUF_SZ	80
+#define BUF_SZ	256
 
 /* current extent info */
 struct extent_info dump_extent;
@@ -38,6 +38,7 @@  void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
 {
 	struct f2fs_nat_block *nat_block;
 	struct f2fs_node *node_block;
+	struct node_footer *footer;
 	nid_t nid;
 	pgoff_t block_addr;
 	char buf[BUF_SZ];
@@ -47,6 +48,7 @@  void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
 	ASSERT(nat_block);
 	node_block = (struct f2fs_node *)calloc(F2FS_BLKSIZE, 1);
 	ASSERT(node_block);
+	footer = F2FS_NODE_FOOTER(node_block);
 
 	fd = open("dump_nat", O_CREAT|O_WRONLY|O_TRUNC, 0666);
 	ASSERT(fd >= 0);
@@ -54,6 +56,7 @@  void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
 	for (nid = start_nat; nid < end_nat; nid++) {
 		struct f2fs_nat_entry raw_nat;
 		struct node_info ni;
+		int len;
 		if(nid == 0 || nid == F2FS_NODE_INO(sbi) ||
 					nid == F2FS_META_INO(sbi))
 			continue;
@@ -66,15 +69,15 @@  void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
 			ret = dev_read_block(node_block, ni.blk_addr);
 			ASSERT(ret >= 0);
 			if (ni.blk_addr != 0x0) {
-				memset(buf, 0, BUF_SZ);
-				snprintf(buf, BUF_SZ,
+				len = snprintf(buf, BUF_SZ,
 					"nid:%5u\tino:%5u\toffset:%5u"
-					"\tblkaddr:%10u\tpack:%d\n",
+					"\tblkaddr:%10u\tpack:%d"
+					"\tcp_ver:0x%08x\n",
 					ni.nid, ni.ino,
-					le32_to_cpu(F2FS_NODE_FOOTER(node_block)->flag) >>
-						OFFSET_BIT_SHIFT,
-					ni.blk_addr, pack);
-				ret = write(fd, buf, strlen(buf));
+					le32_to_cpu(footer->flag) >> OFFSET_BIT_SHIFT,
+					ni.blk_addr, pack,
+					(uint32_t)le64_to_cpu(footer->cp_ver));
+				ret = write(fd, buf, len);
 				ASSERT(ret >= 0);
 			}
 		} else {
@@ -87,15 +90,15 @@  void nat_dump(struct f2fs_sb_info *sbi, nid_t start_nat, nid_t end_nat)
 
 			ret = dev_read_block(node_block, ni.blk_addr);
 			ASSERT(ret >= 0);
-			memset(buf, 0, BUF_SZ);
-			snprintf(buf, BUF_SZ,
+			len = snprintf(buf, BUF_SZ,
 				"nid:%5u\tino:%5u\toffset:%5u"
-				"\tblkaddr:%10u\tpack:%d\n",
+				"\tblkaddr:%10u\tpack:%d"
+				"\tcp_ver:0x%08x\n",
 				ni.nid, ni.ino,
-				le32_to_cpu(F2FS_NODE_FOOTER(node_block)->flag) >>
-					OFFSET_BIT_SHIFT,
-				ni.blk_addr, pack);
-			ret = write(fd, buf, strlen(buf));
+				le32_to_cpu(footer->flag) >> OFFSET_BIT_SHIFT,
+				ni.blk_addr, pack,
+				(uint32_t)le64_to_cpu(footer->cp_ver));
+			ret = write(fd, buf, len);
 			ASSERT(ret >= 0);
 		}
 	}