Re: [PATCH 4/6] f2fs: introduce cp_lock to protect updating of ckpt_flags
On 2016/9/20 10:41, Jaegeuk Kim wrote: > On Tue, Sep 20, 2016 at 09:47:20AM +0800, Chao Yu wrote: >> Hi Jaegeuk, >> >> On 2016/9/20 5:49, Jaegeuk Kim wrote: >>> Hi Chao, >>> >>> On Sun, Sep 18, 2016 at 11:30:06PM +0800, Chao Yu wrote: From: Chao Yu This patch introduces spinlock to protect updating process of ckpt_flags field in struct f2fs_checkpoint, it avoids incorrectly updating in race condition. >>> >>> So, I'm seeing a race condition between f2fs_stop_checkpoint(), >>> write_checkpoint(), and f2fs_fill_super(). >>> >>> How about just adding f2fs_lock_op() in f2fs_stop_checkpoint()? >> >> I'm afraid there will be a potential deadlock here: >> >> Thread A Thread B >> - write_checkpoint >> - block_operations >> - f2fs_lock_all >> - do_checkpoint >> - wait_on_all_pages_writeback >> - f2fs_write_end_io >> - f2fs_stop_checkpoint >>- f2fs_lock_op >> - end_page_writeback >> - atomic_dec_and_test >> - wake_up >> >> Right? > > Okay, I see. Let me try to understand in more details. > Basically, there'd be no problem if there is no f2fs_stop_checkpoint(), since > every {set|clear}_ckpt_flags() are called under f2fs_lock_op() or in > fill_super(). And, you're probably concerned about any breakage of ckpt->flags > due to accidental f2fs_stop_checkpoint() trigger. So, we're able to lose > ERROR_FLAG because of data race. > > Oh, I found one potential corruption case in f2fs_write_checkpoint(). > Before writing the last checkpoint block, we used to check its IO error. > But, if set_ckpt_flags() and f2fs_stop_checkpoint() were called concurrently, > ckpt_flags was able to lose ERROR_FLAG, resulting in finalizing its checkpoint > pack. So, we can get valid checkpoint pack with EIO'ed metadata block. That's right. > > BTW, we can do multiple set/clear flags in do_checkpoint() with single > spin_lock > call, tho. Agree, let me refactor the code. :) Thanks, > > Thanks, > >>> Signed-off-by: Chao Yu --- fs/f2fs/checkpoint.c | 24 fs/f2fs/f2fs.h | 29 + fs/f2fs/recovery.c | 2 +- fs/f2fs/segment.c| 4 ++-- fs/f2fs/super.c | 5 +++-- 5 files changed, 39 insertions(+), 25 deletions(-) diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c index df56a43..0338f8c 100644 --- a/fs/f2fs/checkpoint.c +++ b/fs/f2fs/checkpoint.c @@ -28,7 +28,7 @@ struct kmem_cache *inode_entry_slab; void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi, bool end_io) { - set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG); + set_ckpt_flags(sbi, CP_ERROR_FLAG); sbi->sb->s_flags |= MS_RDONLY; if (!end_io) f2fs_flush_merged_bios(sbi); @@ -571,7 +571,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi) block_t start_blk, orphan_blocks, i, j; int err; - if (!is_set_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG)) + if (!is_set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG)) return 0; start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi); @@ -595,7 +595,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi) f2fs_put_page(page, 1); } /* clear Orphan Flag */ - clear_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG); + clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); return 0; } @@ -1054,9 +1054,9 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc) /* 2 cp + n data seg summary + orphan inode blocks */ data_sum_blocks = npages_for_summary_flush(sbi, false); if (data_sum_blocks < NR_CURSEG_DATA_TYPE) - set_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG); + set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG); else - clear_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG); + clear_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG); orphan_blocks = GET_ORPHAN_BLOCKS(orphan_num); ckpt->cp_pack_start_sum = cpu_to_le32(1 + cp_payload_blks + @@ -1072,22 +1072,22 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc) orphan_blocks); if (cpc->reason == CP_UMOUNT) - set_ckpt_flags(ckpt, CP_UMOUNT_FLAG); + set_ckpt_flags(sbi, CP_UMOUNT_FLAG); else - clear_ckpt_flags(ckpt, CP_UMOUNT_FLAG); + clear_ckpt_flags(sbi, CP_UMOUNT_FLAG); if (cpc->reason == CP_FASTBOOT) - set_ckpt_flags(ckpt, CP_FASTBOOT_FLAG); + set_ckpt_flags(sbi, CP_FASTBOOT_FLAG); else
Re: [PATCH 4/6] f2fs: introduce cp_lock to protect updating of ckpt_flags
On Tue, Sep 20, 2016 at 09:47:20AM +0800, Chao Yu wrote: > Hi Jaegeuk, > > On 2016/9/20 5:49, Jaegeuk Kim wrote: > > Hi Chao, > > > > On Sun, Sep 18, 2016 at 11:30:06PM +0800, Chao Yu wrote: > >> From: Chao Yu > >> > >> This patch introduces spinlock to protect updating process of ckpt_flags > >> field in struct f2fs_checkpoint, it avoids incorrectly updating in race > >> condition. > > > > So, I'm seeing a race condition between f2fs_stop_checkpoint(), > > write_checkpoint(), and f2fs_fill_super(). > > > > How about just adding f2fs_lock_op() in f2fs_stop_checkpoint()? > > I'm afraid there will be a potential deadlock here: > > Thread A Thread B > - write_checkpoint > - block_operations > - f2fs_lock_all > - do_checkpoint > - wait_on_all_pages_writeback > - f2fs_write_end_io >- f2fs_stop_checkpoint > - f2fs_lock_op >- end_page_writeback >- atomic_dec_and_test >- wake_up > > Right? Okay, I see. Let me try to understand in more details. Basically, there'd be no problem if there is no f2fs_stop_checkpoint(), since every {set|clear}_ckpt_flags() are called under f2fs_lock_op() or in fill_super(). And, you're probably concerned about any breakage of ckpt->flags due to accidental f2fs_stop_checkpoint() trigger. So, we're able to lose ERROR_FLAG because of data race. Oh, I found one potential corruption case in f2fs_write_checkpoint(). Before writing the last checkpoint block, we used to check its IO error. But, if set_ckpt_flags() and f2fs_stop_checkpoint() were called concurrently, ckpt_flags was able to lose ERROR_FLAG, resulting in finalizing its checkpoint pack. So, we can get valid checkpoint pack with EIO'ed metadata block. BTW, we can do multiple set/clear flags in do_checkpoint() with single spin_lock call, tho. Thanks, > > > >> Signed-off-by: Chao Yu > >> --- > >> fs/f2fs/checkpoint.c | 24 > >> fs/f2fs/f2fs.h | 29 + > >> fs/f2fs/recovery.c | 2 +- > >> fs/f2fs/segment.c| 4 ++-- > >> fs/f2fs/super.c | 5 +++-- > >> 5 files changed, 39 insertions(+), 25 deletions(-) > >> > >> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > >> index df56a43..0338f8c 100644 > >> --- a/fs/f2fs/checkpoint.c > >> +++ b/fs/f2fs/checkpoint.c > >> @@ -28,7 +28,7 @@ struct kmem_cache *inode_entry_slab; > >> > >> void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi, bool end_io) > >> { > >> - set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG); > >> + set_ckpt_flags(sbi, CP_ERROR_FLAG); > >>sbi->sb->s_flags |= MS_RDONLY; > >>if (!end_io) > >>f2fs_flush_merged_bios(sbi); > >> @@ -571,7 +571,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi) > >>block_t start_blk, orphan_blocks, i, j; > >>int err; > >> > >> - if (!is_set_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG)) > >> + if (!is_set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG)) > >>return 0; > >> > >>start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi); > >> @@ -595,7 +595,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi) > >>f2fs_put_page(page, 1); > >>} > >>/* clear Orphan Flag */ > >> - clear_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG); > >> + clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); > >>return 0; > >> } > >> > >> @@ -1054,9 +1054,9 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, > >> struct cp_control *cpc) > >>/* 2 cp + n data seg summary + orphan inode blocks */ > >>data_sum_blocks = npages_for_summary_flush(sbi, false); > >>if (data_sum_blocks < NR_CURSEG_DATA_TYPE) > >> - set_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG); > >> + set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG); > >>else > >> - clear_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG); > >> + clear_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG); > >> > >>orphan_blocks = GET_ORPHAN_BLOCKS(orphan_num); > >>ckpt->cp_pack_start_sum = cpu_to_le32(1 + cp_payload_blks + > >> @@ -1072,22 +1072,22 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, > >> struct cp_control *cpc) > >>orphan_blocks); > >> > >>if (cpc->reason == CP_UMOUNT) > >> - set_ckpt_flags(ckpt, CP_UMOUNT_FLAG); > >> + set_ckpt_flags(sbi, CP_UMOUNT_FLAG); > >>else > >> - clear_ckpt_flags(ckpt, CP_UMOUNT_FLAG); > >> + clear_ckpt_flags(sbi, CP_UMOUNT_FLAG); > >> > >>if (cpc->reason == CP_FASTBOOT) > >> - set_ckpt_flags(ckpt, CP_FASTBOOT_FLAG); > >> + set_ckpt_flags(sbi, CP_FASTBOOT_FLAG); > >>else > >> - clear_ckpt_flags(ckpt, CP_FASTBOOT_FLAG); > >> + clear_ckpt_flags(sbi, CP_FASTBOOT_FLAG); > >> > >>if (orphan_num) > >> -
Re: [PATCH 4/6] f2fs: introduce cp_lock to protect updating of ckpt_flags
Hi Jaegeuk, On 2016/9/20 5:49, Jaegeuk Kim wrote: > Hi Chao, > > On Sun, Sep 18, 2016 at 11:30:06PM +0800, Chao Yu wrote: >> From: Chao Yu >> >> This patch introduces spinlock to protect updating process of ckpt_flags >> field in struct f2fs_checkpoint, it avoids incorrectly updating in race >> condition. > > So, I'm seeing a race condition between f2fs_stop_checkpoint(), > write_checkpoint(), and f2fs_fill_super(). > > How about just adding f2fs_lock_op() in f2fs_stop_checkpoint()? I'm afraid there will be a potential deadlock here: Thread AThread B - write_checkpoint - block_operations - f2fs_lock_all - do_checkpoint - wait_on_all_pages_writeback - f2fs_write_end_io - f2fs_stop_checkpoint - f2fs_lock_op - end_page_writeback - atomic_dec_and_test - wake_up Right? Thanks, > > Thanks, > >> Signed-off-by: Chao Yu >> --- >> fs/f2fs/checkpoint.c | 24 >> fs/f2fs/f2fs.h | 29 + >> fs/f2fs/recovery.c | 2 +- >> fs/f2fs/segment.c| 4 ++-- >> fs/f2fs/super.c | 5 +++-- >> 5 files changed, 39 insertions(+), 25 deletions(-) >> >> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c >> index df56a43..0338f8c 100644 >> --- a/fs/f2fs/checkpoint.c >> +++ b/fs/f2fs/checkpoint.c >> @@ -28,7 +28,7 @@ struct kmem_cache *inode_entry_slab; >> >> void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi, bool end_io) >> { >> -set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG); >> +set_ckpt_flags(sbi, CP_ERROR_FLAG); >> sbi->sb->s_flags |= MS_RDONLY; >> if (!end_io) >> f2fs_flush_merged_bios(sbi); >> @@ -571,7 +571,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi) >> block_t start_blk, orphan_blocks, i, j; >> int err; >> >> -if (!is_set_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG)) >> +if (!is_set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG)) >> return 0; >> >> start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi); >> @@ -595,7 +595,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi) >> f2fs_put_page(page, 1); >> } >> /* clear Orphan Flag */ >> -clear_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG); >> +clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); >> return 0; >> } >> >> @@ -1054,9 +1054,9 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, >> struct cp_control *cpc) >> /* 2 cp + n data seg summary + orphan inode blocks */ >> data_sum_blocks = npages_for_summary_flush(sbi, false); >> if (data_sum_blocks < NR_CURSEG_DATA_TYPE) >> -set_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG); >> +set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG); >> else >> -clear_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG); >> +clear_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG); >> >> orphan_blocks = GET_ORPHAN_BLOCKS(orphan_num); >> ckpt->cp_pack_start_sum = cpu_to_le32(1 + cp_payload_blks + >> @@ -1072,22 +1072,22 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, >> struct cp_control *cpc) >> orphan_blocks); >> >> if (cpc->reason == CP_UMOUNT) >> -set_ckpt_flags(ckpt, CP_UMOUNT_FLAG); >> +set_ckpt_flags(sbi, CP_UMOUNT_FLAG); >> else >> -clear_ckpt_flags(ckpt, CP_UMOUNT_FLAG); >> +clear_ckpt_flags(sbi, CP_UMOUNT_FLAG); >> >> if (cpc->reason == CP_FASTBOOT) >> -set_ckpt_flags(ckpt, CP_FASTBOOT_FLAG); >> +set_ckpt_flags(sbi, CP_FASTBOOT_FLAG); >> else >> -clear_ckpt_flags(ckpt, CP_FASTBOOT_FLAG); >> +clear_ckpt_flags(sbi, CP_FASTBOOT_FLAG); >> >> if (orphan_num) >> -set_ckpt_flags(ckpt, CP_ORPHAN_PRESENT_FLAG); >> +set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); >> else >> -clear_ckpt_flags(ckpt, CP_ORPHAN_PRESENT_FLAG); >> +clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); >> >> if (is_sbi_flag_set(sbi, SBI_NEED_FSCK)) >> -set_ckpt_flags(ckpt, CP_FSCK_FLAG); >> +set_ckpt_flags(sbi, CP_FSCK_FLAG); >> >> /* update SIT/NAT bitmap */ >> get_sit_bitmap(sbi, __bitmap_ptr(sbi, SIT_BITMAP)); >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >> index c30f744b..b615f3f 100644 >> --- a/fs/f2fs/f2fs.h >> +++ b/fs/f2fs/f2fs.h >> @@ -814,6 +814,7 @@ struct f2fs_sb_info { >> >> /* for checkpoint */ >> struct f2fs_checkpoint *ckpt; /* raw checkpoint pointer */ >> +spinlock_t cp_lock; /* for flag in ckpt */ >> struct inode *meta_inode; /* cache meta blocks */ >> struct mutex cp_mutex; /* checkpoint procedure
Re: [PATCH 4/6] f2fs: introduce cp_lock to protect updating of ckpt_flags
Hi Chao, On Sun, Sep 18, 2016 at 11:30:06PM +0800, Chao Yu wrote: > From: Chao Yu > > This patch introduces spinlock to protect updating process of ckpt_flags > field in struct f2fs_checkpoint, it avoids incorrectly updating in race > condition. So, I'm seeing a race condition between f2fs_stop_checkpoint(), write_checkpoint(), and f2fs_fill_super(). How about just adding f2fs_lock_op() in f2fs_stop_checkpoint()? Thanks, > Signed-off-by: Chao Yu > --- > fs/f2fs/checkpoint.c | 24 > fs/f2fs/f2fs.h | 29 + > fs/f2fs/recovery.c | 2 +- > fs/f2fs/segment.c| 4 ++-- > fs/f2fs/super.c | 5 +++-- > 5 files changed, 39 insertions(+), 25 deletions(-) > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > index df56a43..0338f8c 100644 > --- a/fs/f2fs/checkpoint.c > +++ b/fs/f2fs/checkpoint.c > @@ -28,7 +28,7 @@ struct kmem_cache *inode_entry_slab; > > void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi, bool end_io) > { > - set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG); > + set_ckpt_flags(sbi, CP_ERROR_FLAG); > sbi->sb->s_flags |= MS_RDONLY; > if (!end_io) > f2fs_flush_merged_bios(sbi); > @@ -571,7 +571,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi) > block_t start_blk, orphan_blocks, i, j; > int err; > > - if (!is_set_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG)) > + if (!is_set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG)) > return 0; > > start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi); > @@ -595,7 +595,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi) > f2fs_put_page(page, 1); > } > /* clear Orphan Flag */ > - clear_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG); > + clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); > return 0; > } > > @@ -1054,9 +1054,9 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, > struct cp_control *cpc) > /* 2 cp + n data seg summary + orphan inode blocks */ > data_sum_blocks = npages_for_summary_flush(sbi, false); > if (data_sum_blocks < NR_CURSEG_DATA_TYPE) > - set_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG); > + set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG); > else > - clear_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG); > + clear_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG); > > orphan_blocks = GET_ORPHAN_BLOCKS(orphan_num); > ckpt->cp_pack_start_sum = cpu_to_le32(1 + cp_payload_blks + > @@ -1072,22 +1072,22 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, > struct cp_control *cpc) > orphan_blocks); > > if (cpc->reason == CP_UMOUNT) > - set_ckpt_flags(ckpt, CP_UMOUNT_FLAG); > + set_ckpt_flags(sbi, CP_UMOUNT_FLAG); > else > - clear_ckpt_flags(ckpt, CP_UMOUNT_FLAG); > + clear_ckpt_flags(sbi, CP_UMOUNT_FLAG); > > if (cpc->reason == CP_FASTBOOT) > - set_ckpt_flags(ckpt, CP_FASTBOOT_FLAG); > + set_ckpt_flags(sbi, CP_FASTBOOT_FLAG); > else > - clear_ckpt_flags(ckpt, CP_FASTBOOT_FLAG); > + clear_ckpt_flags(sbi, CP_FASTBOOT_FLAG); > > if (orphan_num) > - set_ckpt_flags(ckpt, CP_ORPHAN_PRESENT_FLAG); > + set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); > else > - clear_ckpt_flags(ckpt, CP_ORPHAN_PRESENT_FLAG); > + clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); > > if (is_sbi_flag_set(sbi, SBI_NEED_FSCK)) > - set_ckpt_flags(ckpt, CP_FSCK_FLAG); > + set_ckpt_flags(sbi, CP_FSCK_FLAG); > > /* update SIT/NAT bitmap */ > get_sit_bitmap(sbi, __bitmap_ptr(sbi, SIT_BITMAP)); > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index c30f744b..b615f3f 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -814,6 +814,7 @@ struct f2fs_sb_info { > > /* for checkpoint */ > struct f2fs_checkpoint *ckpt; /* raw checkpoint pointer */ > + spinlock_t cp_lock; /* for flag in ckpt */ > struct inode *meta_inode; /* cache meta blocks */ > struct mutex cp_mutex; /* checkpoint procedure lock */ > struct rw_semaphore cp_rwsem; /* blocking FS operations */ > @@ -1083,24 +1084,36 @@ static inline unsigned long long > cur_cp_version(struct f2fs_checkpoint *cp) > return le64_to_cpu(cp->checkpoint_ver); > } > > -static inline bool is_set_ckpt_flags(struct f2fs_checkpoint *cp, unsigned > int f) > +static inline bool is_set_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int > f) > { > + struct f2fs_checkpoint *cp = F2FS_CKPT(sbi); > unsigned int ckpt_flags = le32_to_cpu(cp->ckpt_flags); > + > return ckpt_flags & f; > } > > -static inline void set_ckpt_flags(struct f2fs_checkpoint *cp, unsigned int f) > +static inline void set_ckpt_fl
[PATCH 4/6] f2fs: introduce cp_lock to protect updating of ckpt_flags
From: Chao Yu This patch introduces spinlock to protect updating process of ckpt_flags field in struct f2fs_checkpoint, it avoids incorrectly updating in race condition. Signed-off-by: Chao Yu --- fs/f2fs/checkpoint.c | 24 fs/f2fs/f2fs.h | 29 + fs/f2fs/recovery.c | 2 +- fs/f2fs/segment.c| 4 ++-- fs/f2fs/super.c | 5 +++-- 5 files changed, 39 insertions(+), 25 deletions(-) diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c index df56a43..0338f8c 100644 --- a/fs/f2fs/checkpoint.c +++ b/fs/f2fs/checkpoint.c @@ -28,7 +28,7 @@ struct kmem_cache *inode_entry_slab; void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi, bool end_io) { - set_ckpt_flags(sbi->ckpt, CP_ERROR_FLAG); + set_ckpt_flags(sbi, CP_ERROR_FLAG); sbi->sb->s_flags |= MS_RDONLY; if (!end_io) f2fs_flush_merged_bios(sbi); @@ -571,7 +571,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi) block_t start_blk, orphan_blocks, i, j; int err; - if (!is_set_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG)) + if (!is_set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG)) return 0; start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi); @@ -595,7 +595,7 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi) f2fs_put_page(page, 1); } /* clear Orphan Flag */ - clear_ckpt_flags(F2FS_CKPT(sbi), CP_ORPHAN_PRESENT_FLAG); + clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); return 0; } @@ -1054,9 +1054,9 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc) /* 2 cp + n data seg summary + orphan inode blocks */ data_sum_blocks = npages_for_summary_flush(sbi, false); if (data_sum_blocks < NR_CURSEG_DATA_TYPE) - set_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG); + set_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG); else - clear_ckpt_flags(ckpt, CP_COMPACT_SUM_FLAG); + clear_ckpt_flags(sbi, CP_COMPACT_SUM_FLAG); orphan_blocks = GET_ORPHAN_BLOCKS(orphan_num); ckpt->cp_pack_start_sum = cpu_to_le32(1 + cp_payload_blks + @@ -1072,22 +1072,22 @@ static int do_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc) orphan_blocks); if (cpc->reason == CP_UMOUNT) - set_ckpt_flags(ckpt, CP_UMOUNT_FLAG); + set_ckpt_flags(sbi, CP_UMOUNT_FLAG); else - clear_ckpt_flags(ckpt, CP_UMOUNT_FLAG); + clear_ckpt_flags(sbi, CP_UMOUNT_FLAG); if (cpc->reason == CP_FASTBOOT) - set_ckpt_flags(ckpt, CP_FASTBOOT_FLAG); + set_ckpt_flags(sbi, CP_FASTBOOT_FLAG); else - clear_ckpt_flags(ckpt, CP_FASTBOOT_FLAG); + clear_ckpt_flags(sbi, CP_FASTBOOT_FLAG); if (orphan_num) - set_ckpt_flags(ckpt, CP_ORPHAN_PRESENT_FLAG); + set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); else - clear_ckpt_flags(ckpt, CP_ORPHAN_PRESENT_FLAG); + clear_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG); if (is_sbi_flag_set(sbi, SBI_NEED_FSCK)) - set_ckpt_flags(ckpt, CP_FSCK_FLAG); + set_ckpt_flags(sbi, CP_FSCK_FLAG); /* update SIT/NAT bitmap */ get_sit_bitmap(sbi, __bitmap_ptr(sbi, SIT_BITMAP)); diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index c30f744b..b615f3f 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -814,6 +814,7 @@ struct f2fs_sb_info { /* for checkpoint */ struct f2fs_checkpoint *ckpt; /* raw checkpoint pointer */ + spinlock_t cp_lock; /* for flag in ckpt */ struct inode *meta_inode; /* cache meta blocks */ struct mutex cp_mutex; /* checkpoint procedure lock */ struct rw_semaphore cp_rwsem; /* blocking FS operations */ @@ -1083,24 +1084,36 @@ static inline unsigned long long cur_cp_version(struct f2fs_checkpoint *cp) return le64_to_cpu(cp->checkpoint_ver); } -static inline bool is_set_ckpt_flags(struct f2fs_checkpoint *cp, unsigned int f) +static inline bool is_set_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int f) { + struct f2fs_checkpoint *cp = F2FS_CKPT(sbi); unsigned int ckpt_flags = le32_to_cpu(cp->ckpt_flags); + return ckpt_flags & f; } -static inline void set_ckpt_flags(struct f2fs_checkpoint *cp, unsigned int f) +static inline void set_ckpt_flags(struct f2fs_sb_info *sbi, unsigned int f) { - unsigned int ckpt_flags = le32_to_cpu(cp->ckpt_flags); + struct f2fs_checkpoint *cp = F2FS_CKPT(sbi); + unsigned int ckpt_flags; + + spin_lock(&sbi->cp_lock); + ckpt_flags = le32_to_cpu(cp->ckpt_flags); ckpt_flags |= f; cp->ckpt_flags = cpu_to_le32(ckpt_flags); +