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

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