linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
* [f2fs-dev] [PATCH 1/4] fsck.f2fs: fix to recover out-of-border inline size
@ 2019-08-12 11:45 Chao Yu
  2019-08-12 11:45 ` [f2fs-dev] [PATCH 2/4] fsck.f2fs: fix to check c.fix_on before repair Chao Yu
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Chao Yu @ 2019-08-12 11:45 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel

It tries to let fsck be noticed wrong inline size, and do the fix.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fsck/fsck.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index d53317c..7eb599d 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -771,6 +771,8 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
 	ofs = get_extra_isize(node_blk);
 
 	if ((node_blk->i.i_inline & F2FS_INLINE_DATA)) {
+		unsigned int inline_size = MAX_INLINE_DATA(node_blk);
+
 		if (le32_to_cpu(node_blk->i.i_addr[ofs]) != 0) {
 			/* should fix this bug all the time */
 			FIX_MSG("inline_data has wrong 0'th block = %x",
@@ -779,6 +781,12 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
 			node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
 			need_fix = 1;
 		}
+		if (!i_size || i_size > inline_size) {
+			node_blk->i.i_size = cpu_to_le64(inline_size);
+			FIX_MSG("inline_data has wrong i_size %lu",
+						(unsigned long)i_size);
+			need_fix = 1;
+		}
 		if (!(node_blk->i.i_inline & F2FS_DATA_EXIST)) {
 			char buf[MAX_INLINE_DATA(node_blk)];
 			memset(buf, 0, MAX_INLINE_DATA(node_blk));
-- 
2.18.0.rc1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* [f2fs-dev] [PATCH 2/4] fsck.f2fs: fix to check c.fix_on before repair
  2019-08-12 11:45 [f2fs-dev] [PATCH 1/4] fsck.f2fs: fix to recover out-of-border inline size Chao Yu
@ 2019-08-12 11:45 ` Chao Yu
  2019-08-16  1:06   ` Jaegeuk Kim
  2019-08-12 11:45 ` [f2fs-dev] [PATCH 3/4] fsck.f2fs: fix to show removed x_nid correctly Chao Yu
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Chao Yu @ 2019-08-12 11:45 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel

