Re: [f2fs-dev] Reply: Reply: Reply: [PATCH -next 2/4] f2fs: extent cache: support extent for no-compressed file

[Chao Yu <chao@kernel.org>]

On 2022/9/22 12:54, zhangqilong wrote:
>> On 2022/9/22 10:07, zhangqilong wrote:
>>>> On 2022/9/22 0:00, zhangqilong wrote:
>>>>>> On 2022/9/21 21:57, zhangqilong wrote:
>>>>>>>> On 2022/9/21 20:14, zhangqilong wrote:
>>>>>>>>>> On 2022/9/21 15:57, Zhang Qilong wrote:
>>>>>>>>>>> No-compressed file may suffer read performance issue due to
>> it
>>>>>>>>>>> can't use extent cache or the largest extent in inode can't
>>>>>>>>>>> covered other parts of continuous blocks in readonly format
>>>>>>>>>>> f2fs
>>>>>> image.
>>>>>>>>>>>
>>>>>>>>>>> Now it won't build extent cacge tree for no-compressed file in
>>>>>>>>>>> readonly format f2fs image.
>>>>>>>>>>>
>>>>>>>>>>> For readonly format f2fs image, maybe the no-compressed file
>>>>>> don't
>>>>>>>>>>> have the largest extent, or it have more than one part which
>>>>>>>>>>> have
>>>>>>>>>>
>>>>>>>>>> Why it can not have largest extent in f2fs_inode?
>>>>>>>>>
>>>>>>>>> The following several situations may occur:
>>>>>>>>> 	1) Wrote w/o the extent when the filesystem is read-write
>> fs.
>>>>>>>>>
>>>>>>>>>          2) Largest extent have been drop after being re-wrote,
>>>>>>>>> or it have
>>>>>>>> been split to smaller parts.
>>>>>>>>>
>>>>>>>>>          3) The largest extent only covered one part of
>>>>>>>>> continuous blocks,
>>>>>>>> like:
>>>>>>>>>            |------parts 1(continuous blocks)-----|----not
>>>>>>>> continuous---|---------------------parts 2 (continuous
>>>>>>>> continuous---|blocks)-----------|---------|
>>>>>>>>>            The largest extent is part 2, but other parts (like
>>>>>>>>> part1, ) can't be
>>>>>>>> mapped in readonly format f2fs image which should have been
>>>>>> mapped.
>>>>>>>>
>>>>>>>> largest extent of non-compressed file should be updated during
>>>>>>>> sload in a ro f2fs image?
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> I am sorry, I don't fully understand what you mean. I want to show
>>>>>>> that the extent of file in readonly format f2fs image could not
>>>>>>> existed or
>>>>>> can't covered other parts that contain continuous blocks.
>>>>>>
>>>>>> Well, I mean the extent should be updated due to below flow?
>> during
>>>>>> the file was loaded into a formated f2fs image w/ ro feature.
>>>>>>
>>>>>> - f2fs_sload
>>>>>>      - build_directory
>>>>>>       - f2fs_build_file
>>>>>>
>>>>>> 	if (!c.compress.enabled || (c.feature &
>>>>>> cpu_to_le32(F2FS_FEATURE_RO)))
>>>>>> 		update_largest_extent(sbi, de->ino);
>>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> I get it. I think we could consider this flow.
>>>>> But it will change the metadata of the file, is it was a little
>> inappropriate?
>>>>> Maybe users does not want to do that during being loaded and will
>>>> refuse this change.
>>>>
>>>> I don't get it.
>>>>
>>>> IIUC, we can only load files into ro f2fs image w/ sload, and sload
>>>> has updated largest extent for file, or am I missing something?
>>>
>>> Ah, maybe I misunderstood your idea just now. We could updated
>> largest
>>> extent for file when loading into ro f2fs image w/ sload, but it will
>>> not solve situations 3) issue for the no-compressed file that be
>> mentioned previously.
>>
>> Why case 3) can happen? The data blocks should be allocated
>> contiguously while sload loads file into image?
> 
> That's what I thought before, it should allocated contiguously while
> sload loads file into image. But I found that it may not be completely
> continuous for the big file recently. It may contain several discontinuous
> parts.

I doubt it was caused by hot/cold data separation strategy, but I guess the
strategy is not necessary for ro image, can you look into it?

Thanks,

