On 2017/2/24 19:37, Chao Yu wrote: > On 2017/2/24 19:11, Yunlong Song wrote: >> I think we do not need to care about the CP_CRC_RECOVERY_FLAG status of old >> image, I mean we >> do not need to check the already-been-written node footer in the image, what >> we care about is the >> on-going-to-write node footer, which is used for recovery. >> >> If CP_CRC_RECOVERY_FLAG is defined, then __set_ckpt_flags(ckpt, >> CP_CRC_RECOVERY_FLAG); is >> executed in each do_checkpoint actually, and CP will have that flag for each >> on-going-to-write node footer. >> I think the recovery process only needs to use the on-going-to-write node >> rather than the already-been-written >> node in the old image. The already-been-written node in the old image >> should not appear in the node >> chain of recovery process, right? > Previously, we changed the disk layout of footer in node block, and then we > applied new verifying approach which has better reliability in order to avoid > chaining garbage node block. > > In order to distinguish old disk layout and the new one, we introduce > CP_CRC_RECOVERY_FLAG, once a CP is triggered, we will tag current CP with the > flag, and use new disk layout and new verifying approach for the following > node > block updating flow and abnormal power-cut recovery flow. > > For old image which has no CP_CRC_RECOVERY_FLAG flag been set, f2fs needs to > use > old disk layout and old verifying approach during recovery for the > compatibility. So that's why we need to check the flag in CP here.
For the old disk layout, because we still use new approach to set CP_CRC_RECOVERY_FLAG in the node footer in each do_checkpoint, then I think f2fs should also use new verifying approach during recovery rather than old verifying approach. What is the problem if we do like this? > Thanks, > >> On 2017/2/24 18:29, Chao Yu wrote: >>> On 2017/2/24 18:06, Yunlong Song wrote: >>>> No need to check the "if" condition each time, just change it to macro >>>> codes. >>> We're going to check flag in CP, not just in code of f2fs. >>> >>> Thanks, >>> >>>> Signed-off-by: Yunlong Song <[email protected]> >>>> --- >>>> fs/f2fs/node.h | 20 ++++++++++---------- >>>> fs/f2fs/segment.c | 5 +++-- >>>> 2 files changed, 13 insertions(+), 12 deletions(-) >>>> >>>> diff --git a/fs/f2fs/node.h b/fs/f2fs/node.h >>>> index 3fc9c4b..3e5a58b 100644 >>>> --- a/fs/f2fs/node.h >>>> +++ b/fs/f2fs/node.h >>>> @@ -303,11 +303,11 @@ static inline void fill_node_footer_blkaddr(struct >>>> page *page, block_t blkaddr) >>>> size_t crc_offset = le32_to_cpu(ckpt->checksum_offset); >>>> __u64 cp_ver = le64_to_cpu(ckpt->checkpoint_ver); >>>> >>>> - if (__is_set_ckpt_flags(ckpt, CP_CRC_RECOVERY_FLAG)) { >>>> - __u64 crc = le32_to_cpu(*((__le32 *) >>>> - ((unsigned char *)ckpt + crc_offset))); >>>> - cp_ver |= (crc << 32); >>>> - } >>>> +#ifdef CP_CRC_RECOVERY_FLAG >>>> + __u64 crc = le32_to_cpu(*((__le32 *) >>>> + ((unsigned char *)ckpt + crc_offset))); >>>> + cp_ver |= (crc << 32); >>>> +#endif >>>> rn->footer.cp_ver = cpu_to_le64(cp_ver); >>>> rn->footer.next_blkaddr = cpu_to_le32(blkaddr); >>>> } >>>> @@ -318,11 +318,11 @@ static inline bool is_recoverable_dnode(struct page >>>> *page) >>>> size_t crc_offset = le32_to_cpu(ckpt->checksum_offset); >>>> __u64 cp_ver = cur_cp_version(ckpt); >>>> >>>> - if (__is_set_ckpt_flags(ckpt, CP_CRC_RECOVERY_FLAG)) { >>>> - __u64 crc = le32_to_cpu(*((__le32 *) >>>> - ((unsigned char *)ckpt + crc_offset))); >>>> - cp_ver |= (crc << 32); >>>> - } >>>> +#ifdef CP_CRC_RECOVERY_FLAG >>>> + __u64 crc = le32_to_cpu(*((__le32 *) >>>> + ((unsigned char *)ckpt + crc_offset))); >>>> + cp_ver |= (crc << 32); >>>> +#endif >>>> return cp_ver == cpver_of_node(page); >>>> } >>>> >>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c >>>> index 9eb6d89..6c2e1ee 100644 >>>> --- a/fs/f2fs/segment.c >>>> +++ b/fs/f2fs/segment.c >>>> @@ -1573,9 +1573,10 @@ static void allocate_segment_by_default(struct >>>> f2fs_sb_info *sbi, >>>> { >>>> if (force) >>>> new_curseg(sbi, type, true); >>>> - else if (!is_set_ckpt_flags(sbi, CP_CRC_RECOVERY_FLAG) && >>>> - type == CURSEG_WARM_NODE) >>>> +#ifndef CP_CRC_RECOVERY_FLAG >>>> + else if (type == CURSEG_WARM_NODE) >>>> new_curseg(sbi, type, false); >>>> +#endif >>>> else if (need_SSR(sbi) && get_ssr_segment(sbi, type)) >>>> change_curseg(sbi, type, true); >>>> else >>>> >>> . >>> >> > > . > -- Thanks, Yunlong Song ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
