On 2018/1/2 14:49, Chao Yu wrote:
On 2017/12/30 15:42, Yunlong Song wrote:
In some case, the node blocks has wrong blkaddr whose segment type is
You mean *data block* has wrong blkaddr whose segment type is NODE?
Yes.
NODE, e.g., recover inode has missing xattr flag and the blkaddr is in
the xattr range. Since fsck.f2fs does not check the recovery nodes, this
will cause __f2fs_replace_block change the curseg of node and do the
update_sit_entry(sbi, new_blkaddr, 1) with no next_blkoff refresh, as a
Do you mean the root cause is that __f2fs_replace_block didn't update
next_blkoff?
No, it's not the root cause. The root cause may be something like DDR flip.
result, when recovery process write checkpoint and sync nodes, the
next_blkoff of curseg is used in the segment bit map, then it will
cause f2fs_bug_on. So let's check the segment type before recover data,
and stop recover if it is not in DATA segment.
Sorry, I can't catch the whole cause and effect from you description, if
possible, could you give an example?
For example, the i_inline flag has F2FS_INLINE_XATTR, and the last 50
i_addrs have xattr
context. But if DDR flips, the i_inline flag may miss F2FS_INLINE_XATTR,
and the last 50 i_addrs
are considered as data block addr. If the xattr context is 0x1234, and
0x1234 happens to be
a valid block addr, and the block 0x1234 happens to be in a warm node
segment. Then do_recover_data
will call f2fs_replace_block() with dest = 0x1234, which will change
curseg of warm node to
0x1234's segment, and make update_sit_entry(sbi, 0x1234, 1), the
curseg->next_blkoff also
points to 0x1234's offset in its segment. When recovery process calls
write_checkpoint, sync
nodes will write to 0x1234's offset of curseg warm node. The
update_sit_entry will check that
offset and find the bitmap is already set to 1 and then calls f2fs_bug_on.
Thanks,
Signed-off-by: Yunlong Song <yunlong.song@xxxxxxxxxx>
---
fs/f2fs/recovery.c | 3 ++-
fs/f2fs/segment.h | 3 +++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 7d63faf..e8fee4a 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -478,7 +478,8 @@ static int do_recover_data(struct f2fs_sb_info *sbi, struct inode *inode,
}
/* dest is valid block, try to recover from src to dest */
- if (is_valid_blkaddr(sbi, dest, META_POR)) {
+ if (is_valid_blkaddr(sbi, dest, META_POR) &&
+ is_data_blkaddr(sbi, dest)) {
if (src == NULL_ADDR) {
err = reserve_new_block(&dn);
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 71a2aaa..5c5a215 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -115,6 +115,9 @@
#define SECTOR_TO_BLOCK(sectors) \
((sectors) >> F2FS_LOG_SECTORS_PER_BLOCK)
+#define is_data_blkaddr(sbi, blkaddr) \
+ (IS_DATASEG(get_seg_entry(sbi, GET_SEGNO(sbi, blkaddr))->type))
+
/*
* indicate a block allocation direction: RIGHT and LEFT.
* RIGHT means allocating new sections towards the end of volume.
.
--
Thanks,
Yunlong Song
