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 17:32, zhangqilong wrote:
>>
>> 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?
> 
> HI,
> 
> "Looking into hot/cold data separation strategy" is a little difficult for me
> now but I will continuous attention and research it in the future.
> In addition, concurrent writing of multiple files also is likely to cause this issue,
> so I think it is valueable sometimes :).

How can we write files into ro image?

Thanks,

> 
> Thanks,
> 
>>
>> 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