We should always set c.bug_on whenever found a bug, then fix them
if c.fix_on is on, otherwise, some bugs won't be shown unless we
enable debug log.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fsck/fsck.c | 137 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 83 insertions(+), 54 deletions(-)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index 7eb599d..91ddd49 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -712,12 +712,13 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
 	fsck_reada_node_block(sbi, le32_to_cpu(node_blk->i.i_xattr_nid));
 
 	if (fsck_chk_xattr_blk(sbi, nid,
-			le32_to_cpu(node_blk->i.i_xattr_nid), blk_cnt) &&
-			c.fix_on) {
-		node_blk->i.i_xattr_nid = 0;
-		need_fix = 1;
-		FIX_MSG("Remove xattr block: 0x%x, x_nid = 0x%x",
-				nid, le32_to_cpu(node_blk->i.i_xattr_nid));
+			le32_to_cpu(node_blk->i.i_xattr_nid), blk_cnt)) {
+		if (c.fix_on) {
+			node_blk->i.i_xattr_nid = 0;
+			need_fix = 1;
+			FIX_MSG("Remove xattr block: 0x%x, x_nid = 0x%x",
+					nid, le32_to_cpu(node_blk->i.i_xattr_nid));
+		}
 	}
 
 	if (ftype == F2FS_FT_CHRDEV || ftype == F2FS_FT_BLKDEV ||
@@ -730,24 +731,32 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
 
 	if (f2fs_has_extra_isize(&node_blk->i)) {
 		if (c.feature & cpu_to_le32(F2FS_FEATURE_EXTRA_ATTR)) {
-			if (node_blk->i.i_extra_isize >
-				cpu_to_le16(F2FS_TOTAL_EXTRA_ATTR_SIZE)) {
-				FIX_MSG("ino[0x%x] recover i_extra_isize "
-					"from %u to %u",
-					nid,
-					le16_to_cpu(node_blk->i.i_extra_isize),
-					calc_extra_isize());
-				node_blk->i.i_extra_isize =
-					cpu_to_le16(calc_extra_isize());
-				need_fix = 1;
+			unsigned int isize =
+				le16_to_cpu(node_blk->i.i_extra_isize);
+
+			if (isize > F2FS_TOTAL_EXTRA_ATTR_SIZE) {
+				ASSERT_MSG("[0x%x] wrong i_extra_isize=0x%x",
+						nid, isize);
+				if (c.fix_on) {
+					FIX_MSG("ino[0x%x] recover i_extra_isize "
+						"from %u to %u",
+						nid, isize,
+						calc_extra_isize());
+					node_blk->i.i_extra_isize =
+						cpu_to_le16(calc_extra_isize());
+					need_fix = 1;
+				}
 			}
 		} else {
-			FIX_MSG("ino[0x%x] remove F2FS_EXTRA_ATTR "
-				"flag in i_inline:%u",
-				nid, node_blk->i.i_inline);
-			/* we don't support tuning F2FS_FEATURE_EXTRA_ATTR now */
-			node_blk->i.i_inline &= ~F2FS_EXTRA_ATTR;
-			need_fix = 1;
+			ASSERT_MSG("[0x%x] wrong extra_attr flag", nid);
+			if (c.fix_on) {
+				FIX_MSG("ino[0x%x] remove F2FS_EXTRA_ATTR "
+					"flag in i_inline:%u",
+					nid, node_blk->i.i_inline);
+				/* we don't support tuning F2FS_FEATURE_EXTRA_ATTR now */
+				node_blk->i.i_inline &= ~F2FS_EXTRA_ATTR;
+				need_fix = 1;
+			}
 		}
 
 		if ((c.feature &
@@ -758,13 +767,17 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
 
 			if (!inline_size ||
 					inline_size > MAX_INLINE_XATTR_SIZE) {
-				FIX_MSG("ino[0x%x] recover inline xattr size "
-					"from %u to %u",
-					nid, inline_size,
-					DEFAULT_INLINE_XATTR_ADDRS);
-				node_blk->i.i_inline_xattr_size =
-					cpu_to_le16(DEFAULT_INLINE_XATTR_ADDRS);
-				need_fix = 1;
+				ASSERT_MSG("[0x%x] wrong inline_xattr_size:%u",
+						nid, inline_size);
+				if (c.fix_on) {
+					FIX_MSG("ino[0x%x] recover inline xattr size "
+						"from %u to %u",
+						nid, inline_size,
+						DEFAULT_INLINE_XATTR_ADDRS);
+					node_blk->i.i_inline_xattr_size =
+						cpu_to_le16(DEFAULT_INLINE_XATTR_ADDRS);
+					need_fix = 1;
+				}
 			}
 		}
 	}
@@ -772,20 +785,28 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
 
 	if ((node_blk->i.i_inline & F2FS_INLINE_DATA)) {
 		unsigned int inline_size = MAX_INLINE_DATA(node_blk);
+		block_t blkaddr = le32_to_cpu(node_blk->i.i_addr[ofs]);
 
-		if (le32_to_cpu(node_blk->i.i_addr[ofs]) != 0) {
-			/* should fix this bug all the time */
-			FIX_MSG("inline_data has wrong 0'th block = %x",
-					le32_to_cpu(node_blk->i.i_addr[ofs]));
-			node_blk->i.i_addr[ofs] = 0;
-			node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
-			need_fix = 1;
+		if (blkaddr != 0) {
+			ASSERT_MSG("[0x%x] wrong inline reserve blkaddr:%u",
+					nid, blkaddr);
+			if (c.fix_on) {
+				FIX_MSG("inline_data has wrong 0'th block = %x",
+								blkaddr);
+				node_blk->i.i_addr[ofs] = 0;
+				node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
+				need_fix = 1;
+			}
 		}
 		if (!i_size || i_size > inline_size) {
-			node_blk->i.i_size = cpu_to_le64(inline_size);
-			FIX_MSG("inline_data has wrong i_size %lu",
-						(unsigned long)i_size);
-			need_fix = 1;
+			ASSERT_MSG("[0x%x] wrong inline size:%lu",
+					nid, (unsigned long)i_size);
+			if (c.fix_on) {
+				node_blk->i.i_size = cpu_to_le64(inline_size);
+				FIX_MSG("inline_data has wrong i_size %lu",
+							(unsigned long)i_size);
+				need_fix = 1;
+			}
 		}
 		if (!(node_blk->i.i_inline & F2FS_DATA_EXIST)) {
 			char buf[MAX_INLINE_DATA(node_blk)];
@@ -793,9 +814,12 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
 
 			if (memcmp(buf, inline_data_addr(node_blk),
 						MAX_INLINE_DATA(node_blk))) {
-				FIX_MSG("inline_data has DATA_EXIST");
-				node_blk->i.i_inline |= F2FS_DATA_EXIST;
-				need_fix = 1;
+				ASSERT_MSG("[0x%x] junk inline data", nid);
+				if (c.fix_on) {
+					FIX_MSG("inline_data has DATA_EXIST");
+					node_blk->i.i_inline |= F2FS_DATA_EXIST;
+					need_fix = 1;
+				}
 			}
 		}
 		DBG(3, "ino[0x%x] has inline data!\n", nid);
@@ -804,20 +828,25 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
 	}
 
 	if ((node_blk->i.i_inline & F2FS_INLINE_DENTRY)) {
+		block_t blkaddr = le32_to_cpu(node_blk->i.i_addr[ofs]);
+
 		DBG(3, "ino[0x%x] has inline dentry!\n", nid);
-		if (le32_to_cpu(node_blk->i.i_addr[ofs]) != 0) {
-			/* should fix this bug all the time */
-			FIX_MSG("inline_dentry has wrong 0'th block = %x",
-					le32_to_cpu(node_blk->i.i_addr[ofs]));
-			node_blk->i.i_addr[ofs] = 0;
-			node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
-			need_fix = 1;
+		if (blkaddr != 0) {
+			ASSERT_MSG("[0x%x] wrong inline reserve blkaddr:%u",
+								nid, blkaddr);
+			if (c.fix_on) {
+				FIX_MSG("inline_dentry has wrong 0'th block = %x",
+								blkaddr);
+				node_blk->i.i_addr[ofs] = 0;
+				node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
+				need_fix = 1;
+			}
 		}
 
 		ret = fsck_chk_inline_dentries(sbi, node_blk, &child);
 		if (ret < 0) {
-			/* should fix this bug all the time */
-			need_fix = 1;
+			if (c.fix_on)
+				need_fix = 1;
 		}
 		child.state |= FSCK_INLINE_INODE;
 		goto check;
@@ -999,7 +1028,7 @@ skip_blkcnt_fix:
 	free(en);
 
 	if (ftype == F2FS_FT_SYMLINK && i_blocks && i_size == 0) {
-		DBG(1, "ino: 0x%x i_blocks: %lu with zero i_size",
+		ASSERT_MSG("ino: 0x%x i_blocks: %lu with zero i_size\n",
 						nid, (unsigned long)i_blocks);
 		if (c.fix_on) {
 			u64 i_size = i_blocks * F2FS_BLKSIZE;
@@ -1012,7 +1041,7 @@ skip_blkcnt_fix:
 	}
 
 	if (ftype == F2FS_FT_ORPHAN && i_links) {
-		MSG(0, "ino: 0x%x is orphan inode, but has i_links: %u",
+		ASSERT_MSG("ino: 0x%x is orphan inode, but has i_links: %u",
 				nid, i_links);
 		if (c.fix_on) {
 			node_blk->i.i_links = 0;
-- 
2.18.0.rc1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* [f2fs-dev] [PATCH 3/4] fsck.f2fs: fix to show removed x_nid correctly
  2019-08-12 11:45 [f2fs-dev] [PATCH 1/4] fsck.f2fs: fix to recover out-of-border inline size Chao Yu
  2019-08-12 11:45 ` [f2fs-dev] [PATCH 2/4] fsck.f2fs: fix to check c.fix_on before repair Chao Yu
@ 2019-08-12 11:45 ` Chao Yu
  2019-08-12 11:45 ` [f2fs-dev] [PATCH 4/4] fsck.f2fs: fix symlink correctly Chao Yu
  2019-08-15 22:39 ` [f2fs-dev] [PATCH 1/4] fsck.f2fs: fix to recover out-of-border inline size Jaegeuk Kim
  3 siblings, 0 replies; 9+ messages in thread
From: Chao Yu @ 2019-08-12 11:45 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel

Otherwise, we just show fixed zero x_nid value.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fsck/fsck.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index 91ddd49..1ea8590 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -659,6 +659,7 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
 	u32 i_links = le32_to_cpu(node_blk->i.i_links);
 	u64 i_size = le64_to_cpu(node_blk->i.i_size);
 	u64 i_blocks = le64_to_cpu(node_blk->i.i_blocks);
+	nid_t i_xattr_nid = le32_to_cpu(node_blk->i.i_xattr_nid);
 	int ofs;
 	char *en;
 	u32 namelen;
@@ -709,15 +710,14 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
 	}
 
 	/* readahead xattr node block */
-	fsck_reada_node_block(sbi, le32_to_cpu(node_blk->i.i_xattr_nid));
+	fsck_reada_node_block(sbi, i_xattr_nid);
 
-	if (fsck_chk_xattr_blk(sbi, nid,
-			le32_to_cpu(node_blk->i.i_xattr_nid), blk_cnt)) {
+	if (fsck_chk_xattr_blk(sbi, nid, i_xattr_nid, blk_cnt)) {
 		if (c.fix_on) {
 			node_blk->i.i_xattr_nid = 0;
 			need_fix = 1;
 			FIX_MSG("Remove xattr block: 0x%x, x_nid = 0x%x",
-					nid, le32_to_cpu(node_blk->i.i_xattr_nid));
+							nid, i_xattr_nid);
 		}
 	}
 
-- 
2.18.0.rc1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* [f2fs-dev] [PATCH 4/4] fsck.f2fs: fix symlink correctly
  2019-08-12 11:45 [f2fs-dev] [PATCH 1/4] fsck.f2fs: fix to recover out-of-border inline size Chao Yu
  2019-08-12 11:45 ` [f2fs-dev] [PATCH 2/4] fsck.f2fs: fix to check c.fix_on before repair Chao Yu
  2019-08-12 11:45 ` [f2fs-dev] [PATCH 3/4] fsck.f2fs: fix to show removed x_nid correctly Chao Yu
@ 2019-08-12 11:45 ` Chao Yu
  2019-08-15 22:39 ` [f2fs-dev] [PATCH 1/4] fsck.f2fs: fix to recover out-of-border inline size Jaegeuk Kim
  3 siblings, 0 replies; 9+ messages in thread
From: Chao Yu @ 2019-08-12 11:45 UTC (permalink / raw)
  To: jaegeuk; +Cc: linux-f2fs-devel

inode.i_blocks includes inode, xnode and data block count, so, only
fix in below condition:
- i_blocks := 3 (inode + xnode + data_block)
- i_blocks := 2 (inode + data_block)

In addition, it recovers symlink's i_size to 4k rather than i_blocks *
4k.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
---
 fsck/fsck.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index 1ea8590..61d1a8d 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -1027,16 +1027,15 @@ skip_blkcnt_fix:
 
 	free(en);
 
-	if (ftype == F2FS_FT_SYMLINK && i_blocks && i_size == 0) {
+	if (ftype == F2FS_FT_SYMLINK && i_size == 0 &&
+			i_blocks == (i_xattr_nid ? 3 : 2)) {
 		ASSERT_MSG("ino: 0x%x i_blocks: %lu with zero i_size\n",
 						nid, (unsigned long)i_blocks);
 		if (c.fix_on) {
-			u64 i_size = i_blocks * F2FS_BLKSIZE;
-
-			node_blk->i.i_size = cpu_to_le64(i_size);
+			node_blk->i.i_size = cpu_to_le64(F2FS_BLKSIZE);
 			need_fix = 1;
 			FIX_MSG("Symlink: recover 0x%x with i_size=%lu",
-						nid, (unsigned long)i_size);
+					nid, (unsigned long)F2FS_BLKSIZE);
 		}
 	}
 
-- 
2.18.0.rc1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [f2fs-dev] [PATCH 1/4] fsck.f2fs: fix to recover out-of-border inline size
  2019-08-12 11:45 [f2fs-dev] [PATCH 1/4] fsck.f2fs: fix to recover out-of-border inline size Chao Yu
                   ` (2 preceding siblings ...)
  2019-08-12 11:45 ` [f2fs-dev] [PATCH 4/4] fsck.f2fs: fix symlink correctly Chao Yu
@ 2019-08-15 22:39 ` Jaegeuk Kim
  2019-08-16  1:03   ` Jaegeuk Kim
  3 siblings, 1 reply; 9+ messages in thread
From: Jaegeuk Kim @ 2019-08-15 22:39 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel

On 08/12, Chao Yu wrote:
> It tries to let fsck be noticed wrong inline size, and do the fix.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fsck/fsck.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/fsck/fsck.c b/fsck/fsck.c
> index d53317c..7eb599d 100644
> --- a/fsck/fsck.c
> +++ b/fsck/fsck.c
> @@ -771,6 +771,8 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>  	ofs = get_extra_isize(node_blk);
>  
>  	if ((node_blk->i.i_inline & F2FS_INLINE_DATA)) {
> +		unsigned int inline_size = MAX_INLINE_DATA(node_blk);
> +
>  		if (le32_to_cpu(node_blk->i.i_addr[ofs]) != 0) {
>  			/* should fix this bug all the time */
>  			FIX_MSG("inline_data has wrong 0'th block = %x",
> @@ -779,6 +781,12 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>  			node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
>  			need_fix = 1;
>  		}
> +		if (!i_size || i_size > inline_size) {

i_size=0 should be fine?

> +			node_blk->i.i_size = cpu_to_le64(inline_size);
> +			FIX_MSG("inline_data has wrong i_size %lu",
> +						(unsigned long)i_size);
> +			need_fix = 1;
> +		}
>  		if (!(node_blk->i.i_inline & F2FS_DATA_EXIST)) {
>  			char buf[MAX_INLINE_DATA(node_blk)];
>  			memset(buf, 0, MAX_INLINE_DATA(node_blk));
> -- 
> 2.18.0.rc1


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [f2fs-dev] [PATCH 1/4] fsck.f2fs: fix to recover out-of-border inline size
  2019-08-15 22:39 ` [f2fs-dev] [PATCH 1/4] fsck.f2fs: fix to recover out-of-border inline size Jaegeuk Kim
@ 2019-08-16  1:03   ` Jaegeuk Kim
  0 siblings, 0 replies; 9+ messages in thread
From: Jaegeuk Kim @ 2019-08-16  1:03 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel

On 08/15, Jaegeuk Kim wrote:
> On 08/12, Chao Yu wrote:
> > It tries to let fsck be noticed wrong inline size, and do the fix.
> > 
> > Signed-off-by: Chao Yu <yuchao0@huawei.com>
> > ---
> >  fsck/fsck.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/fsck/fsck.c b/fsck/fsck.c
> > index d53317c..7eb599d 100644
> > --- a/fsck/fsck.c
> > +++ b/fsck/fsck.c
> > @@ -771,6 +771,8 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
> >  	ofs = get_extra_isize(node_blk);
> >  
> >  	if ((node_blk->i.i_inline & F2FS_INLINE_DATA)) {
> > +		unsigned int inline_size = MAX_INLINE_DATA(node_blk);
> > +
> >  		if (le32_to_cpu(node_blk->i.i_addr[ofs]) != 0) {
> >  			/* should fix this bug all the time */
> >  			FIX_MSG("inline_data has wrong 0'th block = %x",
> > @@ -779,6 +781,12 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
> >  			node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
> >  			need_fix = 1;
> >  		}
> > +		if (!i_size || i_size > inline_size) {
> 
> i_size=0 should be fine?

Missed v2.

> 
> > +			node_blk->i.i_size = cpu_to_le64(inline_size);
> > +			FIX_MSG("inline_data has wrong i_size %lu",
> > +						(unsigned long)i_size);
> > +			need_fix = 1;
> > +		}
> >  		if (!(node_blk->i.i_inline & F2FS_DATA_EXIST)) {
> >  			char buf[MAX_INLINE_DATA(node_blk)];
> >  			memset(buf, 0, MAX_INLINE_DATA(node_blk));
> > -- 
> > 2.18.0.rc1
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [f2fs-dev] [PATCH 2/4] fsck.f2fs: fix to check c.fix_on before repair
  2019-08-12 11:45 ` [f2fs-dev] [PATCH 2/4] fsck.f2fs: fix to check c.fix_on before repair Chao Yu
@ 2019-08-16  1:06   ` Jaegeuk Kim
  2019-08-16  1:20     ` Chao Yu
  0 siblings, 1 reply; 9+ messages in thread
From: Jaegeuk Kim @ 2019-08-16  1:06 UTC (permalink / raw)
  To: Chao Yu; +Cc: linux-f2fs-devel

On 08/12, Chao Yu wrote:
> We should always set c.bug_on whenever found a bug, then fix them
> if c.fix_on is on, otherwise, some bugs won't be shown unless we
> enable debug log.
> 
> Signed-off-by: Chao Yu <yuchao0@huawei.com>
> ---
>  fsck/fsck.c | 137 +++++++++++++++++++++++++++++++---------------------
>  1 file changed, 83 insertions(+), 54 deletions(-)
> 
> diff --git a/fsck/fsck.c b/fsck/fsck.c
> index 7eb599d..91ddd49 100644
> --- a/fsck/fsck.c
> +++ b/fsck/fsck.c
> @@ -712,12 +712,13 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>  	fsck_reada_node_block(sbi, le32_to_cpu(node_blk->i.i_xattr_nid));
>  
>  	if (fsck_chk_xattr_blk(sbi, nid,
> -			le32_to_cpu(node_blk->i.i_xattr_nid), blk_cnt) &&
> -			c.fix_on) {
> -		node_blk->i.i_xattr_nid = 0;
> -		need_fix = 1;
> -		FIX_MSG("Remove xattr block: 0x%x, x_nid = 0x%x",
> -				nid, le32_to_cpu(node_blk->i.i_xattr_nid));
> +			le32_to_cpu(node_blk->i.i_xattr_nid), blk_cnt)) {
> +		if (c.fix_on) {
> +			node_blk->i.i_xattr_nid = 0;
> +			need_fix = 1;
> +			FIX_MSG("Remove xattr block: 0x%x, x_nid = 0x%x",
> +					nid, le32_to_cpu(node_blk->i.i_xattr_nid));
> +		}

Why do we need this change?

>  	}
>  
>  	if (ftype == F2FS_FT_CHRDEV || ftype == F2FS_FT_BLKDEV ||
> @@ -730,24 +731,32 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>  
>  	if (f2fs_has_extra_isize(&node_blk->i)) {
>  		if (c.feature & cpu_to_le32(F2FS_FEATURE_EXTRA_ATTR)) {
> -			if (node_blk->i.i_extra_isize >
> -				cpu_to_le16(F2FS_TOTAL_EXTRA_ATTR_SIZE)) {
> -				FIX_MSG("ino[0x%x] recover i_extra_isize "
> -					"from %u to %u",
> -					nid,
> -					le16_to_cpu(node_blk->i.i_extra_isize),
> -					calc_extra_isize());
> -				node_blk->i.i_extra_isize =
> -					cpu_to_le16(calc_extra_isize());
> -				need_fix = 1;
> +			unsigned int isize =
> +				le16_to_cpu(node_blk->i.i_extra_isize);
> +
> +			if (isize > F2FS_TOTAL_EXTRA_ATTR_SIZE) {
> +				ASSERT_MSG("[0x%x] wrong i_extra_isize=0x%x",
> +						nid, isize);
> +				if (c.fix_on) {
> +					FIX_MSG("ino[0x%x] recover i_extra_isize "
> +						"from %u to %u",
> +						nid, isize,
> +						calc_extra_isize());
> +					node_blk->i.i_extra_isize =
> +						cpu_to_le16(calc_extra_isize());
> +					need_fix = 1;
> +				}
>  			}
>  		} else {
> -			FIX_MSG("ino[0x%x] remove F2FS_EXTRA_ATTR "
> -				"flag in i_inline:%u",
> -				nid, node_blk->i.i_inline);
> -			/* we don't support tuning F2FS_FEATURE_EXTRA_ATTR now */
> -			node_blk->i.i_inline &= ~F2FS_EXTRA_ATTR;
> -			need_fix = 1;
> +			ASSERT_MSG("[0x%x] wrong extra_attr flag", nid);
> +			if (c.fix_on) {
> +				FIX_MSG("ino[0x%x] remove F2FS_EXTRA_ATTR "
> +					"flag in i_inline:%u",
> +					nid, node_blk->i.i_inline);
> +				/* we don't support tuning F2FS_FEATURE_EXTRA_ATTR now */
> +				node_blk->i.i_inline &= ~F2FS_EXTRA_ATTR;
> +				need_fix = 1;
> +			}
>  		}
>  
>  		if ((c.feature &
> @@ -758,13 +767,17 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>  
>  			if (!inline_size ||
>  					inline_size > MAX_INLINE_XATTR_SIZE) {
> -				FIX_MSG("ino[0x%x] recover inline xattr size "
> -					"from %u to %u",
> -					nid, inline_size,
> -					DEFAULT_INLINE_XATTR_ADDRS);
> -				node_blk->i.i_inline_xattr_size =
> -					cpu_to_le16(DEFAULT_INLINE_XATTR_ADDRS);
> -				need_fix = 1;
> +				ASSERT_MSG("[0x%x] wrong inline_xattr_size:%u",
> +						nid, inline_size);
> +				if (c.fix_on) {
> +					FIX_MSG("ino[0x%x] recover inline xattr size "
> +						"from %u to %u",
> +						nid, inline_size,
> +						DEFAULT_INLINE_XATTR_ADDRS);
> +					node_blk->i.i_inline_xattr_size =
> +						cpu_to_le16(DEFAULT_INLINE_XATTR_ADDRS);
> +					need_fix = 1;
> +				}
>  			}
>  		}
>  	}
> @@ -772,20 +785,28 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>  
>  	if ((node_blk->i.i_inline & F2FS_INLINE_DATA)) {
>  		unsigned int inline_size = MAX_INLINE_DATA(node_blk);
> +		block_t blkaddr = le32_to_cpu(node_blk->i.i_addr[ofs]);
>  
> -		if (le32_to_cpu(node_blk->i.i_addr[ofs]) != 0) {
> -			/* should fix this bug all the time */
> -			FIX_MSG("inline_data has wrong 0'th block = %x",
> -					le32_to_cpu(node_blk->i.i_addr[ofs]));
> -			node_blk->i.i_addr[ofs] = 0;
> -			node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
> -			need_fix = 1;
> +		if (blkaddr != 0) {
> +			ASSERT_MSG("[0x%x] wrong inline reserve blkaddr:%u",
> +					nid, blkaddr);
> +			if (c.fix_on) {
> +				FIX_MSG("inline_data has wrong 0'th block = %x",
> +								blkaddr);
> +				node_blk->i.i_addr[ofs] = 0;
> +				node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
> +				need_fix = 1;
> +			}
>  		}
>  		if (!i_size || i_size > inline_size) {
> -			node_blk->i.i_size = cpu_to_le64(inline_size);
> -			FIX_MSG("inline_data has wrong i_size %lu",
> -						(unsigned long)i_size);
> -			need_fix = 1;
> +			ASSERT_MSG("[0x%x] wrong inline size:%lu",
> +					nid, (unsigned long)i_size);
> +			if (c.fix_on) {
> +				node_blk->i.i_size = cpu_to_le64(inline_size);
> +				FIX_MSG("inline_data has wrong i_size %lu",
> +							(unsigned long)i_size);
> +				need_fix = 1;
> +			}
>  		}
>  		if (!(node_blk->i.i_inline & F2FS_DATA_EXIST)) {
>  			char buf[MAX_INLINE_DATA(node_blk)];
> @@ -793,9 +814,12 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>  
>  			if (memcmp(buf, inline_data_addr(node_blk),
>  						MAX_INLINE_DATA(node_blk))) {
> -				FIX_MSG("inline_data has DATA_EXIST");
> -				node_blk->i.i_inline |= F2FS_DATA_EXIST;
> -				need_fix = 1;
> +				ASSERT_MSG("[0x%x] junk inline data", nid);
> +				if (c.fix_on) {
> +					FIX_MSG("inline_data has DATA_EXIST");
> +					node_blk->i.i_inline |= F2FS_DATA_EXIST;
> +					need_fix = 1;
> +				}
>  			}
>  		}
>  		DBG(3, "ino[0x%x] has inline data!\n", nid);
> @@ -804,20 +828,25 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>  	}
>  
>  	if ((node_blk->i.i_inline & F2FS_INLINE_DENTRY)) {
> +		block_t blkaddr = le32_to_cpu(node_blk->i.i_addr[ofs]);
> +
>  		DBG(3, "ino[0x%x] has inline dentry!\n", nid);
> -		if (le32_to_cpu(node_blk->i.i_addr[ofs]) != 0) {
> -			/* should fix this bug all the time */
> -			FIX_MSG("inline_dentry has wrong 0'th block = %x",
> -					le32_to_cpu(node_blk->i.i_addr[ofs]));
> -			node_blk->i.i_addr[ofs] = 0;
> -			node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
> -			need_fix = 1;
> +		if (blkaddr != 0) {
> +			ASSERT_MSG("[0x%x] wrong inline reserve blkaddr:%u",
> +								nid, blkaddr);
> +			if (c.fix_on) {
> +				FIX_MSG("inline_dentry has wrong 0'th block = %x",
> +								blkaddr);
> +				node_blk->i.i_addr[ofs] = 0;
> +				node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
> +				need_fix = 1;
> +			}
>  		}
>  
>  		ret = fsck_chk_inline_dentries(sbi, node_blk, &child);
>  		if (ret < 0) {
> -			/* should fix this bug all the time */
> -			need_fix = 1;
> +			if (c.fix_on)
> +				need_fix = 1;
>  		}
>  		child.state |= FSCK_INLINE_INODE;
>  		goto check;
> @@ -999,7 +1028,7 @@ skip_blkcnt_fix:
>  	free(en);
>  
>  	if (ftype == F2FS_FT_SYMLINK && i_blocks && i_size == 0) {
> -		DBG(1, "ino: 0x%x i_blocks: %lu with zero i_size",
> +		ASSERT_MSG("ino: 0x%x i_blocks: %lu with zero i_size\n",
>  						nid, (unsigned long)i_blocks);
>  		if (c.fix_on) {
>  			u64 i_size = i_blocks * F2FS_BLKSIZE;
> @@ -1012,7 +1041,7 @@ skip_blkcnt_fix:
>  	}
>  
>  	if (ftype == F2FS_FT_ORPHAN && i_links) {
> -		MSG(0, "ino: 0x%x is orphan inode, but has i_links: %u",
> +		ASSERT_MSG("ino: 0x%x is orphan inode, but has i_links: %u",
>  				nid, i_links);
>  		if (c.fix_on) {
>  			node_blk->i.i_links = 0;
> -- 
> 2.18.0.rc1


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [f2fs-dev] [PATCH 2/4] fsck.f2fs: fix to check c.fix_on before repair
  2019-08-16  1:06   ` Jaegeuk Kim
@ 2019-08-16  1:20     ` Chao Yu
  2019-08-20 11:34       ` Chao Yu
  0 siblings, 1 reply; 9+ messages in thread
From: Chao Yu @ 2019-08-16  1:20 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel

On 2019/8/16 9:06, Jaegeuk Kim wrote:
> On 08/12, Chao Yu wrote:
>> We should always set c.bug_on whenever found a bug, then fix them
>> if c.fix_on is on, otherwise, some bugs won't be shown unless we
>> enable debug log.
>>
>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>> ---
>>  fsck/fsck.c | 137 +++++++++++++++++++++++++++++++---------------------
>>  1 file changed, 83 insertions(+), 54 deletions(-)
>>
>> diff --git a/fsck/fsck.c b/fsck/fsck.c
>> index 7eb599d..91ddd49 100644
>> --- a/fsck/fsck.c
>> +++ b/fsck/fsck.c
>> @@ -712,12 +712,13 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>>  	fsck_reada_node_block(sbi, le32_to_cpu(node_blk->i.i_xattr_nid));
>>  
>>  	if (fsck_chk_xattr_blk(sbi, nid,
>> -			le32_to_cpu(node_blk->i.i_xattr_nid), blk_cnt) &&
>> -			c.fix_on) {
>> -		node_blk->i.i_xattr_nid = 0;
>> -		need_fix = 1;
>> -		FIX_MSG("Remove xattr block: 0x%x, x_nid = 0x%x",
>> -				nid, le32_to_cpu(node_blk->i.i_xattr_nid));
>> +			le32_to_cpu(node_blk->i.i_xattr_nid), blk_cnt)) {
>> +		if (c.fix_on) {
>> +			node_blk->i.i_xattr_nid = 0;
>> +			need_fix = 1;
>> +			FIX_MSG("Remove xattr block: 0x%x, x_nid = 0x%x",
>> +					nid, le32_to_cpu(node_blk->i.i_xattr_nid));
>> +		}
> 
> Why do we need this change?

Actually, it's not necessary, but just cleanup to keep the same style. :P

Thanks,

> 
>>  	}
>>  
>>  	if (ftype == F2FS_FT_CHRDEV || ftype == F2FS_FT_BLKDEV ||
>> @@ -730,24 +731,32 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>>  
>>  	if (f2fs_has_extra_isize(&node_blk->i)) {
>>  		if (c.feature & cpu_to_le32(F2FS_FEATURE_EXTRA_ATTR)) {
>> -			if (node_blk->i.i_extra_isize >
>> -				cpu_to_le16(F2FS_TOTAL_EXTRA_ATTR_SIZE)) {
>> -				FIX_MSG("ino[0x%x] recover i_extra_isize "
>> -					"from %u to %u",
>> -					nid,
>> -					le16_to_cpu(node_blk->i.i_extra_isize),
>> -					calc_extra_isize());
>> -				node_blk->i.i_extra_isize =
>> -					cpu_to_le16(calc_extra_isize());
>> -				need_fix = 1;
>> +			unsigned int isize =
>> +				le16_to_cpu(node_blk->i.i_extra_isize);
>> +
>> +			if (isize > F2FS_TOTAL_EXTRA_ATTR_SIZE) {
>> +				ASSERT_MSG("[0x%x] wrong i_extra_isize=0x%x",
>> +						nid, isize);
>> +				if (c.fix_on) {
>> +					FIX_MSG("ino[0x%x] recover i_extra_isize "
>> +						"from %u to %u",
>> +						nid, isize,
>> +						calc_extra_isize());
>> +					node_blk->i.i_extra_isize =
>> +						cpu_to_le16(calc_extra_isize());
>> +					need_fix = 1;
>> +				}
>>  			}
>>  		} else {
>> -			FIX_MSG("ino[0x%x] remove F2FS_EXTRA_ATTR "
>> -				"flag in i_inline:%u",
>> -				nid, node_blk->i.i_inline);
>> -			/* we don't support tuning F2FS_FEATURE_EXTRA_ATTR now */
>> -			node_blk->i.i_inline &= ~F2FS_EXTRA_ATTR;
>> -			need_fix = 1;
>> +			ASSERT_MSG("[0x%x] wrong extra_attr flag", nid);
>> +			if (c.fix_on) {
>> +				FIX_MSG("ino[0x%x] remove F2FS_EXTRA_ATTR "
>> +					"flag in i_inline:%u",
>> +					nid, node_blk->i.i_inline);
>> +				/* we don't support tuning F2FS_FEATURE_EXTRA_ATTR now */
>> +				node_blk->i.i_inline &= ~F2FS_EXTRA_ATTR;
>> +				need_fix = 1;
>> +			}
>>  		}
>>  
>>  		if ((c.feature &
>> @@ -758,13 +767,17 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>>  
>>  			if (!inline_size ||
>>  					inline_size > MAX_INLINE_XATTR_SIZE) {
>> -				FIX_MSG("ino[0x%x] recover inline xattr size "
>> -					"from %u to %u",
>> -					nid, inline_size,
>> -					DEFAULT_INLINE_XATTR_ADDRS);
>> -				node_blk->i.i_inline_xattr_size =
>> -					cpu_to_le16(DEFAULT_INLINE_XATTR_ADDRS);
>> -				need_fix = 1;
>> +				ASSERT_MSG("[0x%x] wrong inline_xattr_size:%u",
>> +						nid, inline_size);
>> +				if (c.fix_on) {
>> +					FIX_MSG("ino[0x%x] recover inline xattr size "
>> +						"from %u to %u",
>> +						nid, inline_size,
>> +						DEFAULT_INLINE_XATTR_ADDRS);
>> +					node_blk->i.i_inline_xattr_size =
>> +						cpu_to_le16(DEFAULT_INLINE_XATTR_ADDRS);
>> +					need_fix = 1;
>> +				}
>>  			}
>>  		}
>>  	}
>> @@ -772,20 +785,28 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>>  
>>  	if ((node_blk->i.i_inline & F2FS_INLINE_DATA)) {
>>  		unsigned int inline_size = MAX_INLINE_DATA(node_blk);
>> +		block_t blkaddr = le32_to_cpu(node_blk->i.i_addr[ofs]);
>>  
>> -		if (le32_to_cpu(node_blk->i.i_addr[ofs]) != 0) {
>> -			/* should fix this bug all the time */
>> -			FIX_MSG("inline_data has wrong 0'th block = %x",
>> -					le32_to_cpu(node_blk->i.i_addr[ofs]));
>> -			node_blk->i.i_addr[ofs] = 0;
>> -			node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
>> -			need_fix = 1;
>> +		if (blkaddr != 0) {
>> +			ASSERT_MSG("[0x%x] wrong inline reserve blkaddr:%u",
>> +					nid, blkaddr);
>> +			if (c.fix_on) {
>> +				FIX_MSG("inline_data has wrong 0'th block = %x",
>> +								blkaddr);
>> +				node_blk->i.i_addr[ofs] = 0;
>> +				node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
>> +				need_fix = 1;
>> +			}
>>  		}
>>  		if (!i_size || i_size > inline_size) {
>> -			node_blk->i.i_size = cpu_to_le64(inline_size);
>> -			FIX_MSG("inline_data has wrong i_size %lu",
>> -						(unsigned long)i_size);
>> -			need_fix = 1;
>> +			ASSERT_MSG("[0x%x] wrong inline size:%lu",
>> +					nid, (unsigned long)i_size);
>> +			if (c.fix_on) {
>> +				node_blk->i.i_size = cpu_to_le64(inline_size);
>> +				FIX_MSG("inline_data has wrong i_size %lu",
>> +							(unsigned long)i_size);
>> +				need_fix = 1;
>> +			}
>>  		}
>>  		if (!(node_blk->i.i_inline & F2FS_DATA_EXIST)) {
>>  			char buf[MAX_INLINE_DATA(node_blk)];
>> @@ -793,9 +814,12 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>>  
>>  			if (memcmp(buf, inline_data_addr(node_blk),
>>  						MAX_INLINE_DATA(node_blk))) {
>> -				FIX_MSG("inline_data has DATA_EXIST");
>> -				node_blk->i.i_inline |= F2FS_DATA_EXIST;
>> -				need_fix = 1;
>> +				ASSERT_MSG("[0x%x] junk inline data", nid);
>> +				if (c.fix_on) {
>> +					FIX_MSG("inline_data has DATA_EXIST");
>> +					node_blk->i.i_inline |= F2FS_DATA_EXIST;
>> +					need_fix = 1;
>> +				}
>>  			}
>>  		}
>>  		DBG(3, "ino[0x%x] has inline data!\n", nid);
>> @@ -804,20 +828,25 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>>  	}
>>  
>>  	if ((node_blk->i.i_inline & F2FS_INLINE_DENTRY)) {
>> +		block_t blkaddr = le32_to_cpu(node_blk->i.i_addr[ofs]);
>> +
>>  		DBG(3, "ino[0x%x] has inline dentry!\n", nid);
>> -		if (le32_to_cpu(node_blk->i.i_addr[ofs]) != 0) {
>> -			/* should fix this bug all the time */
>> -			FIX_MSG("inline_dentry has wrong 0'th block = %x",
>> -					le32_to_cpu(node_blk->i.i_addr[ofs]));
>> -			node_blk->i.i_addr[ofs] = 0;
>> -			node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
>> -			need_fix = 1;
>> +		if (blkaddr != 0) {
>> +			ASSERT_MSG("[0x%x] wrong inline reserve blkaddr:%u",
>> +								nid, blkaddr);
>> +			if (c.fix_on) {
>> +				FIX_MSG("inline_dentry has wrong 0'th block = %x",
>> +								blkaddr);
>> +				node_blk->i.i_addr[ofs] = 0;
>> +				node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
>> +				need_fix = 1;
>> +			}
>>  		}
>>  
>>  		ret = fsck_chk_inline_dentries(sbi, node_blk, &child);
>>  		if (ret < 0) {
>> -			/* should fix this bug all the time */
>> -			need_fix = 1;
>> +			if (c.fix_on)
>> +				need_fix = 1;
>>  		}
>>  		child.state |= FSCK_INLINE_INODE;
>>  		goto check;
>> @@ -999,7 +1028,7 @@ skip_blkcnt_fix:
>>  	free(en);
>>  
>>  	if (ftype == F2FS_FT_SYMLINK && i_blocks && i_size == 0) {
>> -		DBG(1, "ino: 0x%x i_blocks: %lu with zero i_size",
>> +		ASSERT_MSG("ino: 0x%x i_blocks: %lu with zero i_size\n",
>>  						nid, (unsigned long)i_blocks);
>>  		if (c.fix_on) {
>>  			u64 i_size = i_blocks * F2FS_BLKSIZE;
>> @@ -1012,7 +1041,7 @@ skip_blkcnt_fix:
>>  	}
>>  
>>  	if (ftype == F2FS_FT_ORPHAN && i_links) {
>> -		MSG(0, "ino: 0x%x is orphan inode, but has i_links: %u",
>> +		ASSERT_MSG("ino: 0x%x is orphan inode, but has i_links: %u",
>>  				nid, i_links);
>>  		if (c.fix_on) {
>>  			node_blk->i.i_links = 0;
>> -- 
>> 2.18.0.rc1
> .
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

* Re: [f2fs-dev] [PATCH 2/4] fsck.f2fs: fix to check c.fix_on before repair
  2019-08-16  1:20     ` Chao Yu
@ 2019-08-20 11:34       ` Chao Yu
  0 siblings, 0 replies; 9+ messages in thread
From: Chao Yu @ 2019-08-20 11:34 UTC (permalink / raw)
  To: Jaegeuk Kim; +Cc: linux-f2fs-devel

Jaegeuk,

Could you merge [2,3,4/4] patch as well, I just hit one case with por_fsstress.

[ASSERT] (fsck_chk_inode_blk: 803)  --> [0x1ec84] wrong inline size:149820

Thanks,

On 2019/8/16 9:20, Chao Yu wrote:
> On 2019/8/16 9:06, Jaegeuk Kim wrote:
>> On 08/12, Chao Yu wrote:
>>> We should always set c.bug_on whenever found a bug, then fix them
>>> if c.fix_on is on, otherwise, some bugs won't be shown unless we
>>> enable debug log.
>>>
>>> Signed-off-by: Chao Yu <yuchao0@huawei.com>
>>> ---
>>>  fsck/fsck.c | 137 +++++++++++++++++++++++++++++++---------------------
>>>  1 file changed, 83 insertions(+), 54 deletions(-)
>>>
>>> diff --git a/fsck/fsck.c b/fsck/fsck.c
>>> index 7eb599d..91ddd49 100644
>>> --- a/fsck/fsck.c
>>> +++ b/fsck/fsck.c
>>> @@ -712,12 +712,13 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>>>  	fsck_reada_node_block(sbi, le32_to_cpu(node_blk->i.i_xattr_nid));
>>>  
>>>  	if (fsck_chk_xattr_blk(sbi, nid,
>>> -			le32_to_cpu(node_blk->i.i_xattr_nid), blk_cnt) &&
>>> -			c.fix_on) {
>>> -		node_blk->i.i_xattr_nid = 0;
>>> -		need_fix = 1;
>>> -		FIX_MSG("Remove xattr block: 0x%x, x_nid = 0x%x",
>>> -				nid, le32_to_cpu(node_blk->i.i_xattr_nid));
>>> +			le32_to_cpu(node_blk->i.i_xattr_nid), blk_cnt)) {
>>> +		if (c.fix_on) {
>>> +			node_blk->i.i_xattr_nid = 0;
>>> +			need_fix = 1;
>>> +			FIX_MSG("Remove xattr block: 0x%x, x_nid = 0x%x",
>>> +					nid, le32_to_cpu(node_blk->i.i_xattr_nid));
>>> +		}
>>
>> Why do we need this change?
> 
> Actually, it's not necessary, but just cleanup to keep the same style. :P
> 
> Thanks,
> 
>>
>>>  	}
>>>  
>>>  	if (ftype == F2FS_FT_CHRDEV || ftype == F2FS_FT_BLKDEV ||
>>> @@ -730,24 +731,32 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>>>  
>>>  	if (f2fs_has_extra_isize(&node_blk->i)) {
>>>  		if (c.feature & cpu_to_le32(F2FS_FEATURE_EXTRA_ATTR)) {
>>> -			if (node_blk->i.i_extra_isize >
>>> -				cpu_to_le16(F2FS_TOTAL_EXTRA_ATTR_SIZE)) {
>>> -				FIX_MSG("ino[0x%x] recover i_extra_isize "
>>> -					"from %u to %u",
>>> -					nid,
>>> -					le16_to_cpu(node_blk->i.i_extra_isize),
>>> -					calc_extra_isize());
>>> -				node_blk->i.i_extra_isize =
>>> -					cpu_to_le16(calc_extra_isize());
>>> -				need_fix = 1;
>>> +			unsigned int isize =
>>> +				le16_to_cpu(node_blk->i.i_extra_isize);
>>> +
>>> +			if (isize > F2FS_TOTAL_EXTRA_ATTR_SIZE) {
>>> +				ASSERT_MSG("[0x%x] wrong i_extra_isize=0x%x",
>>> +						nid, isize);
>>> +				if (c.fix_on) {
>>> +					FIX_MSG("ino[0x%x] recover i_extra_isize "
>>> +						"from %u to %u",
>>> +						nid, isize,
>>> +						calc_extra_isize());
>>> +					node_blk->i.i_extra_isize =
>>> +						cpu_to_le16(calc_extra_isize());
>>> +					need_fix = 1;
>>> +				}
>>>  			}
>>>  		} else {
>>> -			FIX_MSG("ino[0x%x] remove F2FS_EXTRA_ATTR "
>>> -				"flag in i_inline:%u",
>>> -				nid, node_blk->i.i_inline);
>>> -			/* we don't support tuning F2FS_FEATURE_EXTRA_ATTR now */
>>> -			node_blk->i.i_inline &= ~F2FS_EXTRA_ATTR;
>>> -			need_fix = 1;
>>> +			ASSERT_MSG("[0x%x] wrong extra_attr flag", nid);
>>> +			if (c.fix_on) {
>>> +				FIX_MSG("ino[0x%x] remove F2FS_EXTRA_ATTR "
>>> +					"flag in i_inline:%u",
>>> +					nid, node_blk->i.i_inline);
>>> +				/* we don't support tuning F2FS_FEATURE_EXTRA_ATTR now */
>>> +				node_blk->i.i_inline &= ~F2FS_EXTRA_ATTR;
>>> +				need_fix = 1;
>>> +			}
>>>  		}
>>>  
>>>  		if ((c.feature &
>>> @@ -758,13 +767,17 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>>>  
>>>  			if (!inline_size ||
>>>  					inline_size > MAX_INLINE_XATTR_SIZE) {
>>> -				FIX_MSG("ino[0x%x] recover inline xattr size "
>>> -					"from %u to %u",
>>> -					nid, inline_size,
>>> -					DEFAULT_INLINE_XATTR_ADDRS);
>>> -				node_blk->i.i_inline_xattr_size =
>>> -					cpu_to_le16(DEFAULT_INLINE_XATTR_ADDRS);
>>> -				need_fix = 1;
>>> +				ASSERT_MSG("[0x%x] wrong inline_xattr_size:%u",
>>> +						nid, inline_size);
>>> +				if (c.fix_on) {
>>> +					FIX_MSG("ino[0x%x] recover inline xattr size "
>>> +						"from %u to %u",
>>> +						nid, inline_size,
>>> +						DEFAULT_INLINE_XATTR_ADDRS);
>>> +					node_blk->i.i_inline_xattr_size =
>>> +						cpu_to_le16(DEFAULT_INLINE_XATTR_ADDRS);
>>> +					need_fix = 1;
>>> +				}
>>>  			}
>>>  		}
>>>  	}
>>> @@ -772,20 +785,28 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>>>  
>>>  	if ((node_blk->i.i_inline & F2FS_INLINE_DATA)) {
>>>  		unsigned int inline_size = MAX_INLINE_DATA(node_blk);
>>> +		block_t blkaddr = le32_to_cpu(node_blk->i.i_addr[ofs]);
>>>  
>>> -		if (le32_to_cpu(node_blk->i.i_addr[ofs]) != 0) {
>>> -			/* should fix this bug all the time */
>>> -			FIX_MSG("inline_data has wrong 0'th block = %x",
>>> -					le32_to_cpu(node_blk->i.i_addr[ofs]));
>>> -			node_blk->i.i_addr[ofs] = 0;
>>> -			node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
>>> -			need_fix = 1;
>>> +		if (blkaddr != 0) {
>>> +			ASSERT_MSG("[0x%x] wrong inline reserve blkaddr:%u",
>>> +					nid, blkaddr);
>>> +			if (c.fix_on) {
>>> +				FIX_MSG("inline_data has wrong 0'th block = %x",
>>> +								blkaddr);
>>> +				node_blk->i.i_addr[ofs] = 0;
>>> +				node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
>>> +				need_fix = 1;
>>> +			}
>>>  		}
>>>  		if (!i_size || i_size > inline_size) {
>>> -			node_blk->i.i_size = cpu_to_le64(inline_size);
>>> -			FIX_MSG("inline_data has wrong i_size %lu",
>>> -						(unsigned long)i_size);
>>> -			need_fix = 1;
>>> +			ASSERT_MSG("[0x%x] wrong inline size:%lu",
>>> +					nid, (unsigned long)i_size);
>>> +			if (c.fix_on) {
>>> +				node_blk->i.i_size = cpu_to_le64(inline_size);
>>> +				FIX_MSG("inline_data has wrong i_size %lu",
>>> +							(unsigned long)i_size);
>>> +				need_fix = 1;
>>> +			}
>>>  		}
>>>  		if (!(node_blk->i.i_inline & F2FS_DATA_EXIST)) {
>>>  			char buf[MAX_INLINE_DATA(node_blk)];
>>> @@ -793,9 +814,12 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>>>  
>>>  			if (memcmp(buf, inline_data_addr(node_blk),
>>>  						MAX_INLINE_DATA(node_blk))) {
>>> -				FIX_MSG("inline_data has DATA_EXIST");
>>> -				node_blk->i.i_inline |= F2FS_DATA_EXIST;
>>> -				need_fix = 1;
>>> +				ASSERT_MSG("[0x%x] junk inline data", nid);
>>> +				if (c.fix_on) {
>>> +					FIX_MSG("inline_data has DATA_EXIST");
>>> +					node_blk->i.i_inline |= F2FS_DATA_EXIST;
>>> +					need_fix = 1;
>>> +				}
>>>  			}
>>>  		}
>>>  		DBG(3, "ino[0x%x] has inline data!\n", nid);
>>> @@ -804,20 +828,25 @@ void fsck_chk_inode_blk(struct f2fs_sb_info *sbi, u32 nid,
>>>  	}
>>>  
>>>  	if ((node_blk->i.i_inline & F2FS_INLINE_DENTRY)) {
>>> +		block_t blkaddr = le32_to_cpu(node_blk->i.i_addr[ofs]);
>>> +
>>>  		DBG(3, "ino[0x%x] has inline dentry!\n", nid);
>>> -		if (le32_to_cpu(node_blk->i.i_addr[ofs]) != 0) {
>>> -			/* should fix this bug all the time */
>>> -			FIX_MSG("inline_dentry has wrong 0'th block = %x",
>>> -					le32_to_cpu(node_blk->i.i_addr[ofs]));
>>> -			node_blk->i.i_addr[ofs] = 0;
>>> -			node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
>>> -			need_fix = 1;
>>> +		if (blkaddr != 0) {
>>> +			ASSERT_MSG("[0x%x] wrong inline reserve blkaddr:%u",
>>> +								nid, blkaddr);
>>> +			if (c.fix_on) {
>>> +				FIX_MSG("inline_dentry has wrong 0'th block = %x",
>>> +								blkaddr);
>>> +				node_blk->i.i_addr[ofs] = 0;
>>> +				node_blk->i.i_blocks = cpu_to_le64(*blk_cnt);
>>> +				need_fix = 1;
>>> +			}
>>>  		}
>>>  
>>>  		ret = fsck_chk_inline_dentries(sbi, node_blk, &child);
>>>  		if (ret < 0) {
>>> -			/* should fix this bug all the time */
>>> -			need_fix = 1;
>>> +			if (c.fix_on)
>>> +				need_fix = 1;
>>>  		}
>>>  		child.state |= FSCK_INLINE_INODE;
>>>  		goto check;
>>> @@ -999,7 +1028,7 @@ skip_blkcnt_fix:
>>>  	free(en);
>>>  
>>>  	if (ftype == F2FS_FT_SYMLINK && i_blocks && i_size == 0) {
>>> -		DBG(1, "ino: 0x%x i_blocks: %lu with zero i_size",
>>> +		ASSERT_MSG("ino: 0x%x i_blocks: %lu with zero i_size\n",
>>>  						nid, (unsigned long)i_blocks);
>>>  		if (c.fix_on) {
>>>  			u64 i_size = i_blocks * F2FS_BLKSIZE;
>>> @@ -1012,7 +1041,7 @@ skip_blkcnt_fix:
>>>  	}
>>>  
>>>  	if (ftype == F2FS_FT_ORPHAN && i_links) {
>>> -		MSG(0, "ino: 0x%x is orphan inode, but has i_links: %u",
>>> +		ASSERT_MSG("ino: 0x%x is orphan inode, but has i_links: %u",
>>>  				nid, i_links);
>>>  		if (c.fix_on) {
>>>  			node_blk->i.i_links = 0;
>>> -- 
>>> 2.18.0.rc1
>> .
>>
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> .
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

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

end of thread, other threads:[~2019-08-20 11:34 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-12 11:45 [f2fs-dev] [PATCH 1/4] fsck.f2fs: fix to recover out-of-border inline size Chao Yu
2019-08-12 11:45 ` [f2fs-dev] [PATCH 2/4] fsck.f2fs: fix to check c.fix_on before repair Chao Yu
2019-08-16  1:06   ` Jaegeuk Kim
2019-08-16  1:20     ` Chao Yu
2019-08-20 11:34       ` Chao Yu
2019-08-12 11:45 ` [f2fs-dev] [PATCH 3/4] fsck.f2fs: fix to show removed x_nid correctly Chao Yu
2019-08-12 11:45 ` [f2fs-dev] [PATCH 4/4] fsck.f2fs: fix symlink correctly Chao Yu
2019-08-15 22:39 ` [f2fs-dev] [PATCH 1/4] fsck.f2fs: fix to recover out-of-border inline size Jaegeuk Kim
2019-08-16  1:03   ` Jaegeuk Kim

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).