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