* 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; 4+ 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] 4+ 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 [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 10:14 ` Chao Yu
2022-09-22 11:13 ` [f2fs-dev] 答复: " zhangqilong via Linux-f2fs-devel
0 siblings, 1 reply; 4+ 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] 4+ messages in thread
* [f2fs-dev] 答复: Reply: Reply: Reply: [PATCH -next 2/4] f2fs: extent cache: support extent for no-compressed file
2022-09-22 10:14 ` Chao Yu
@ 2022-09-22 11:13 ` zhangqilong via Linux-f2fs-devel
2022-09-22 11:52 ` Chao Yu
0 siblings, 1 reply; 4+ messages in thread
From: zhangqilong via Linux-f2fs-devel @ 2022-09-22 11:13 UTC (permalink / raw)
To: Chao Yu, 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?
I means it is wrote when being sloaded to f2fs ro image.
Thanks,
>
> 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] 4+ 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 11:13 ` [f2fs-dev] 答复: " zhangqilong via Linux-f2fs-devel
@ 2022-09-22 11:52 ` Chao Yu
0 siblings, 0 replies; 4+ messages in thread
From: Chao Yu @ 2022-09-22 11:52 UTC (permalink / raw)
To: zhangqilong, jaegeuk; +Cc: linux-f2fs-devel
On 2022/9/22 19:13, Zhangqilong wrote:
> I means it is wrote when being sloaded to f2fs ro image.
sload doesn't trigger concurrent writes.
Thanks
_______________________________________________
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] 4+ messages in thread
end of thread, other threads:[~2022-09-22 11:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-22 9:32 [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 10:14 ` Chao Yu
2022-09-22 11:13 ` [f2fs-dev] 答复: " zhangqilong via Linux-f2fs-devel
2022-09-22 11:52 ` Chao Yu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.