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

[Chao Yu]

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@xxxxxxxx>
> > > ---
> > >    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@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel