f2fs: fix wrong i_atime recovery

From: Chao Yu <[email protected]>

i_times of inode will be set with current system time which can be configured through 'date', so it's not safe to judge dnode block as garbage data depend on i_times.

Now, we have used enhanced 'cp_ver + cp' crc method to verify valid dnode block, so I expect recoverying invalid dnode is almost not possible.

Signed-off-by Chao Yu <[email protected]>

---
 fs/f2fs/recovery.c | 31 +------------------------------
 1 file changed, 1 insertion(+), 30 deletions(-)

fs/f2fs/recovery.c +1 -30

index d2ba4da..62523b2 100644
@@ -196,32 +196,6 @@ static void recover_inode(struct inode *inode, struct page *page)
 			ino_of_node(page), name);
 }
 
-static bool is_same_inode(struct inode *inode, struct page *ipage)
-{
-	struct f2fs_inode *ri = F2FS_INODE(ipage);
-	struct timespec disk;
-
-	if (!IS_INODE(ipage))
-		return true;
-
-	disk.tv_sec = le64_to_cpu(ri->i_ctime);
-	disk.tv_nsec = le32_to_cpu(ri->i_ctime_nsec);
-	if (timespec_compare(&inode->i_ctime, &disk) > 0)
-		return false;
-
-	disk.tv_sec = le64_to_cpu(ri->i_atime);
-	disk.tv_nsec = le32_to_cpu(ri->i_atime_nsec);
-	if (timespec_compare(&inode->i_atime, &disk) > 0)
-		return false;
-
-	disk.tv_sec = le64_to_cpu(ri->i_mtime);
-	disk.tv_nsec = le32_to_cpu(ri->i_mtime_nsec);
-	if (timespec_compare(&inode->i_mtime, &disk) > 0)
-		return false;
-
-	return true;
-}
-
 static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head)
 {
 	struct curseg_info *curseg;

@@ -248,10 +222,7 @@ static int find_fsync_dnodes(struct f2fs_sb_info *sbi, struct list_head *head)
 			goto next;
 
 		entry = get_fsync_inode(head, ino_of_node(page));
-		if (entry) {
-			if (!is_same_inode(entry->inode, page))
-				goto next;
-		} else {
+		if (!entry) {
 			if (IS_INODE(page) && is_dent_dnode(page)) {
 				err = recover_inode_page(sbi, page);
 				if (err)

Signature · 1 line

2.10.1

Quoted 5 lines · Chao Yu

From: Chao Yu <[email protected]>

i_times of inode will be set with current system time which can be configured through 'date', so it's not safe to judge dnode block as garbage data depend on i_times.

This is not to detect garbage data, but to skip redundant unchanged inode.

Quoted 7 lines · Jaegeuk Kim

Quoted 5 lines · Chao Yu

From: Chao Yu <[email protected]>

i_times of inode will be set with current system time which can be configured through 'date', so it's not safe to judge dnode block as garbage data depend on i_times.

Quoted 1 line

This is not to detect garbage data, but to skip redundant unchanged inode.

Oops, seems 807b1e1c8e08 ("f2fs: do not recover from previous remained wrong dnodes") did't describe like that. But after reading the codes, it looks like the purpose of this change is to skip unchanged inode. So, commit log in original is incorrect, right?

Thanks,

Quoted 8 lines · Chao Yu

Quoted 7 lines · Jaegeuk Kim

Quoted 5 lines · Chao Yu

From: Chao Yu <[email protected]>

i_times of inode will be set with current system time which can be configured through 'date', so it's not safe to judge dnode block as garbage data depend on i_times.

Quoted 6 lines

Quoted 1 line

This is not to detect garbage data, but to skip redundant unchanged inode.

Oops, seems 807b1e1c8e08 ("f2fs: do not recover from previous remained wrong dnodes") did't describe like that. But after reading the codes, it looks like the purpose of this change is to skip unchanged inode. So, commit log in original is incorrect, right?

Oh, right. This indicats both of purposes: stale data and detecting same inodes.

Let me just revert the original patch.

Thanks,

Quoted 2 lines

Thanks,

Quoted 9 lines · Jaegeuk Kim

Quoted 8 lines · Chao Yu

Quoted 7 lines · Jaegeuk Kim

Quoted 5 lines · Chao Yu

From: Chao Yu <[email protected]>

i_times of inode will be set with current system time which can be configured through 'date', so it's not safe to judge dnode block as garbage data depend on i_times.

Quoted 8 lines

Quoted 6 lines

Quoted 1 line

This is not to detect garbage data, but to skip redundant unchanged inode.

Oops, seems 807b1e1c8e08 ("f2fs: do not recover from previous remained wrong dnodes") did't describe like that. But after reading the codes, it looks like the purpose of this change is to skip unchanged inode. So, commit log in original is incorrect, right?

Oh, right. This indicats both of purposes: stale data and detecting same inodes.

Alright.

Quoted 1 line

Let me just revert the original patch.

I can see that you have did reverting it in your git tree, but seems commit number is not right.

Could you please merge my updated v2 patch instead?

Thanks,

Quoted 5 lines

Thanks,

Quoted 2 lines

Thanks,