Messages in this thread Patch in this message | | | Subject | Re:Re: [f2fs-dev] [PATCH] f2fs: set prefree as free segments after clear prefree segments | From | 李扬韬 <> | Date | Fri, 23 Apr 2021 17:02:15 +0800 (GMT+08:00) |
| |
HI Chao, > >> For do_checkpoint sucess: >> >> f2fs_write_checkpoint >> ->f2fs_flush_sit_entries >> ->set_prefree_as_free_segments >> ->do_checkpoint >> ->f2fs_clear_prefree_segments >> >> >> Calling set_prefree_as_free_segments when do_checkpoint fails, >> seems to be incorrect. I think clear free bitmap should be after >> clear prefree bitmap. >> >> For do_checkpoint fail: >> >> f2fs_write_checkpoint >> ->f2fs_flush_sit_entries >> ->set_prefree_as_free_segments >> ->do_checkpoint >> ->f2fs_release_discard_addrs >> >> The prefree bitmap is not cleared, but free bitmap is cleared,which means >> we can use these segments that are marked as free. When the free segments >> is used, the next f2fs_clear_prefree_segments will mark prefree as free again, >> causing some problem. > >Okay, I can understand that. > >But the problem here is, after applying this patch, successful checkpoint >may record wrong free_segment value: > >- f2fs_write_checkpoint > - f2fs_flush_sit_entries > - do_checkpoint > - ckpt->free_segment_count = cpu_to_le32(free_segments(sbi)); > - f2fs_clear_prefree_segments > - __set_test_and_free > - free_i->free_segments++;
Yeah.
> >I guess for the case of do_checkpoint() fails, maybe we can reset >free segment to prefree status. > >Thoughts? >
Reset free segment to prefree status or separate the number of statistics from the clear bitmap.
How about this one below?
diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c index be5415a0dbbc..0200af4d02ef 100644 --- a/fs/f2fs/checkpoint.c +++ b/fs/f2fs/checkpoint.c @@ -1647,10 +1647,12 @@ int f2fs_write_checkpoint(struct f2fs_sb_info *sbi, struct cp_control *cpc) f2fs_save_inmem_curseg(sbi); err = do_checkpoint(sbi, cpc); - if (err) + if (err) { f2fs_release_discard_addrs(sbi); - else + f2fs_set_free_as_prefree_segments(sbi); + } else { f2fs_clear_prefree_segments(sbi, cpc); + } f2fs_restore_inmem_curseg(sbi); stop: diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index e2d302ae3a46..1618e9a74e89 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -3369,6 +3369,7 @@ bool f2fs_is_checkpointed_data(struct f2fs_sb_info *sbi, block_t blkaddr); void f2fs_drop_discard_cmd(struct f2fs_sb_info *sbi); void f2fs_stop_discard_thread(struct f2fs_sb_info *sbi); bool f2fs_issue_discard_timeout(struct f2fs_sb_info *sbi); +void f2fs_set_free_as_prefree_segments(struct f2fs_sb_info *sbi); void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc); void f2fs_dirty_to_prefree(struct f2fs_sb_info *sbi); diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index c2866561263e..334e499a0f43 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -1959,6 +1959,19 @@ static void set_prefree_as_free_segments(struct f2fs_sb_info *sbi) mutex_unlock(&dirty_i->seglist_lock); } +void f2fs_set_free_as_prefree_segments(struct f2fs_sb_info *sbi) +{ + struct dirty_seglist_info *dirty_i = DIRTY_I(sbi); + unsigned int segno; + + mutex_lock(&dirty_i->seglist_lock); + for_each_set_bit(segno, dirty_i->dirty_segmap[PRE], MAIN_SEGS(sbi)) { + if (__set_test_and_inuse(sbi, segno)) + test_and_clear_bit(segno, dirty_i->dirty_segmap[PRE]); + } + mutex_unlock(&dirty_i->seglist_lock); +} + void f2fs_clear_prefree_segments(struct f2fs_sb_info *sbi, struct cp_control *cpc) { diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h index e9a7a637d688..5da8d1100b87 100644 --- a/fs/f2fs/segment.h +++ b/fs/f2fs/segment.h @@ -487,19 +487,24 @@ static inline void __set_test_and_free(struct f2fs_sb_info *sbi, spin_unlock(&free_i->segmap_lock); } -static inline void __set_test_and_inuse(struct f2fs_sb_info *sbi, +static inline bool __set_test_and_inuse(struct f2fs_sb_info *sbi, unsigned int segno) { struct free_segmap_info *free_i = FREE_I(sbi); unsigned int secno = GET_SEC_FROM_SEG(sbi, segno); + bool is_inuse = false; spin_lock(&free_i->segmap_lock); if (!test_and_set_bit(segno, free_i->free_segmap)) { free_i->free_segments--; if (!test_and_set_bit(secno, free_i->free_secmap)) free_i->free_sections--; + } else { + is_inuse = true; } spin_unlock(&free_i->segmap_lock); + + return is_inuse; } static inline void get_sit_bitmap(struct f2fs_sb_info *sbi, Thx
| |