On 2017/11/17 11:30, Yunlong Song wrote:
> How about add file_write_and_wait_range in __write_node_page as following:
> if (atomic && !test_opt(sbi, NOBARRIER)) {
> file_write_and_wait_range(file, 0, LLONG_MAX);
Nope, GCed encrypted data wouldn't be cached in inode page cache.
I don't think adding raw code to flush data here is a good implementation,
it incurs complicated lock dependency relation. Instead, IMO, it will be
better to use inmem_lock to avoid race in between GC and atomic commit flow.
Thanks,
> fio.op_flags |= REQ_PREFLUSH | REQ_FUA;
> }
>
> The all the GCed data will be flushed to non-volatile before last node
> write with REQ_PREFLUSH | REQ_FUA.
>
> On 2017/11/17 11:20, Chao Yu wrote:
>> On 2017/11/17 11:04, Yunlong Song wrote:
>>> The atomic commit will trigger:
>>> -f2fs_do_sync_file(filp, 0, LLONG_MAX, 0, true)
>>> -file_write_and_wait_range(file, 0, LLONG_MAX)
>>> -fsync_node_pages
>>> -__write_node_page
>>> -REQ_PREFLUSH | REQ_FUA
>>>
>>> So data is flushed to non-volatile before last node write with > REQ_PREFLUSH | REQ_FUA,
>> I mean GCed data.
>>
>> - file_write_and_wait_range
>> - move_data_block
>> - f2fs_submit_page_write
>> - f2fs_update_data_blkaddr
>> - set_page_dirty
>> - fsync_node_pages
>>
>> Thanks,
>>
>>> we do not need to worry about the inconsistent problem. Right?
>>>
>>> On 2017/11/17 10:49, Chao Yu wrote:
>>>> On 2017/11/17 8:58, Yunlong Song wrote:
>>>>> Is there any problem if just deleting the judgement condition in this patch?
>>>> IIRC, dirty node comes from data segment GC can be writebacked & flushed during
>>>> atomic commit, but related data will still be in inner bio cache, after later
>>>> SPOR, data would be inconsistent.
>>>>
>>>> Thanks,
>>>>
>>>>> On 2017/11/8 17:28, Chao Yu wrote:
>>>>>> On 2017/11/8 10:34, Yunlong Song wrote:
>>>>>>> If some files are opened with atomic flag and have not commited yet, at
>>>>>>> the same time, if all the target victim segments have at least one page
>>>>>>> of these atomic files, then f2fs gc will fail to do gc and hangs in the
>>>>>>> process of go to gc_more, since gc_date_segment will not move any data
>>>>>>> and get_valid_blocks will never be 0, then do_garbage_collect will
>>>>>>> always return 0.
>>>>>> Oh, I added this judgment condition to avoid ruining atomic write by data
>>>>>> GC, could we find another way to solve this issue? BTW, if there is direct
>>>>>> IO, we will also skip data segment GC.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>>> Signed-off-by: Yunlong Song <yunlong.song@xxxxxxxxxx>
>>>>>>> ---
>>>>>>> fs/f2fs/gc.c | 6 ------
>>>>>>> 1 file changed, 6 deletions(-)
>>>>>>>
>>>>>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>>>>>> index 5d5bba4..3fdcd04 100644
>>>>>>> --- a/fs/f2fs/gc.c
>>>>>>> +++ b/fs/f2fs/gc.c
>>>>>>> @@ -621,9 +621,6 @@ static void move_data_block(struct inode *inode, block_t bidx,
>>>>>>> if (!check_valid_map(F2FS_I_SB(inode), segno, off))
>>>>>>> goto out;
>>>>>>>
>>>>>>> - if (f2fs_is_atomic_file(inode))
>>>>>>> - goto out;
>>>>>>> -
>>>>>>> set_new_dnode(&dn, inode, NULL, NULL, 0);
>>>>>>> err = get_dnode_of_data(&dn, bidx, LOOKUP_NODE);
>>>>>>> if (err)
>>>>>>> @@ -718,9 +715,6 @@ static void move_data_page(struct inode *inode, block_t bidx, int gc_type,
>>>>>>> if (!check_valid_map(F2FS_I_SB(inode), segno, off))
>>>>>>> goto out;
>>>>>>>
>>>>>>> - if (f2fs_is_atomic_file(inode))
>>>>>>> - goto out;
>>>>>>> -
>>>>>>> if (gc_type == BG_GC) {
>>>>>>> if (PageWriteback(page))
>>>>>>> goto out;
>>>>>>>
>>>>>> .
>>>>>>
>>>> .
>>>>
>>
>> .
>>
>
