Re: [PATCH] f2fs: fix to redirty page if fail to gc data page
On Fri, Jun 03, 2016 at 01:59:15PM +0800, Chao Yu wrote: ... > >>> Do we have another expectation? > >> > >> ENOMEM or EIO? > > > > EIO will stop everything. > > ENOMEM would be better to wait for a while from page reclaim? > > Agree, but for ioctl path, IMO, we don't need to let user waiting for ENOMEM > case looping. Well, if user wanted to do a synchronous gc, we need that, IMO.
Re: [PATCH] f2fs: fix to redirty page if fail to gc data page
On Fri, Jun 03, 2016 at 01:59:15PM +0800, Chao Yu wrote: ... > >>> Do we have another expectation? > >> > >> ENOMEM or EIO? > > > > EIO will stop everything. > > ENOMEM would be better to wait for a while from page reclaim? > > Agree, but for ioctl path, IMO, we don't need to let user waiting for ENOMEM > case looping. Well, if user wanted to do a synchronous gc, we need that, IMO.
Re: [PATCH] f2fs: fix to redirty page if fail to gc data page
On 2016/6/3 13:17, Jaegeuk Kim wrote: > On Fri, Jun 03, 2016 at 01:13:21PM +0800, Chao Yu wrote: >> On 2016/6/3 13:08, Jaegeuk Kim wrote: >>> On Tue, May 31, 2016 at 02:10:50PM +0800, Chao Yu wrote: Hi Jaegeuk, On 2016/5/30 10:37, Jaegeuk Kim wrote: > Hi Chao, > > On Sat, May 21, 2016 at 01:19:11PM +0800, Chao Yu wrote: >> From: Chao Yu>> >> If we fail to move data page during foreground GC, we should give another >> chance to writeback that page which was set dirty previously by writer. >> >> Signed-off-by: Chao Yu >> --- >> fs/f2fs/gc.c | 5 - >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c >> index 38d56f6..ee213a8 100644 >> --- a/fs/f2fs/gc.c >> +++ b/fs/f2fs/gc.c >> @@ -653,12 +653,15 @@ static void move_data_page(struct inode *inode, >> block_t bidx, int gc_type) >> .page = page, >> .encrypted_page = NULL, >> }; >> +bool is_dirty = PageDirty(page); >> + >> set_page_dirty(page); >> f2fs_wait_on_page_writeback(page, DATA, true); >> if (clear_page_dirty_for_io(page)) >> inode_dec_dirty_pages(inode); >> set_cold_data(page); >> -do_write_data_page(); >> +if (do_write_data_page() && is_dirty) >> +set_page_dirty(page); > > If this page is truncated with -ENOENT, we don't need to set it dirty > again. Agree > I expect that, if we get an error here, do_garbage_collect() would retry > FG_GC IIRC, you have reworked the FG_GC flows changed from an infinite loop to trying do the movement just one time. Here, I think if there are just few of blocks are failed to be moved, we can give one more time for retrying. How do you think? >>> >>> Mostly I expected here -ENOENT caused by race condition. >> >> If we hit ENOENT case, we can pass get_valid_blocks check, so we don't need >> to >> worry about this case, right? >> >>> Do we have another expectation? >> >> ENOMEM or EIO? > > EIO will stop everything. > ENOMEM would be better to wait for a while from page reclaim? Agree, but for ioctl path, IMO, we don't need to let user waiting for ENOMEM case looping. > >> >> Thanks, >> >>> >>> Thanks, >>> > again. > > Thanks, > >> clear_cold_data(page); >> } >> out: >> -- >> 2.7.2 > . > >>> . >>> > . >
Re: [PATCH] f2fs: fix to redirty page if fail to gc data page
On 2016/6/3 13:17, Jaegeuk Kim wrote: > On Fri, Jun 03, 2016 at 01:13:21PM +0800, Chao Yu wrote: >> On 2016/6/3 13:08, Jaegeuk Kim wrote: >>> On Tue, May 31, 2016 at 02:10:50PM +0800, Chao Yu wrote: Hi Jaegeuk, On 2016/5/30 10:37, Jaegeuk Kim wrote: > Hi Chao, > > On Sat, May 21, 2016 at 01:19:11PM +0800, Chao Yu wrote: >> From: Chao Yu >> >> If we fail to move data page during foreground GC, we should give another >> chance to writeback that page which was set dirty previously by writer. >> >> Signed-off-by: Chao Yu >> --- >> fs/f2fs/gc.c | 5 - >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c >> index 38d56f6..ee213a8 100644 >> --- a/fs/f2fs/gc.c >> +++ b/fs/f2fs/gc.c >> @@ -653,12 +653,15 @@ static void move_data_page(struct inode *inode, >> block_t bidx, int gc_type) >> .page = page, >> .encrypted_page = NULL, >> }; >> +bool is_dirty = PageDirty(page); >> + >> set_page_dirty(page); >> f2fs_wait_on_page_writeback(page, DATA, true); >> if (clear_page_dirty_for_io(page)) >> inode_dec_dirty_pages(inode); >> set_cold_data(page); >> -do_write_data_page(); >> +if (do_write_data_page() && is_dirty) >> +set_page_dirty(page); > > If this page is truncated with -ENOENT, we don't need to set it dirty > again. Agree > I expect that, if we get an error here, do_garbage_collect() would retry > FG_GC IIRC, you have reworked the FG_GC flows changed from an infinite loop to trying do the movement just one time. Here, I think if there are just few of blocks are failed to be moved, we can give one more time for retrying. How do you think? >>> >>> Mostly I expected here -ENOENT caused by race condition. >> >> If we hit ENOENT case, we can pass get_valid_blocks check, so we don't need >> to >> worry about this case, right? >> >>> Do we have another expectation? >> >> ENOMEM or EIO? > > EIO will stop everything. > ENOMEM would be better to wait for a while from page reclaim? Agree, but for ioctl path, IMO, we don't need to let user waiting for ENOMEM case looping. > >> >> Thanks, >> >>> >>> Thanks, >>> > again. > > Thanks, > >> clear_cold_data(page); >> } >> out: >> -- >> 2.7.2 > . > >>> . >>> > . >
Re: [PATCH] f2fs: fix to redirty page if fail to gc data page
On Fri, Jun 03, 2016 at 01:13:21PM +0800, Chao Yu wrote: > On 2016/6/3 13:08, Jaegeuk Kim wrote: > > On Tue, May 31, 2016 at 02:10:50PM +0800, Chao Yu wrote: > >> Hi Jaegeuk, > >> > >> On 2016/5/30 10:37, Jaegeuk Kim wrote: > >>> Hi Chao, > >>> > >>> On Sat, May 21, 2016 at 01:19:11PM +0800, Chao Yu wrote: > From: Chao Yu> > If we fail to move data page during foreground GC, we should give another > chance to writeback that page which was set dirty previously by writer. > > Signed-off-by: Chao Yu > --- > fs/f2fs/gc.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > index 38d56f6..ee213a8 100644 > --- a/fs/f2fs/gc.c > +++ b/fs/f2fs/gc.c > @@ -653,12 +653,15 @@ static void move_data_page(struct inode *inode, > block_t bidx, int gc_type) > .page = page, > .encrypted_page = NULL, > }; > +bool is_dirty = PageDirty(page); > + > set_page_dirty(page); > f2fs_wait_on_page_writeback(page, DATA, true); > if (clear_page_dirty_for_io(page)) > inode_dec_dirty_pages(inode); > set_cold_data(page); > -do_write_data_page(); > +if (do_write_data_page() && is_dirty) > +set_page_dirty(page); > >>> > >>> If this page is truncated with -ENOENT, we don't need to set it dirty > >>> again. > >> > >> Agree > >> > >>> I expect that, if we get an error here, do_garbage_collect() would retry > >>> FG_GC > >> > >> IIRC, you have reworked the FG_GC flows changed from an infinite loop to > >> trying > >> do the movement just one time. Here, I think if there are just few of > >> blocks are > >> failed to be moved, we can give one more time for retrying. How do you > >> think? > > > > Mostly I expected here -ENOENT caused by race condition. > > If we hit ENOENT case, we can pass get_valid_blocks check, so we don't need to > worry about this case, right? > > > Do we have another expectation? > > ENOMEM or EIO? EIO will stop everything. ENOMEM would be better to wait for a while from page reclaim? > > Thanks, > > > > > Thanks, > > > >> > >>> again. > >>> > >>> Thanks, > >>> > clear_cold_data(page); > } > out: > -- > 2.7.2 > >>> . > >>> > > . > >
Re: [PATCH] f2fs: fix to redirty page if fail to gc data page
On Fri, Jun 03, 2016 at 01:13:21PM +0800, Chao Yu wrote: > On 2016/6/3 13:08, Jaegeuk Kim wrote: > > On Tue, May 31, 2016 at 02:10:50PM +0800, Chao Yu wrote: > >> Hi Jaegeuk, > >> > >> On 2016/5/30 10:37, Jaegeuk Kim wrote: > >>> Hi Chao, > >>> > >>> On Sat, May 21, 2016 at 01:19:11PM +0800, Chao Yu wrote: > From: Chao Yu > > If we fail to move data page during foreground GC, we should give another > chance to writeback that page which was set dirty previously by writer. > > Signed-off-by: Chao Yu > --- > fs/f2fs/gc.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > index 38d56f6..ee213a8 100644 > --- a/fs/f2fs/gc.c > +++ b/fs/f2fs/gc.c > @@ -653,12 +653,15 @@ static void move_data_page(struct inode *inode, > block_t bidx, int gc_type) > .page = page, > .encrypted_page = NULL, > }; > +bool is_dirty = PageDirty(page); > + > set_page_dirty(page); > f2fs_wait_on_page_writeback(page, DATA, true); > if (clear_page_dirty_for_io(page)) > inode_dec_dirty_pages(inode); > set_cold_data(page); > -do_write_data_page(); > +if (do_write_data_page() && is_dirty) > +set_page_dirty(page); > >>> > >>> If this page is truncated with -ENOENT, we don't need to set it dirty > >>> again. > >> > >> Agree > >> > >>> I expect that, if we get an error here, do_garbage_collect() would retry > >>> FG_GC > >> > >> IIRC, you have reworked the FG_GC flows changed from an infinite loop to > >> trying > >> do the movement just one time. Here, I think if there are just few of > >> blocks are > >> failed to be moved, we can give one more time for retrying. How do you > >> think? > > > > Mostly I expected here -ENOENT caused by race condition. > > If we hit ENOENT case, we can pass get_valid_blocks check, so we don't need to > worry about this case, right? > > > Do we have another expectation? > > ENOMEM or EIO? EIO will stop everything. ENOMEM would be better to wait for a while from page reclaim? > > Thanks, > > > > > Thanks, > > > >> > >>> again. > >>> > >>> Thanks, > >>> > clear_cold_data(page); > } > out: > -- > 2.7.2 > >>> . > >>> > > . > >
Re: [PATCH] f2fs: fix to redirty page if fail to gc data page
On 2016/6/3 13:08, Jaegeuk Kim wrote: > On Tue, May 31, 2016 at 02:10:50PM +0800, Chao Yu wrote: >> Hi Jaegeuk, >> >> On 2016/5/30 10:37, Jaegeuk Kim wrote: >>> Hi Chao, >>> >>> On Sat, May 21, 2016 at 01:19:11PM +0800, Chao Yu wrote: From: Chao YuIf we fail to move data page during foreground GC, we should give another chance to writeback that page which was set dirty previously by writer. Signed-off-by: Chao Yu --- fs/f2fs/gc.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index 38d56f6..ee213a8 100644 --- a/fs/f2fs/gc.c +++ b/fs/f2fs/gc.c @@ -653,12 +653,15 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type) .page = page, .encrypted_page = NULL, }; + bool is_dirty = PageDirty(page); + set_page_dirty(page); f2fs_wait_on_page_writeback(page, DATA, true); if (clear_page_dirty_for_io(page)) inode_dec_dirty_pages(inode); set_cold_data(page); - do_write_data_page(); + if (do_write_data_page() && is_dirty) + set_page_dirty(page); >>> >>> If this page is truncated with -ENOENT, we don't need to set it dirty again. >> >> Agree >> >>> I expect that, if we get an error here, do_garbage_collect() would retry >>> FG_GC >> >> IIRC, you have reworked the FG_GC flows changed from an infinite loop to >> trying >> do the movement just one time. Here, I think if there are just few of blocks >> are >> failed to be moved, we can give one more time for retrying. How do you think? > > Mostly I expected here -ENOENT caused by race condition. If we hit ENOENT case, we can pass get_valid_blocks check, so we don't need to worry about this case, right? > Do we have another expectation? ENOMEM or EIO? Thanks, > > Thanks, > >> >>> again. >>> >>> Thanks, >>> clear_cold_data(page); } out: -- 2.7.2 >>> . >>> > . >
Re: [PATCH] f2fs: fix to redirty page if fail to gc data page
On 2016/6/3 13:08, Jaegeuk Kim wrote: > On Tue, May 31, 2016 at 02:10:50PM +0800, Chao Yu wrote: >> Hi Jaegeuk, >> >> On 2016/5/30 10:37, Jaegeuk Kim wrote: >>> Hi Chao, >>> >>> On Sat, May 21, 2016 at 01:19:11PM +0800, Chao Yu wrote: From: Chao Yu If we fail to move data page during foreground GC, we should give another chance to writeback that page which was set dirty previously by writer. Signed-off-by: Chao Yu --- fs/f2fs/gc.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index 38d56f6..ee213a8 100644 --- a/fs/f2fs/gc.c +++ b/fs/f2fs/gc.c @@ -653,12 +653,15 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type) .page = page, .encrypted_page = NULL, }; + bool is_dirty = PageDirty(page); + set_page_dirty(page); f2fs_wait_on_page_writeback(page, DATA, true); if (clear_page_dirty_for_io(page)) inode_dec_dirty_pages(inode); set_cold_data(page); - do_write_data_page(); + if (do_write_data_page() && is_dirty) + set_page_dirty(page); >>> >>> If this page is truncated with -ENOENT, we don't need to set it dirty again. >> >> Agree >> >>> I expect that, if we get an error here, do_garbage_collect() would retry >>> FG_GC >> >> IIRC, you have reworked the FG_GC flows changed from an infinite loop to >> trying >> do the movement just one time. Here, I think if there are just few of blocks >> are >> failed to be moved, we can give one more time for retrying. How do you think? > > Mostly I expected here -ENOENT caused by race condition. If we hit ENOENT case, we can pass get_valid_blocks check, so we don't need to worry about this case, right? > Do we have another expectation? ENOMEM or EIO? Thanks, > > Thanks, > >> >>> again. >>> >>> Thanks, >>> clear_cold_data(page); } out: -- 2.7.2 >>> . >>> > . >
Re: [PATCH] f2fs: fix to redirty page if fail to gc data page
On Tue, May 31, 2016 at 02:10:50PM +0800, Chao Yu wrote: > Hi Jaegeuk, > > On 2016/5/30 10:37, Jaegeuk Kim wrote: > > Hi Chao, > > > > On Sat, May 21, 2016 at 01:19:11PM +0800, Chao Yu wrote: > >> From: Chao Yu> >> > >> If we fail to move data page during foreground GC, we should give another > >> chance to writeback that page which was set dirty previously by writer. > >> > >> Signed-off-by: Chao Yu > >> --- > >> fs/f2fs/gc.c | 5 - > >> 1 file changed, 4 insertions(+), 1 deletion(-) > >> > >> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > >> index 38d56f6..ee213a8 100644 > >> --- a/fs/f2fs/gc.c > >> +++ b/fs/f2fs/gc.c > >> @@ -653,12 +653,15 @@ static void move_data_page(struct inode *inode, > >> block_t bidx, int gc_type) > >>.page = page, > >>.encrypted_page = NULL, > >>}; > >> + bool is_dirty = PageDirty(page); > >> + > >>set_page_dirty(page); > >>f2fs_wait_on_page_writeback(page, DATA, true); > >>if (clear_page_dirty_for_io(page)) > >>inode_dec_dirty_pages(inode); > >>set_cold_data(page); > >> - do_write_data_page(); > >> + if (do_write_data_page() && is_dirty) > >> + set_page_dirty(page); > > > > If this page is truncated with -ENOENT, we don't need to set it dirty again. > > Agree > > > I expect that, if we get an error here, do_garbage_collect() would retry > > FG_GC > > IIRC, you have reworked the FG_GC flows changed from an infinite loop to > trying > do the movement just one time. Here, I think if there are just few of blocks > are > failed to be moved, we can give one more time for retrying. How do you think? Mostly I expected here -ENOENT caused by race condition. Do we have another expectation? Thanks, > > > again. > > > > Thanks, > > > >>clear_cold_data(page); > >>} > >> out: > >> -- > >> 2.7.2 > > . > >
Re: [PATCH] f2fs: fix to redirty page if fail to gc data page
On Tue, May 31, 2016 at 02:10:50PM +0800, Chao Yu wrote: > Hi Jaegeuk, > > On 2016/5/30 10:37, Jaegeuk Kim wrote: > > Hi Chao, > > > > On Sat, May 21, 2016 at 01:19:11PM +0800, Chao Yu wrote: > >> From: Chao Yu > >> > >> If we fail to move data page during foreground GC, we should give another > >> chance to writeback that page which was set dirty previously by writer. > >> > >> Signed-off-by: Chao Yu > >> --- > >> fs/f2fs/gc.c | 5 - > >> 1 file changed, 4 insertions(+), 1 deletion(-) > >> > >> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > >> index 38d56f6..ee213a8 100644 > >> --- a/fs/f2fs/gc.c > >> +++ b/fs/f2fs/gc.c > >> @@ -653,12 +653,15 @@ static void move_data_page(struct inode *inode, > >> block_t bidx, int gc_type) > >>.page = page, > >>.encrypted_page = NULL, > >>}; > >> + bool is_dirty = PageDirty(page); > >> + > >>set_page_dirty(page); > >>f2fs_wait_on_page_writeback(page, DATA, true); > >>if (clear_page_dirty_for_io(page)) > >>inode_dec_dirty_pages(inode); > >>set_cold_data(page); > >> - do_write_data_page(); > >> + if (do_write_data_page() && is_dirty) > >> + set_page_dirty(page); > > > > If this page is truncated with -ENOENT, we don't need to set it dirty again. > > Agree > > > I expect that, if we get an error here, do_garbage_collect() would retry > > FG_GC > > IIRC, you have reworked the FG_GC flows changed from an infinite loop to > trying > do the movement just one time. Here, I think if there are just few of blocks > are > failed to be moved, we can give one more time for retrying. How do you think? Mostly I expected here -ENOENT caused by race condition. Do we have another expectation? Thanks, > > > again. > > > > Thanks, > > > >>clear_cold_data(page); > >>} > >> out: > >> -- > >> 2.7.2 > > . > >
Re: [PATCH] f2fs: fix to redirty page if fail to gc data page
Hi Jaegeuk, On 2016/5/30 10:37, Jaegeuk Kim wrote: > Hi Chao, > > On Sat, May 21, 2016 at 01:19:11PM +0800, Chao Yu wrote: >> From: Chao Yu>> >> If we fail to move data page during foreground GC, we should give another >> chance to writeback that page which was set dirty previously by writer. >> >> Signed-off-by: Chao Yu >> --- >> fs/f2fs/gc.c | 5 - >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c >> index 38d56f6..ee213a8 100644 >> --- a/fs/f2fs/gc.c >> +++ b/fs/f2fs/gc.c >> @@ -653,12 +653,15 @@ static void move_data_page(struct inode *inode, >> block_t bidx, int gc_type) >> .page = page, >> .encrypted_page = NULL, >> }; >> +bool is_dirty = PageDirty(page); >> + >> set_page_dirty(page); >> f2fs_wait_on_page_writeback(page, DATA, true); >> if (clear_page_dirty_for_io(page)) >> inode_dec_dirty_pages(inode); >> set_cold_data(page); >> -do_write_data_page(); >> +if (do_write_data_page() && is_dirty) >> +set_page_dirty(page); > > If this page is truncated with -ENOENT, we don't need to set it dirty again. Agree > I expect that, if we get an error here, do_garbage_collect() would retry FG_GC IIRC, you have reworked the FG_GC flows changed from an infinite loop to trying do the movement just one time. Here, I think if there are just few of blocks are failed to be moved, we can give one more time for retrying. How do you think? > again. > > Thanks, > >> clear_cold_data(page); >> } >> out: >> -- >> 2.7.2 > . >
Re: [PATCH] f2fs: fix to redirty page if fail to gc data page
Hi Jaegeuk, On 2016/5/30 10:37, Jaegeuk Kim wrote: > Hi Chao, > > On Sat, May 21, 2016 at 01:19:11PM +0800, Chao Yu wrote: >> From: Chao Yu >> >> If we fail to move data page during foreground GC, we should give another >> chance to writeback that page which was set dirty previously by writer. >> >> Signed-off-by: Chao Yu >> --- >> fs/f2fs/gc.c | 5 - >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c >> index 38d56f6..ee213a8 100644 >> --- a/fs/f2fs/gc.c >> +++ b/fs/f2fs/gc.c >> @@ -653,12 +653,15 @@ static void move_data_page(struct inode *inode, >> block_t bidx, int gc_type) >> .page = page, >> .encrypted_page = NULL, >> }; >> +bool is_dirty = PageDirty(page); >> + >> set_page_dirty(page); >> f2fs_wait_on_page_writeback(page, DATA, true); >> if (clear_page_dirty_for_io(page)) >> inode_dec_dirty_pages(inode); >> set_cold_data(page); >> -do_write_data_page(); >> +if (do_write_data_page() && is_dirty) >> +set_page_dirty(page); > > If this page is truncated with -ENOENT, we don't need to set it dirty again. Agree > I expect that, if we get an error here, do_garbage_collect() would retry FG_GC IIRC, you have reworked the FG_GC flows changed from an infinite loop to trying do the movement just one time. Here, I think if there are just few of blocks are failed to be moved, we can give one more time for retrying. How do you think? > again. > > Thanks, > >> clear_cold_data(page); >> } >> out: >> -- >> 2.7.2 > . >
Re: [PATCH] f2fs: fix to redirty page if fail to gc data page
Hi Chao, On Sat, May 21, 2016 at 01:19:11PM +0800, Chao Yu wrote: > From: Chao Yu> > If we fail to move data page during foreground GC, we should give another > chance to writeback that page which was set dirty previously by writer. > > Signed-off-by: Chao Yu > --- > fs/f2fs/gc.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > index 38d56f6..ee213a8 100644 > --- a/fs/f2fs/gc.c > +++ b/fs/f2fs/gc.c > @@ -653,12 +653,15 @@ static void move_data_page(struct inode *inode, block_t > bidx, int gc_type) > .page = page, > .encrypted_page = NULL, > }; > + bool is_dirty = PageDirty(page); > + > set_page_dirty(page); > f2fs_wait_on_page_writeback(page, DATA, true); > if (clear_page_dirty_for_io(page)) > inode_dec_dirty_pages(inode); > set_cold_data(page); > - do_write_data_page(); > + if (do_write_data_page() && is_dirty) > + set_page_dirty(page); If this page is truncated with -ENOENT, we don't need to set it dirty again. I expect that, if we get an error here, do_garbage_collect() would retry FG_GC again. Thanks, > clear_cold_data(page); > } > out: > -- > 2.7.2
Re: [PATCH] f2fs: fix to redirty page if fail to gc data page
Hi Chao, On Sat, May 21, 2016 at 01:19:11PM +0800, Chao Yu wrote: > From: Chao Yu > > If we fail to move data page during foreground GC, we should give another > chance to writeback that page which was set dirty previously by writer. > > Signed-off-by: Chao Yu > --- > fs/f2fs/gc.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > index 38d56f6..ee213a8 100644 > --- a/fs/f2fs/gc.c > +++ b/fs/f2fs/gc.c > @@ -653,12 +653,15 @@ static void move_data_page(struct inode *inode, block_t > bidx, int gc_type) > .page = page, > .encrypted_page = NULL, > }; > + bool is_dirty = PageDirty(page); > + > set_page_dirty(page); > f2fs_wait_on_page_writeback(page, DATA, true); > if (clear_page_dirty_for_io(page)) > inode_dec_dirty_pages(inode); > set_cold_data(page); > - do_write_data_page(); > + if (do_write_data_page() && is_dirty) > + set_page_dirty(page); If this page is truncated with -ENOENT, we don't need to set it dirty again. I expect that, if we get an error here, do_garbage_collect() would retry FG_GC again. Thanks, > clear_cold_data(page); > } > out: > -- > 2.7.2
Re: [PATCH] f2fs: fix to redirty page if fail to gc data page
Ping, On 2016/5/21 13:19, Chao Yu wrote: > From: Chao Yu> > If we fail to move data page during foreground GC, we should give another > chance to writeback that page which was set dirty previously by writer. > > Signed-off-by: Chao Yu > --- > fs/f2fs/gc.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > index 38d56f6..ee213a8 100644 > --- a/fs/f2fs/gc.c > +++ b/fs/f2fs/gc.c > @@ -653,12 +653,15 @@ static void move_data_page(struct inode *inode, block_t > bidx, int gc_type) > .page = page, > .encrypted_page = NULL, > }; > + bool is_dirty = PageDirty(page); > + > set_page_dirty(page); > f2fs_wait_on_page_writeback(page, DATA, true); > if (clear_page_dirty_for_io(page)) > inode_dec_dirty_pages(inode); > set_cold_data(page); > - do_write_data_page(); > + if (do_write_data_page() && is_dirty) > + set_page_dirty(page); > clear_cold_data(page); > } > out: >
Re: [PATCH] f2fs: fix to redirty page if fail to gc data page
Ping, On 2016/5/21 13:19, Chao Yu wrote: > From: Chao Yu > > If we fail to move data page during foreground GC, we should give another > chance to writeback that page which was set dirty previously by writer. > > Signed-off-by: Chao Yu > --- > fs/f2fs/gc.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c > index 38d56f6..ee213a8 100644 > --- a/fs/f2fs/gc.c > +++ b/fs/f2fs/gc.c > @@ -653,12 +653,15 @@ static void move_data_page(struct inode *inode, block_t > bidx, int gc_type) > .page = page, > .encrypted_page = NULL, > }; > + bool is_dirty = PageDirty(page); > + > set_page_dirty(page); > f2fs_wait_on_page_writeback(page, DATA, true); > if (clear_page_dirty_for_io(page)) > inode_dec_dirty_pages(inode); > set_cold_data(page); > - do_write_data_page(); > + if (do_write_data_page() && is_dirty) > + set_page_dirty(page); > clear_cold_data(page); > } > out: >