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

[Wu Bo <bo.wu@vivo.com>]

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