* [PATCH] f2fs: return directly if block has been removed from the victim
@ 2016-10-24 3:37 Yunlei He
2016-10-26 1:09 ` Jaegeuk Kim
0 siblings, 1 reply; 6+ messages in thread
From: Yunlei He @ 2016-10-24 3:37 UTC (permalink / raw)
To: linux-f2fs-devel, jaegeuk, yuchao0; +Cc: heyunlei
If one block has been to written to a new place, just return
in move data process.
Signed-off-by: Yunlei He <heyunlei@huawei.com>
---
fs/f2fs/gc.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index f35fca5..fc78f3e 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -545,7 +545,8 @@ static bool is_alive(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
return true;
}
-static void move_encrypted_block(struct inode *inode, block_t bidx)
+static void move_encrypted_block(struct inode *inode, block_t bidx,
+ unsigned int segno, int off)
{
struct f2fs_io_info fio = {
.sbi = F2FS_I_SB(inode),
@@ -580,6 +581,9 @@ static void move_encrypted_block(struct inode *inode, block_t bidx)
* don't cache encrypted data into meta inode until previous dirty
* data were writebacked to avoid racing between GC and flush.
*/
+ if (!check_valid_map(F2FS_I_SB(inode), segno, off))
+ goto out;
+
f2fs_wait_on_page_writeback(page, DATA, true);
get_node_info(fio.sbi, dn.nid, &ni);
@@ -646,7 +650,8 @@ static void move_encrypted_block(struct inode *inode, block_t bidx)
f2fs_put_page(page, 1);
}
-static void move_data_page(struct inode *inode, block_t bidx, int gc_type)
+static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
+ unsigned int segno, int off)
{
struct page *page;
@@ -655,8 +660,10 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type)
return;
if (gc_type == BG_GC) {
- if (PageWriteback(page))
+ if (!check_valid_map(F2FS_I_SB(inode), segno, off))
goto out;
+
+ f2fs_wait_on_page_writeback(page, DATA, false);
set_page_dirty(page);
set_cold_data(page);
} else {
@@ -797,9 +804,9 @@ static void gc_data_segment(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
start_bidx = start_bidx_of_node(nofs, inode)
+ ofs_in_node;
if (f2fs_encrypted_inode(inode) && S_ISREG(inode->i_mode))
- move_encrypted_block(inode, start_bidx);
+ move_encrypted_block(inode, start_bidx, segno, off);
else
- move_data_page(inode, start_bidx, gc_type);
+ move_data_page(inode, start_bidx, gc_type, segno, off);
if (locked) {
up_write(&fi->dio_rwsem[WRITE]);
--
2.10.1
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] f2fs: return directly if block has been removed from the victim
2016-10-24 3:37 [PATCH] f2fs: return directly if block has been removed from the victim Yunlei He
@ 2016-10-26 1:09 ` Jaegeuk Kim
2016-10-26 2:14 ` heyunlei
0 siblings, 1 reply; 6+ messages in thread
From: Jaegeuk Kim @ 2016-10-26 1:09 UTC (permalink / raw)
To: Yunlei He; +Cc: heyunlei, linux-f2fs-devel
Hi Yunlei,
On Mon, Oct 24, 2016 at 11:37:28AM +0800, Yunlei He wrote:
> If one block has been to written to a new place, just return
> in move data process.
>
> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> ---
> fs/f2fs/gc.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index f35fca5..fc78f3e 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -545,7 +545,8 @@ static bool is_alive(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
> return true;
> }
>
> -static void move_encrypted_block(struct inode *inode, block_t bidx)
> +static void move_encrypted_block(struct inode *inode, block_t bidx,
> + unsigned int segno, int off)
> {
> struct f2fs_io_info fio = {
> .sbi = F2FS_I_SB(inode),
> @@ -580,6 +581,9 @@ static void move_encrypted_block(struct inode *inode, block_t bidx)
> * don't cache encrypted data into meta inode until previous dirty
> * data were writebacked to avoid racing between GC and flush.
> */
> + if (!check_valid_map(F2FS_I_SB(inode), segno, off))
> + goto out;
This is done by gc_data_segment() before calling move function.
Why do we need this again?
> +
> f2fs_wait_on_page_writeback(page, DATA, true);
>
> get_node_info(fio.sbi, dn.nid, &ni);
> @@ -646,7 +650,8 @@ static void move_encrypted_block(struct inode *inode, block_t bidx)
> f2fs_put_page(page, 1);
> }
>
> -static void move_data_page(struct inode *inode, block_t bidx, int gc_type)
> +static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
> + unsigned int segno, int off)
> {
> struct page *page;
>
> @@ -655,8 +660,10 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type)
> return;
>
> if (gc_type == BG_GC) {
> - if (PageWriteback(page))
Seems there is no reason to remove the existing writeback case.
This is a BG_GC case and we don't need to proceed GC in this case.
> + if (!check_valid_map(F2FS_I_SB(inode), segno, off))
> goto out;
> +
> + f2fs_wait_on_page_writeback(page, DATA, false);
> set_page_dirty(page);
> set_cold_data(page);
> } else {
> @@ -797,9 +804,9 @@ static void gc_data_segment(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
> start_bidx = start_bidx_of_node(nofs, inode)
> + ofs_in_node;
> if (f2fs_encrypted_inode(inode) && S_ISREG(inode->i_mode))
> - move_encrypted_block(inode, start_bidx);
> + move_encrypted_block(inode, start_bidx, segno, off);
> else
> - move_data_page(inode, start_bidx, gc_type);
> + move_data_page(inode, start_bidx, gc_type, segno, off);
>
> if (locked) {
> up_write(&fi->dio_rwsem[WRITE]);
> --
> 2.10.1
>
>
> ------------------------------------------------------------------------------
> 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
------------------------------------------------------------------------------
The Command Line: Reinvented for Modern Developers
Did the resurgence of CLI tooling catch you by surprise?
Reconnect with the command line and become more productive.
Learn the new .NET and ASP.NET CLI. Get your free copy!
http://sdm.link/telerik
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] f2fs: return directly if block has been removed from the victim
2016-10-26 1:09 ` Jaegeuk Kim
@ 2016-10-26 2:14 ` heyunlei
2016-10-27 22:34 ` Jaegeuk Kim
0 siblings, 1 reply; 6+ messages in thread
From: heyunlei @ 2016-10-26 2:14 UTC (permalink / raw)
To: Jaegeuk Kim; +Cc: heyunlei, linux-f2fs-devel
On 2016/10/26 9:09, Jaegeuk Kim wrote:
Hi Kim,
> Hi Yunlei,
>
> On Mon, Oct 24, 2016 at 11:37:28AM +0800, Yunlei He wrote:
>> If one block has been to written to a new place, just return
>> in move data process.
>>
>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>> ---
>> fs/f2fs/gc.c | 17 ++++++++++++-----
>> 1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>> index f35fca5..fc78f3e 100644
>> --- a/fs/f2fs/gc.c
>> +++ b/fs/f2fs/gc.c
>> @@ -545,7 +545,8 @@ static bool is_alive(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
>> return true;
>> }
>>
>> -static void move_encrypted_block(struct inode *inode, block_t bidx)
>> +static void move_encrypted_block(struct inode *inode, block_t bidx,
>> + unsigned int segno, int off)
>> {
>> struct f2fs_io_info fio = {
>> .sbi = F2FS_I_SB(inode),
>> @@ -580,6 +581,9 @@ static void move_encrypted_block(struct inode *inode, block_t bidx)
>> * don't cache encrypted data into meta inode until previous dirty
>> * data were writebacked to avoid racing between GC and flush.
>> */
>> + if (!check_valid_map(F2FS_I_SB(inode), segno, off))
>> + goto out;
>
> This is done by gc_data_segment() before calling move function.
> Why do we need this again?
gc_data_segment() check this without lock the data page. If this block
is written to a new place between check in gc_data_segment() and locked
the page, it will need to wait this page write back, and write it again,
it's unnecessary.
>
>> +
>> f2fs_wait_on_page_writeback(page, DATA, true);
>>
>> get_node_info(fio.sbi, dn.nid, &ni);
>> @@ -646,7 +650,8 @@ static void move_encrypted_block(struct inode *inode, block_t bidx)
>> f2fs_put_page(page, 1);
>> }
>>
>> -static void move_data_page(struct inode *inode, block_t bidx, int gc_type)
>> +static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
>> + unsigned int segno, int off)
>> {
>> struct page *page;
>>
>> @@ -655,8 +660,10 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type)
>> return;
>>
>> if (gc_type == BG_GC) {
>> - if (PageWriteback(page))
>
> Seems there is no reason to remove the existing writeback case.
> This is a BG_GC case and we don't need to proceed GC in this case.
Here, PageWriteback(page) means two cases:
1. write to this victim segment
2. write to a new place
In the first case, if we return directly, this victim bg_gc will fail
for some blocks are still valid.
Thanks.
>
>> + if (!check_valid_map(F2FS_I_SB(inode), segno, off))
>> goto out;
>> +
>> + f2fs_wait_on_page_writeback(page, DATA, false);
>> set_page_dirty(page);
>> set_cold_data(page);
>> } else {
>> @@ -797,9 +804,9 @@ static void gc_data_segment(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
>> start_bidx = start_bidx_of_node(nofs, inode)
>> + ofs_in_node;
>> if (f2fs_encrypted_inode(inode) && S_ISREG(inode->i_mode))
>> - move_encrypted_block(inode, start_bidx);
>> + move_encrypted_block(inode, start_bidx, segno, off);
>> else
>> - move_data_page(inode, start_bidx, gc_type);
>> + move_data_page(inode, start_bidx, gc_type, segno, off);
>>
>> if (locked) {
>> up_write(&fi->dio_rwsem[WRITE]);
>> --
>> 2.10.1
>>
>>
>> ------------------------------------------------------------------------------
>> 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
>
> .
>
------------------------------------------------------------------------------
The Command Line: Reinvented for Modern Developers
Did the resurgence of CLI tooling catch you by surprise?
Reconnect with the command line and become more productive.
Learn the new .NET and ASP.NET CLI. Get your free copy!
http://sdm.link/telerik
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] f2fs: return directly if block has been removed from the victim
2016-10-26 2:14 ` heyunlei
@ 2016-10-27 22:34 ` Jaegeuk Kim
2016-11-02 7:19 ` Chao Yu
0 siblings, 1 reply; 6+ messages in thread
From: Jaegeuk Kim @ 2016-10-27 22:34 UTC (permalink / raw)
To: heyunlei; +Cc: heyunlei, linux-f2fs-devel
On Wed, Oct 26, 2016 at 10:14:04AM +0800, heyunlei wrote:
>
>
> On 2016/10/26 9:09, Jaegeuk Kim wrote:
> Hi Kim,
> > Hi Yunlei,
> >
> > On Mon, Oct 24, 2016 at 11:37:28AM +0800, Yunlei He wrote:
> >> If one block has been to written to a new place, just return
> >> in move data process.
> >>
> >> Signed-off-by: Yunlei He <heyunlei@huawei.com>
> >> ---
> >> fs/f2fs/gc.c | 17 ++++++++++++-----
> >> 1 file changed, 12 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> >> index f35fca5..fc78f3e 100644
> >> --- a/fs/f2fs/gc.c
> >> +++ b/fs/f2fs/gc.c
> >> @@ -545,7 +545,8 @@ static bool is_alive(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
> >> return true;
> >> }
> >>
> >> -static void move_encrypted_block(struct inode *inode, block_t bidx)
> >> +static void move_encrypted_block(struct inode *inode, block_t bidx,
> >> + unsigned int segno, int off)
> >> {
> >> struct f2fs_io_info fio = {
> >> .sbi = F2FS_I_SB(inode),
> >> @@ -580,6 +581,9 @@ static void move_encrypted_block(struct inode *inode, block_t bidx)
> >> * don't cache encrypted data into meta inode until previous dirty
> >> * data were writebacked to avoid racing between GC and flush.
> >> */
> >> + if (!check_valid_map(F2FS_I_SB(inode), segno, off))
> >> + goto out;
> >
> > This is done by gc_data_segment() before calling move function.
> > Why do we need this again?
>
> gc_data_segment() check this without lock the data page. If this block
> is written to a new place between check in gc_data_segment() and locked
> the page, it will need to wait this page write back, and write it again,
> it's unnecessary.
>
> >
> >> +
> >> f2fs_wait_on_page_writeback(page, DATA, true);
> >>
> >> get_node_info(fio.sbi, dn.nid, &ni);
> >> @@ -646,7 +650,8 @@ static void move_encrypted_block(struct inode *inode, block_t bidx)
> >> f2fs_put_page(page, 1);
> >> }
> >>
> >> -static void move_data_page(struct inode *inode, block_t bidx, int gc_type)
> >> +static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
> >> + unsigned int segno, int off)
> >> {
> >> struct page *page;
> >>
> >> @@ -655,8 +660,10 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type)
> >> return;
> >>
> >> if (gc_type == BG_GC) {
> >> - if (PageWriteback(page))
> >
> > Seems there is no reason to remove the existing writeback case.
> > This is a BG_GC case and we don't need to proceed GC in this case.
> Here, PageWriteback(page) means two cases:
> 1. write to this victim segment
> 2. write to a new place
>
> In the first case, if we return directly, this victim bg_gc will fail
> for some blocks are still valid.
It should be fine, and its main reason would be to avoid wait_on_page_writeback.
Next bg_gc will do better.
Thanks,
------------------------------------------------------------------------------
The Command Line: Reinvented for Modern Developers
Did the resurgence of CLI tooling catch you by surprise?
Reconnect with the command line and become more productive.
Learn the new .NET and ASP.NET CLI. Get your free copy!
http://sdm.link/telerik
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] f2fs: return directly if block has been removed from the victim
2016-10-27 22:34 ` Jaegeuk Kim
@ 2016-11-02 7:19 ` Chao Yu
2016-11-02 10:40 ` heyunlei
0 siblings, 1 reply; 6+ messages in thread
From: Chao Yu @ 2016-11-02 7:19 UTC (permalink / raw)
To: Jaegeuk Kim, heyunlei; +Cc: linux-f2fs-devel
On 2016/10/28 6:34, Jaegeuk Kim wrote:
> On Wed, Oct 26, 2016 at 10:14:04AM +0800, heyunlei wrote:
>>
>>
>> On 2016/10/26 9:09, Jaegeuk Kim wrote:
>> Hi Kim,
>>> Hi Yunlei,
>>>
>>> On Mon, Oct 24, 2016 at 11:37:28AM +0800, Yunlei He wrote:
>>>> If one block has been to written to a new place, just return
>>>> in move data process.
>>>>
>>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>>> ---
>>>> fs/f2fs/gc.c | 17 ++++++++++++-----
>>>> 1 file changed, 12 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>>> index f35fca5..fc78f3e 100644
>>>> --- a/fs/f2fs/gc.c
>>>> +++ b/fs/f2fs/gc.c
>>>> @@ -545,7 +545,8 @@ static bool is_alive(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
>>>> return true;
>>>> }
>>>>
>>>> -static void move_encrypted_block(struct inode *inode, block_t bidx)
>>>> +static void move_encrypted_block(struct inode *inode, block_t bidx,
>>>> + unsigned int segno, int off)
>>>> {
>>>> struct f2fs_io_info fio = {
>>>> .sbi = F2FS_I_SB(inode),
>>>> @@ -580,6 +581,9 @@ static void move_encrypted_block(struct inode *inode, block_t bidx)
>>>> * don't cache encrypted data into meta inode until previous dirty
>>>> * data were writebacked to avoid racing between GC and flush.
>>>> */
>>>> + if (!check_valid_map(F2FS_I_SB(inode), segno, off))
>>>> + goto out;
>>>
>>> This is done by gc_data_segment() before calling move function.
>>> Why do we need this again?
>>
>> gc_data_segment() check this without lock the data page. If this block
>> is written to a new place between check in gc_data_segment() and locked
>> the page, it will need to wait this page write back, and write it again,
>> it's unnecessary.
>>
>>>
>>>> +
>>>> f2fs_wait_on_page_writeback(page, DATA, true);
>>>>
>>>> get_node_info(fio.sbi, dn.nid, &ni);
>>>> @@ -646,7 +650,8 @@ static void move_encrypted_block(struct inode *inode, block_t bidx)
>>>> f2fs_put_page(page, 1);
>>>> }
>>>>
>>>> -static void move_data_page(struct inode *inode, block_t bidx, int gc_type)
>>>> +static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
>>>> + unsigned int segno, int off)
>>>> {
>>>> struct page *page;
>>>>
>>>> @@ -655,8 +660,10 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type)
>>>> return;
>>>>
>>>> if (gc_type == BG_GC) {
>>>> - if (PageWriteback(page))
>>>
>>> Seems there is no reason to remove the existing writeback case.
>>> This is a BG_GC case and we don't need to proceed GC in this case.
>> Here, PageWriteback(page) means two cases:
>> 1. write to this victim segment
>> 2. write to a new place
>>
>> In the first case, if we return directly, this victim bg_gc will fail
>> for some blocks are still valid.
>
> It should be fine, and its main reason would be to avoid wait_on_page_writeback.
> Next bg_gc will do better.
Agreed.
Yunlei, I think it's better add this judgment condition after we grabbed locked
data page for covering both background and foreground GC cases, instead of
changing previous process logic.
Thanks,
>
> Thanks,
>
> ------------------------------------------------------------------------------
> The Command Line: Reinvented for Modern Developers
> Did the resurgence of CLI tooling catch you by surprise?
> Reconnect with the command line and become more productive.
> Learn the new .NET and ASP.NET CLI. Get your free copy!
> http://sdm.link/telerik
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>
> .
>
------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] f2fs: return directly if block has been removed from the victim
2016-11-02 7:19 ` Chao Yu
@ 2016-11-02 10:40 ` heyunlei
0 siblings, 0 replies; 6+ messages in thread
From: heyunlei @ 2016-11-02 10:40 UTC (permalink / raw)
To: Chao Yu, Jaegeuk Kim; +Cc: linux-f2fs-devel
On 2016/11/2 15:19, Chao Yu wrote:
> On 2016/10/28 6:34, Jaegeuk Kim wrote:
>> On Wed, Oct 26, 2016 at 10:14:04AM +0800, heyunlei wrote:
>>>
>>>
>>> On 2016/10/26 9:09, Jaegeuk Kim wrote:
>>> Hi Kim,
>>>> Hi Yunlei,
>>>>
>>>> On Mon, Oct 24, 2016 at 11:37:28AM +0800, Yunlei He wrote:
>>>>> If one block has been to written to a new place, just return
>>>>> in move data process.
>>>>>
>>>>> Signed-off-by: Yunlei He <heyunlei@huawei.com>
>>>>> ---
>>>>> fs/f2fs/gc.c | 17 ++++++++++++-----
>>>>> 1 file changed, 12 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>>>> index f35fca5..fc78f3e 100644
>>>>> --- a/fs/f2fs/gc.c
>>>>> +++ b/fs/f2fs/gc.c
>>>>> @@ -545,7 +545,8 @@ static bool is_alive(struct f2fs_sb_info *sbi, struct f2fs_summary *sum,
>>>>> return true;
>>>>> }
>>>>>
>>>>> -static void move_encrypted_block(struct inode *inode, block_t bidx)
>>>>> +static void move_encrypted_block(struct inode *inode, block_t bidx,
>>>>> + unsigned int segno, int off)
>>>>> {
>>>>> struct f2fs_io_info fio = {
>>>>> .sbi = F2FS_I_SB(inode),
>>>>> @@ -580,6 +581,9 @@ static void move_encrypted_block(struct inode *inode, block_t bidx)
>>>>> * don't cache encrypted data into meta inode until previous dirty
>>>>> * data were writebacked to avoid racing between GC and flush.
>>>>> */
>>>>> + if (!check_valid_map(F2FS_I_SB(inode), segno, off))
>>>>> + goto out;
>>>>
>>>> This is done by gc_data_segment() before calling move function.
>>>> Why do we need this again?
>>>
>>> gc_data_segment() check this without lock the data page. If this block
>>> is written to a new place between check in gc_data_segment() and locked
>>> the page, it will need to wait this page write back, and write it again,
>>> it's unnecessary.
>>>
>>>>
>>>>> +
>>>>> f2fs_wait_on_page_writeback(page, DATA, true);
>>>>>
>>>>> get_node_info(fio.sbi, dn.nid, &ni);
>>>>> @@ -646,7 +650,8 @@ static void move_encrypted_block(struct inode *inode, block_t bidx)
>>>>> f2fs_put_page(page, 1);
>>>>> }
>>>>>
>>>>> -static void move_data_page(struct inode *inode, block_t bidx, int gc_type)
>>>>> +static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
>>>>> + unsigned int segno, int off)
>>>>> {
>>>>> struct page *page;
>>>>>
>>>>> @@ -655,8 +660,10 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type)
>>>>> return;
>>>>>
>>>>> if (gc_type == BG_GC) {
>>>>> - if (PageWriteback(page))
>>>>
>>>> Seems there is no reason to remove the existing writeback case.
>>>> This is a BG_GC case and we don't need to proceed GC in this case.
>>> Here, PageWriteback(page) means two cases:
>>> 1. write to this victim segment
>>> 2. write to a new place
>>>
>>> In the first case, if we return directly, this victim bg_gc will fail
>>> for some blocks are still valid.
>>
>> It should be fine, and its main reason would be to avoid wait_on_page_writeback.
>> Next bg_gc will do better.
>
> Agreed.
>
> Yunlei, I think it's better add this judgment condition after we grabbed locked
> data page for covering both background and foreground GC cases, instead of
> changing previous process logic.
>
> Thanks,
Ok, I have sent a new version.
Thanks,
>
>>
>> Thanks,
>>
>> ------------------------------------------------------------------------------
>> The Command Line: Reinvented for Modern Developers
>> Did the resurgence of CLI tooling catch you by surprise?
>> Reconnect with the command line and become more productive.
>> Learn the new .NET and ASP.NET CLI. Get your free copy!
>> http://sdm.link/telerik
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>
>> .
>>
>
>
> .
>
------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-11-02 10:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-24 3:37 [PATCH] f2fs: return directly if block has been removed from the victim Yunlei He
2016-10-26 1:09 ` Jaegeuk Kim
2016-10-26 2:14 ` heyunlei
2016-10-27 22:34 ` Jaegeuk Kim
2016-11-02 7:19 ` Chao Yu
2016-11-02 10:40 ` heyunlei
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).