Messages in this thread | | | Subject | Re: [f2fs-dev] [PATCH] f2fs: set prefree as free segments after clear prefree segments | From | Chao Yu <> | Date | Fri, 23 Apr 2021 11:37:05 +0800 |
| |
Hi Yangtao,
On 2021/4/23 10:40, 李扬韬 wrote: > HI Chao, > >>> For now, when do_checkpoint fails, the prefree bitmap is not cleared, >>> but these segments are already in the free state. If these segments >>> are used, the segments in use will be reset to the free state when >>> f2fs_clear_prefree_segments is called next time. >>> >>> So move set_prefree_as_free_segments after clear_prefree_segments. >> >> That's not correct. >> >> /* >> * Should call f2fs_clear_prefree_segments after checkpoint is done. >> */ >> static void set_prefree_as_free_segments(struct f2fs_sb_info *sbi) >> >> Comments above set_prefree_as_free_segments() should have told you >> the rule, otherwise if checkpoint failed, valid data in last valid >> checkpoint could be corrupted after segment reuse.
Oh, it seems I misunderstood what the patch did, please ignore above comments.
> > 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++;
I guess for the case of do_checkpoint() fails, maybe we can reset free segment to prefree status.
Thoughts?
Thanks,
> > With this patch, for do_checkpoint fail: > > f2fs_write_checkpoint > ->f2fs_flush_sit_entries > ->do_checkpoint > ->f2fs_release_discard_addrs > > At this time, we did not mark prefree as free segments, so these segments will not be used. > > Thx >
| |