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 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?

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