On Nov 28, 2019 / 20:26, Chao Yu wrote:
> On 2019/11/28 12:07, Shinichiro Kawasaki wrote:
> > On Nov 25, 2019 / 14:59, Chao Yu wrote:
> >> On 2019/11/14 16:19, Shin'ichiro Kawasaki wrote:
> >>> On sudden f2fs shutdown, write pointers of zoned block devices can go
> >>> further but f2fs meta data keeps current segments at positions before the
> >>> write operations. After remounting the f2fs, this inconsistency causes
> >>> write operations not at write pointers and "Unaligned write command"
> >>> error is reported.
> >>>
> >>> To avoid the error, compare current segments with write pointers of open
> >>> zones the current segments point to, during mount operation. If the write
> >>> pointer position is not aligned with the current segment position, assign
> >>> a new zone to the current segment. Also check the newly assigned zone has
> >>> write pointer at zone start. If not, make mount fail and ask users to run
> >>> fsck.
> >>>
> >>> Perform the consistency check during fsync recovery. Not to lose the
> >>> fsync data, do the check after fsync data gets restored and before
> >>> checkpoint commit which flushes data at current segment positions. Not to
> >>> cause conflict with kworker's dirfy data/node flush, do the fix within
> >>> SBI_POR_DOING protection.
> >>>
> >>> Signed-off-by: Shin'ichiro Kawasaki <[email protected]>
> >>> ---
> >>> fs/f2fs/f2fs.h | 1 +
> >>> fs/f2fs/recovery.c | 17 ++++++-
> >>> fs/f2fs/segment.c | 120 +++++++++++++++++++++++++++++++++++++++++++++
> >>> 3 files changed, 136 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> >>> index 4024790028aa..a2e24718c13b 100644
> >>> --- a/fs/f2fs/f2fs.h
> >>> +++ b/fs/f2fs/f2fs.h
> >>> @@ -3136,6 +3136,7 @@ void f2fs_write_node_summaries(struct f2fs_sb_info
> >>> *sbi, block_t start_blk);
> >>> int f2fs_lookup_journal_in_cursum(struct f2fs_journal *journal, int type,
> >>> unsigned int val, int alloc);
> >>> void f2fs_flush_sit_entries(struct f2fs_sb_info *sbi, struct cp_control
> >>> *cpc);
> >>> +int f2fs_fix_curseg_write_pointer(struct f2fs_sb_info *sbi);
> >>> int f2fs_build_segment_manager(struct f2fs_sb_info *sbi);
> >>> void f2fs_destroy_segment_manager(struct f2fs_sb_info *sbi);
> >>> int __init f2fs_create_segment_manager_caches(void);
> >>> diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
> >>> index 783773e4560d..712054ed8d64 100644
> >>> --- a/fs/f2fs/recovery.c
> >>> +++ b/fs/f2fs/recovery.c
> >>> @@ -784,9 +784,22 @@ int f2fs_recover_fsync_data(struct f2fs_sb_info
> >>> *sbi, bool check_only)
> >>> if (err) {
> >>> truncate_inode_pages_final(NODE_MAPPING(sbi));
> >>> truncate_inode_pages_final(META_MAPPING(sbi));
> >>> - } else {
> >>> - clear_sbi_flag(sbi, SBI_POR_DOING);
> >>> }
> >>> +
> >>> + /*
> >>> + * If fsync data succeeds or there is no fsync data to recover,
> >>> + * and the f2fs is not read only, check and fix zoned block devices'
> >>> + * write pointer consistency.
> >>> + */
> >>> + if (!ret && !err && !f2fs_readonly(sbi->sb)
> >>
> >> Using !check_only will be more readable?
> >>
> >> if (!err && !check_only && !f2fs_readonly(sbi->sb)
> >
> > When check_only is on and there is no fsync data, I think we should fix the
> > write pointer inconsistency. With the condition you suggested, this case can
> > not be covered.
>
> Alright.
>
> >
> > Having said that, my expression with !ret is not good from readability point
> > of view. How about this?
> >
> >
> > bool fix_curseg_write_pointer;
> > fix_curseg_write_pointer = !check_only || (check_only &&
> > list_empty(&inode_list));
>
> fix_curseg_write_pointer = !check_only || list_empty(&inode_list); is enough.
Oops, thanks.
>
> >
> > ...
> >
> > if (!err && fix_curseg_write_pointer && !f2fs_readonly(sbi->sb)
> > && f2fs_sb_has_blkzoned(sbi)) {
> > err = f2fs_fix_curseg_write_pointer(sbi);
> > ret = err;
> > }
>
> It's okay to me.
Will update the patch. Thanks!
--
Best Regards,
Shin'ichiro Kawasaki
_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
