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 <heyun...@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
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