> 
> Thanks,
> 
>>
>> Thanks,
>>
>>>
>>> Thanks,
>>>
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>> Thanks,
>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>>> Whether it exists or not, we will not change or update largest
>>>>>>> extent
>>>>>> during sload in a ro f2fs image.
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>
>>>>>>>>>>> internally continuous blocks. So we add extent cache tree for
>>>>>>>>>>> the no-compressed file in readonly format f2fs image.
>>>>>>>>>>>
>>>>>>>>>>> The cache policy is almost same with compressed file. The
>>>>>>>>>>> difference is that, the no-compressed file part will set
>>>>>>>>>>> min-number of continuous blocks F2FS_MIN_EXTENT_LEN in
>>>> order
>>>>>> to
>>>>>>>>>>> reduce cache
>>>>>>>> fragmentation.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com>
>>>>>>>>>>> ---
>>>>>>>>>>>        fs/f2fs/extent_cache.c | 52
>>>>>>>>>> ++++++++++++++++++++++++++++++++++--------
>>>>>>>>>>>        1 file changed, 42 insertions(+), 10 deletions(-)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>>>>>>>>>>> index
>>>>>>>>>>> 387d53a61270..7e39381edca0 100644
>>>>>>>>>>> --- a/fs/f2fs/extent_cache.c
>>>>>>>>>>> +++ b/fs/f2fs/extent_cache.c
>>>>>>>>>>> @@ -695,9 +695,12 @@ static void
>>>>>>>>>> f2fs_update_extent_tree_range_compressed(struct inode
>>>> *inode,
>>>>>>>>>>>        	set_extent_info(&ei, fofs, blkaddr, llen);
>>>>>>>>>>>        	ei.c_len = c_len;
>>>>>>>>>>>
>>>>>>>>>>> -	if (!__try_merge_extent_node(sbi, et, &ei, prev_en,
>>>> next_en))
>>>>>>>>>>> +	if (!__try_merge_extent_node(sbi, et, &ei, prev_en,
>>>> next_en)) {
>>>>>>>>>>> +		if (!c_len && llen < F2FS_MIN_EXTENT_LEN)
>>>>>>>>>>> +			goto unlock_out;
>>>>>>>>>>>        		__insert_extent_tree(sbi, et, &ei,
>>>>>>>>>>>        				insert_p, insert_parent, leftmost);
>>>>>>>>>>> +	}
>>>>>>>>>>>        unlock_out:
>>>>>>>>>>>        	write_unlock(&et->lock);
>>>>>>>>>>>        }
>>>>>>>>>>> @@ -726,24 +729,53 @@ static unsigned int
>>>>>>>>>> f2fs_cluster_blocks_are_contiguous(struct dnode_of_data
>> *dn)
>>>>>>>>>>>        	return compressed ? i - 1 : i;
>>>>>>>>>>>        }
>>>>>>>>>>>
>>>>>>>>>>> +/*
>>>>>>>>>>> + * check whether normal file blocks are contiguous, and add
>>>>>>>>>>> +extent cache
>>>>>>>>>>> + * entry only if remained blocks are logically and physically
>>>>>>>> contiguous.
>>>>>>>>>>> + */
>>>>>>>>>>> +static unsigned int
>> f2fs_normal_blocks_are_contiguous(struct
>>>>>>>>>>> +dnode_of_data *dn) {
>>>>>>>>>>> +	int i = 0;
>>>>>>>>>>> +	struct inode *inode = dn->inode;
>>>>>>>>>>> +	block_t first_blkaddr = data_blkaddr(inode, dn->node_page,
>>>>>>>>>>> +						dn->ofs_in_node);
>>>>>>>>>>> +	unsigned int max_blocks =
>>>> ADDRS_PER_PAGE(dn->node_page,
>>>>>>>> inode)
>>>>>>>>>>> +					- dn->ofs_in_node;
>>>>>>>>>>> +
>>>>>>>>>>> +	for (i = 1; i < max_blocks; i++) {
>>>>>>>>>>> +		block_t blkaddr = data_blkaddr(inode, dn->node_page,
>>>>>>>>>>> +				dn->ofs_in_node + i);
>>>>>>>>>>> +
>>>>>>>>>>> +		if (!__is_valid_data_blkaddr(blkaddr) ||
>>>>>>>>>>> +				first_blkaddr + i != blkaddr)
>>>>>>>>>>> +			return i;
>>>>>>>>>>> +	}
>>>>>>>>>>> +
>>>>>>>>>>> +	return i;
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>>        void f2fs_readonly_update_extent_cache(struct
>>>>>> dnode_of_data
>>>>>>>> *dn,
>>>>>>>>>>>        					pgoff_t index)
>>>>>>>>>>>        {
>>>>>>>>>>> -	unsigned int c_len =
>>>> f2fs_cluster_blocks_are_contiguous(dn);
>>>>>>>>>>> +	unsigned int c_len = 0;
>>>>>>>>>>> +	unsigned int llen = 0;
>>>>>>>>>>>        	block_t blkaddr;
>>>>>>>>>>>
>>>>>>>>>>> -	if (!c_len)
>>>>>>>>>>> -		return;
>>>>>>>>>>> -
>>>>>>>>>>>        	blkaddr = f2fs_data_blkaddr(dn);
>>>>>>>>>>> -	if (blkaddr == COMPRESS_ADDR)
>>>>>>>>>>> -		blkaddr = data_blkaddr(dn->inode, dn->node_page,
>>>>>>>>>>> +	if (f2fs_compressed_file(dn->inode)) {
>>>>>>>>>>> +		c_len = f2fs_cluster_blocks_are_contiguous(dn);
>>>>>>>>>>> +		if (!c_len)
>>>>>>>>>>> +			return;
>>>>>>>>>>> +		llen = F2FS_I(dn->inode)->i_cluster_size;
>>>>>>>>>>> +		if (blkaddr == COMPRESS_ADDR)
>>>>>>>>>>> +			blkaddr = data_blkaddr(dn->inode,
>>>> dn->node_page,
>>>>>>>>>>>        					dn->ofs_in_node + 1);
>>>>>>>>>>> +	} else {
>>>>>>>>>>> +		llen = f2fs_normal_blocks_are_contiguous(dn);
>>>>>>>>>>> +	}
>>>>>>>>>>>
>>>>>>>>>>>
>>>> 	f2fs_update_extent_tree_range_compressed(dn->inode,
>>>>>>>>>>> -				index, blkaddr,
>>>>>>>>>>> -				F2FS_I(dn->inode)->i_cluster_size,
>>>>>>>>>>> -				c_len);
>>>>>>>>>>> +				index, blkaddr, llen, c_len);
>>>>>>>>>>>        }
>>>>>>>>>>>        #endif
>>>>>>>>>>>


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel