* 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
* Re: [f2fs-dev] Reply: Reply: Reply: [PATCH -next 2/4] f2fs: extent cache: support extent for no-compressed file
2022-09-22 4:54 [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 6:58 ` Chao Yu
0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2022-09-22 6:58 UTC (permalink / raw)
To: zhangqilong, 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?
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 9:32 zhangqilong via Linux-f2fs-devel
@ 2022-09-22 10:14 ` Chao Yu
0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2022-09-22 10:14 UTC (permalink / raw)
To: zhangqilong, jaegeuk; +Cc: linux-f2fs-devel
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
^ 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 2:07 zhangqilong via Linux-f2fs-devel
@ 2022-09-22 2:23 ` Chao Yu
0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2022-09-22 2:23 UTC (permalink / raw)
To: zhangqilong, 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?
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 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-21 16:00 zhangqilong via Linux-f2fs-devel
@ 2022-09-22 1:51 ` Chao Yu
0 siblings, 0 replies; 8+ messages in thread
From: Chao Yu @ 2022-09-22 1:51 UTC (permalink / raw)
To: zhangqilong, 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?
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 4:54 [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 6:58 ` 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 2:07 zhangqilong via Linux-f2fs-devel
2022-09-22 2:23 ` 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).