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

* [f2fs-dev] Reply: Reply: Reply: [PATCH -next 2/4] f2fs: extent cache: support extent for no-compressed file
@ 2022-09-21 16:00 zhangqilong via Linux-f2fs-devel
  2022-09-22  1:51 ` Chao Yu
  0 siblings, 1 reply; 8+ messages in thread
From: zhangqilong via Linux-f2fs-devel @ 2022-09-21 16:00 UTC (permalink / raw)
  To: Chao Yu, jaegeuk; +Cc: linux-f2fs-devel

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

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

^ permalink raw reply	[flat|nested] 8+ messages in thread