On 10/17, Chao Yu wrote: > On 2018/10/16 11:10, Jaegeuk Kim wrote: > > On 10/16, Chao Yu wrote: > >> On 2018/10/16 7:08, Jaegeuk Kim wrote: > >>> On 10/15, Chao Yu wrote: > >>>> On 2018/10/11 5:22, Jaegeuk Kim wrote: > >>>>> If we clear the cold data flag out of the writeback flow, we can > >>>>> miscount > >>>>> -1 by end_io. > >>>> > >>>> I didn't get it, which count do you mean? > >>> > >>> It's the number of dirty pages. > >>> > >>> Balancing F2FS Async: > >>> - IO (CP: 1, Data: -1, Flush: ( 0 0 1), Discard: ( 0 > >>> 129304)) cmd: 0 undiscard: 0 > > Better to add such info in commit message. :) > > >>> > >> > >> So I guess the race should be: > >> > >> GC thread: IRQ > >> - move_data_page() > >> - set_page_dirty() > >> - clear_cold_data() > >> - f2fs_write_end_io() > >> - type = WB_DATA_TYPE(page); > >> here, we get wrong type > >> - dec_page_count(sbi, type); > >> - f2fs_wait_on_page_writeback() > >> > >> How about relocate wait_writeback & set_page_dirty to avoid above race: > > > > Well, wait_on_stable_page() doesn't guarantee its end_io. So, I'm not sure > > this is the only case. > > Yes, you're right, I missed that case. > > Can you use git-revert to generate the patch? so we can remain original > commit info for better backward tracking. > > How do you think pick up original patch I submitted: > > https://lkml.org/lkml/2018/7/27/415
Seems that works. Thanks, > > Thanks, > > > > >> > >> move_data_page() > >> > >> retry: > >> f2fs_wait_on_page_writeback(page, DATA, true); > >> set_page_dirty(page); > >> > >> Thanks, > >> > >>>> > >>>> Thanks, > >>>> > >>>>> > >>>>> Signed-off-by: Jaegeuk Kim <jaeg...@kernel.org> > >>>>> --- > >>>>> fs/f2fs/data.c | 4 ---- > >>>>> 1 file changed, 4 deletions(-) > >>>>> > >>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > >>>>> index 29a9d3b8f709..4102799b5558 100644 > >>>>> --- a/fs/f2fs/data.c > >>>>> +++ b/fs/f2fs/data.c > >>>>> @@ -2636,10 +2636,6 @@ static int f2fs_set_data_page_dirty(struct page > >>>>> *page) > >>>>> if (!PageUptodate(page)) > >>>>> SetPageUptodate(page); > >>>>> > >>>>> - /* don't remain PG_checked flag which was set during GC */ > >>>>> - if (is_cold_data(page)) > >>>>> - clear_cold_data(page); > >>>>> - > >>>>> if (f2fs_is_atomic_file(inode) && > >>>>> !f2fs_is_commit_atomic_write(inode)) { > >>>>> if (!IS_ATOMIC_WRITTEN_PAGE(page)) { > >>>>> f2fs_register_inmem_page(inode, page); > >>>>> > >>> > >>> . > >>> > > > > . > > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel