On 2019/10/23 1:09, Jaegeuk Kim wrote:
> On 10/22, Chao Yu wrote:
>> On 2019/10/18 14:37, Shin'ichiro Kawasaki wrote:
>>> Fsck checks fsync data when UMOUNT flag is not set. When the f2fs was not
>>> cleanly unmouted, UMOUNT flag is not recorded in meta data and fsync data
>>> can be left in the f2fs. The first fsck run checks fsync data to reflect
>>> it on quota status recovery. After that, fsck writes UMOUNT flag in the
>>> f2fs meta data so that second fsck run can skip fsync data check.
>>
>> Oh, IMO, we need that flag to let fsck know there is still fsynced data in
>> the
>> image, could we just leave it as it is?
>>
>> To Jaegeuk, thoughts?
>
> I don't have objection to apply this patch. :P
Anyway, looks my concern is another topic, let's go ahead with this patch.
Reviewed-by: Chao Yu <yuch...@huawei.com>
Thanks,
>
>>
>> Thanks,
>>
>>>
>>> However, fsck for zoned block devices need to care fsync data for all
>>> fsck runs. The first fsck run checks fsync data, then fsck can check
>>> write pointer consistency with fsync data. However, since second fsck run
>>> does not check fsync data, fsck detects write pointer at fsync data end
>>> is not consistent with f2fs meta data. This results in meta data update
>>> by fsck and fsync data gets lost.
>>>
>>> To have fsck check fsync data always for zoned block devices, introduce
>>> need_fsync_data_record() helper function which returns boolean to tell
>>> if fsck needs fsync data check or not. For zoned block devices, always
>>> return true. Otherwise, return true if UMOUNT flag is not set in CP.
>>> Replace UMOUNT flag check codes for fsync data with the function call.
>>>
>>> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawas...@wdc.com>
>>> ---
>>> fsck/fsck.h | 6 ++++++
>>> fsck/mount.c | 14 +++++++-------
>>> fsck/segment.c | 2 +-
>>> 3 files changed, 14 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/fsck/fsck.h b/fsck/fsck.h
>>> index 8da0ebb..75052d8 100644
>>> --- a/fsck/fsck.h
>>> +++ b/fsck/fsck.h
>>> @@ -133,6 +133,12 @@ enum seg_type {
>>>
>>> struct selabel_handle;
>>>
>>> +static inline bool need_fsync_data_record(struct f2fs_sb_info *sbi)
>>> +{
>>> + return !is_set_ckpt_flags(F2FS_CKPT(sbi), CP_UMOUNT_FLAG) ||
>>> + c.zoned_model == F2FS_ZONED_HM;
>>> +}
>>> +
>>> extern int fsck_chk_orphan_node(struct f2fs_sb_info *);
>>> extern int fsck_chk_quota_node(struct f2fs_sb_info *);
>>> extern int fsck_chk_quota_files(struct f2fs_sb_info *);
>>> diff --git a/fsck/mount.c b/fsck/mount.c
>>> index 915f487..2979865 100644
>>> --- a/fsck/mount.c
>>> +++ b/fsck/mount.c
>>> @@ -1525,7 +1525,7 @@ int build_sit_info(struct f2fs_sb_info *sbi)
>>>
>>> bitmap_size = TOTAL_SEGS(sbi) * SIT_VBLOCK_MAP_SIZE;
>>>
>>> - if (!is_set_ckpt_flags(cp, CP_UMOUNT_FLAG))
>>> + if (need_fsync_data_record(sbi))
>>> bitmap_size += bitmap_size;
>>>
>>> sit_i->bitmap = calloc(bitmap_size, 1);
>>> @@ -1540,7 +1540,7 @@ int build_sit_info(struct f2fs_sb_info *sbi)
>>> sit_i->sentries[start].cur_valid_map = bitmap;
>>> bitmap += SIT_VBLOCK_MAP_SIZE;
>>>
>>> - if (!is_set_ckpt_flags(cp, CP_UMOUNT_FLAG)) {
>>> + if (need_fsync_data_record(sbi)) {
>>> sit_i->sentries[start].ckpt_valid_map = bitmap;
>>> bitmap += SIT_VBLOCK_MAP_SIZE;
>>> }
>>> @@ -1887,7 +1887,7 @@ void seg_info_from_raw_sit(struct f2fs_sb_info *sbi,
>>> struct seg_entry *se,
>>> {
>>> __seg_info_from_raw_sit(se, raw_sit);
>>>
>>> - if (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_UMOUNT_FLAG))
>>> + if (!need_fsync_data_record(sbi))
>>> return;
>>> se->ckpt_valid_blocks = se->valid_blocks;
>>> memcpy(se->ckpt_valid_map, se->cur_valid_map, SIT_VBLOCK_MAP_SIZE);
>>> @@ -1903,7 +1903,7 @@ struct seg_entry *get_seg_entry(struct f2fs_sb_info
>>> *sbi,
>>>
>>> unsigned short get_seg_vblocks(struct f2fs_sb_info *sbi, struct seg_entry
>>> *se)
>>> {
>>> - if (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_UMOUNT_FLAG))
>>> + if (!need_fsync_data_record(sbi))
>>> return se->valid_blocks;
>>> else
>>> return se->ckpt_valid_blocks;
>>> @@ -1911,7 +1911,7 @@ unsigned short get_seg_vblocks(struct f2fs_sb_info
>>> *sbi, struct seg_entry *se)
>>>
>>> unsigned char *get_seg_bitmap(struct f2fs_sb_info *sbi, struct seg_entry
>>> *se)
>>> {
>>> - if (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_UMOUNT_FLAG))
>>> + if (!need_fsync_data_record(sbi))
>>> return se->cur_valid_map;
>>> else
>>> return se->ckpt_valid_map;
>>> @@ -1919,7 +1919,7 @@ unsigned char *get_seg_bitmap(struct f2fs_sb_info
>>> *sbi, struct seg_entry *se)
>>>
>>> unsigned char get_seg_type(struct f2fs_sb_info *sbi, struct seg_entry *se)
>>> {
>>> - if (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_UMOUNT_FLAG))
>>> + if (!need_fsync_data_record(sbi))
>>> return se->type;
>>> else
>>> return se->ckpt_type;
>>> @@ -3242,7 +3242,7 @@ static int record_fsync_data(struct f2fs_sb_info *sbi)
>>> struct list_head inode_list = LIST_HEAD_INIT(inode_list);
>>> int ret;
>>>
>>> - if (is_set_ckpt_flags(F2FS_CKPT(sbi), CP_UMOUNT_FLAG))
>>> + if (!need_fsync_data_record(sbi))
>>> return 0;
>>>
>>> ret = find_fsync_inode(sbi, &inode_list);
>>> diff --git a/fsck/segment.c b/fsck/segment.c
>>> index ccde05f..17c42b7 100644
>>> --- a/fsck/segment.c
>>> +++ b/fsck/segment.c
>>> @@ -62,7 +62,7 @@ int reserve_new_block(struct f2fs_sb_info *sbi, block_t
>>> *to,
>>> se->type = type;
>>> se->valid_blocks++;
>>> f2fs_set_bit(offset, (char *)se->cur_valid_map);
>>> - if (!is_set_ckpt_flags(F2FS_CKPT(sbi), CP_UMOUNT_FLAG)) {
>>> + if (need_fsync_data_record(sbi)) {
>>> se->ckpt_type = type;
>>> se->ckpt_valid_blocks++;
>>> f2fs_set_bit(offset, (char *)se->ckpt_valid_map);
>>>
> .
>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
