linux-f2fs-devel.lists.sourceforge.net archive mirror
 help / color / mirror / Atom feed
* Re: [f2fs-dev] Reply: Reply: Reply: [PATCH -next 2/4] f2fs: extent cache: support extent for no-compressed file
@ 2022-09-22  2:07 zhangqilong via Linux-f2fs-devel
  2022-09-22  2:23 ` Chao Yu
  0 siblings, 1 reply; 8+ messages in thread
From: zhangqilong via Linux-f2fs-devel @ 2022-09-22  2:07 UTC (permalink / raw)
  To: Chao Yu, jaegeuk; +Cc: linux-f2fs-devel

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

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

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

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

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

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

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

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

^ permalink raw reply	[flat|nested] 8+ messages in thread
* [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
end of thread, other threads:[~2022-09-22 10:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-22  2:07 [f2fs-dev] Reply: Reply: Reply: [PATCH -next 2/4] f2fs: extent cache: support extent for no-compressed file zhangqilong via Linux-f2fs-devel
2022-09-22  2:23 ` Chao Yu
  -- strict thread matches above, loose matches on Subject: below --
2022-09-22  9:32 zhangqilong via Linux-f2fs-devel
2022-09-22 10:14 ` Chao Yu
2022-09-22  4:54 zhangqilong via Linux-f2fs-devel
2022-09-22  6:58 ` Chao Yu
2022-09-21 16:00 zhangqilong via Linux-f2fs-devel
2022-09-22  1:51 ` Chao Yu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).