Re: [f2fs-dev] [PATCH RESEND 2/5] f2fs: only initialize largest extent without extent_node at inode init

On 6/21/26 23:48, Yongpeng Yang wrote:
> On 6/20/26 11:31 AM, Chao Yu via Linux-f2fs-devel wrote:
>> On 6/19/26 22:34, Yongpeng Yang wrote:
>>>
>>> On 6/15/26 7:55 PM, Chao Yu via Linux-f2fs-devel wrote:
>>>> On 6/12/26 19:58, Yongpeng Yang wrote:
>>>>> From: Yongpeng Yang <[email protected]>
>>>>>
>>>>> The largest extent takes effect during both read mapping and write
>>>>> mapping lookups, while read mapping does not need to access the
>>>>> extent_node. For write mapping, the case where the largest extent is
>>>>> not in the extent tree can already be handled by the merge logic, and
>>>>> cases that cannot be merged do not require the largest extent to
>>>>> participate either.
>>>>>
>>>>> Therefore, the largest extent does not need to initialize a
>>>>> corresponding extent_node, reducing memory footprint.
>>>>>
>>>>> Signed-off-by: Yongpeng Yang <[email protected]>
>>>>> ---
>>>>>   fs/f2fs/extent_cache.c | 18 +-----------------
>>>>>   1 file changed, 1 insertion(+), 17 deletions(-)
>>>>>
>>>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>>>>> index aa368a01b035..f8d94db60dc6 100644
>>>>> --- a/fs/f2fs/extent_cache.c
>>>>> +++ b/fs/f2fs/extent_cache.c
>>>>> @@ -410,10 +410,8 @@ static void __drop_largest_extent(struct extent_tree *et,
>>>>>   void f2fs_init_read_extent_tree(struct inode *inode, struct folio *ifolio)
>>>>>   {
>>>>>       struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>>>>> -    struct extent_tree_info *eti = &sbi->extent_tree[EX_READ];
>>>>>       struct f2fs_extent *i_ext = &F2FS_INODE(ifolio)->i_ext;
>>>>>       struct extent_tree *et;
>>>>> -    struct extent_node *en;
>>>>>       struct extent_info ei = {0};
>>>>>       if (!__may_extent_tree(inode, EX_READ)) {
>>>>> @@ -435,21 +433,7 @@ void f2fs_init_read_extent_tree(struct inode *inode, struct folio *ifolio)
>>>>>       if (atomic_read(&et->node_cnt) || !ei.len)
>>>>>           goto skip;
>>>>> -    if (IS_DEVICE_ALIASING(inode)) {
>>>>> -        et->largest = ei;
>>>>> -        goto skip;
>>>>> -    }
>>>>> -
>>>>> -    en = __attach_extent_node(sbi, et, &ei, NULL,
>>>>> -                &et->root.rb_root.rb_node, true);
>>>>> -    if (en) {
>>>>> -        et->largest = en->ei;
>>>>> -        et->cached_en = en;
>>>>> -
>>>>> -        spin_lock(&eti->extent_lock);
>>>>> -        list_add_tail(&en->list, &eti->extent_list);
>>>>> -        spin_unlock(&eti->extent_lock);
>>>>> -    }
>>>>> +    et->largest = ei;
>>>>
>>>> Previously, we can split largest extent node to two if we punched it, now
>>>> we can not? IIUC.
>>>
>>> Prior to this change, the largest extent could also be shrunk, so the
>>> set of scenarios that need handling during punch remains identical
>>> before and after the modification.
>>>
>>> For the largest extent, it only needs to guarantee that the mapping
>>> information it records stays consistent with the mappings tracked in the
>>> extent tree and multi-level indirect indices. The punch operation does
>>> not break this consistency. Moreover, the largest extent is not required
>>> to be the longest extent among all entries in the extent tree. It merely
>>> needs to match the mappings stored in multi-level indirect indices and
>>> be no smaller than the maximum extent present in the extent tree.
>>
>> What I meant is: e.g. previously, if largest extent is 1024, punching in the
>> middle of the extent will make it being split to two extents (512, 511) in
>> __update_extent_tree_range().
>>
>> If we do not add largest extent node into rb tree in f2fs_init_read_extent_tree(),
>> then we may has no chance to keep small-sized(511) extent in above punch scenario,
>> can you verify that?
> 
> Yes, the 511-block mapping will be dropped. I considered this case earlier: when the newly inserted mapping overlaps with or adjacent with the largest extent, we reinsert the largest extent back into the extent tree. With this logic, the 511-block split extent will remain present in the extent tree. Instead of performing punch handling on the largest extent, we can simply drop it directly.
> However, this approach does not work well for updates targeting the tail of the original extent. For instance, punch 1024 into 1022 and 1 for overwrite case, which will also alloc one more extent_node.

We will only add extent which has size >= F2FS_MIN_EXTENT_LEN, so 1022
will be added, and 1 will be dropped.

Thanks,

> 
> Thanks
> Yongpeng,
> 
>>
>> Thanks,
>>
>>>
>>> Thanks
>>> Yongpeng,
>>>
>>>
>>>>
>>>> Thanks,
>>>>
>>>>>   skip:
>>>>>       /* Let's drop, if checkpoint got corrupted. */
>>>>>       if (f2fs_cp_error(sbi)) {
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Linux-f2fs-devel mailing list
>>>> [email protected]
>>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>>
>>
>>
>>
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 



_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel