On 2016/10/15 0:57, Jaegeuk Kim wrote: > On Fri, Oct 14, 2016 at 09:25:59PM +0800, Chao Yu wrote: >> On 2016/10/14 1:14, Jaegeuk Kim wrote: >>> On Thu, Oct 13, 2016 at 06:33:19PM +0800, Chao Yu wrote: >>>> Hi Jaegeuk, >>>> >>>> On 2016/10/13 7:23, Jaegeuk Kim wrote: >>>>> This patch fixes using a wrong pointer for sum_page in f2fs_gc. >>>>> >>>>> Signed-off-by: Jaegeuk Kim <jaeg...@kernel.org> >>>>> --- >>>>> fs/f2fs/gc.c | 10 +++++----- >>>>> 1 file changed, 5 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c >>>>> index e48142f..9c18917 100644 >>>>> --- a/fs/f2fs/gc.c >>>>> +++ b/fs/f2fs/gc.c >>>>> @@ -854,16 +854,16 @@ static int do_garbage_collect(struct f2fs_sb_info >>>>> *sbi, >>>>> >>>>> for (segno = start_segno; segno < end_segno; segno++) { >>>>> >>>>> - if (get_valid_blocks(sbi, segno, 1) == 0 || >>>>> - unlikely(f2fs_cp_error(sbi))) >>>>> - goto next; >>>>> - >>>>> /* find segment summary of victim */ >>>>> sum_page = find_get_page(META_MAPPING(sbi), >>>>> GET_SUM_BLOCK(sbi, segno)); >>>>> - f2fs_bug_on(sbi, !PageUptodate(sum_page)); >>>>> f2fs_put_page(sum_page, 0); >>>>> >>>>> + if (get_valid_blocks(sbi, segno, 1) == 0 || >>>>> + !PageUptodate(sum_page) || >>>> >>>> Why uptodate flag of summary page can be cleared? someone truncates it? >>> >>> Well, it looks like no problem to remove this, since it will hit >>> f2fs_cp_error(). I just intended to handle the above f2fs_bug_on here. >> >> If summary page becomes non-uptodate, it seems there is a bug in vfs/mm/f2fs, >> why not keep it there to detect bug? > > I found that the above get_sum_page() loop can give a page which is not > uptodate > caused by EIO. Then, we would get the above bug_on, if we move f2fs_cp_error() > case like this patch. > > Thoughts?
Seems this patch has already been merged into linus' tree... Anyway, I can understand why you remove f2fs_bug_on & add PageUptodate here, but why should we repositon below check? if (get_valid_blocks(sbi, segno, 1) == 0 || unlikely(f2fs_cp_error(sbi))) Thanks, > > Thanks, > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel