Re: [f2fs-dev] [PATCH RESEND 3/5] f2fs: punch largest extent instead of dropping it entirely on overlap

On 6/23/26 17:31, Yongpeng Yang wrote:
> 
> On 6/22/26 08:45, Chao Yu via Linux-f2fs-devel wrote:
>> On 6/22/26 00:07, Yongpeng Yang wrote:
>>>
>>> On 6/20/26 2:15 PM, Chao Yu via Linux-f2fs-devel wrote:
>>>> On 6/19/26 22:38, Yongpeng Yang wrote:
>>>>>
>>>>> On 6/15/26 8:05 PM, Chao Yu via Linux-f2fs-devel wrote:
>>>>>> On 6/12/26 19:58, Yongpeng Yang wrote:
>>>>>>> From: Yongpeng Yang <[email protected]>
>>>>>>>
>>>>>>> Previously, when an extent being inserted overlaps with the largest
>>>>>>> extent, the largest extent is dropped entirely. This was done to
>>>>>>> handle
>>>>>>
>>>>>> Please correct me if I missed anything, I remember that we will add
>>>>>> largest
>>>>>> extent in below path?
>>>>>>
>>>>>> - __update_extent_tree_range
>>>>>>   - __insert_extent_tree
>>>>>>    - __try_update_largest_extent : update largest w/ right extent
>>>>>>   - __try_update_largest_extent : update largest w/ left extent
>>>>>
>>>>> The largest extent might not reside in the extent tree. If
>>>>> __update_extent_tree_range invokes __drop_largest_extent, the length of
>>>>
>>>> I meant __update_extent_tree_range invokes __drop_largest_extent to drop
>>>> largest first, and then, it tries to update largest extent w/ larger-
>>>> size
>>>> one of separated two extents.
>>>>
>>>>> the largest extent will have been set to zero. As a result, the largest
>>>>> extent updated inside __try_update_largest_extent can end up smaller
>>>>> than the largest extent obtained after __punch_largest_extent completes
>>>>> its punch operation.
>>>>
>>>> I didn't get it, can you give an example for this?
>>>
>>> The only distinction between punch and drop arises when the largest
>>> extent is not present in the extent tree.
>>>
>>> Before this patch(extent format [fofs, len, blk])
>>> 1. inital state
>>>      largest extent: [0, 1024, 10], extent tree: empty
>>
>>                                                    ^^^^^
>>
>> It's not empty w/o patch 2/5.
> 
> Sorry for the confusion. "empty" means that the largest extent has been
> shrunk.

Okay,

Hmm...I found it's hard to reproduce such condition "only shrink extent slab
cache and keep inode slab cache", so I wonder how common this issue will be
in product.

Have you suffered real issues in product? or just insight from LLM or code
review?

Thanks,

> 
> Thanks
> Yongpeng,
> 
>>
>> Thanks,
>>
>>> 2. insert [511, 1, 10000]
>>>      largest extent: [511, 1, 10000], extent tree: [511, 1, 10000]
>>>
>>> After this patch:
>>> 1. inital state
>>>      largest extent: [0, 1024, 10], extent tree: empty
>>> 2. insert [511, 1, 10000]
>>>      largest extent: [512, 512, 522], extent tree: [511, 1, 10000]
>>>
>>> Thanks
>>> Yongpeng,
>>>
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>> Thanks
>>>>> Yongpeng,
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>>> the case where the largest extent is not in memory, avoiding
>>>>>>> inconsistency between the largest extent and the extent tree.
>>>>>>>
>>>>>>> This patch changes the semantics of __drop_largest_extent (renamed to
>>>>>>> __punch_largest_extent): instead of discarding the entire largest
>>>>>>> extent when any overlap is detected, keep the larger remaining
>>>>>>> portion
>>>>>>> (left or right) after the punch. This preserves extent cache coverage
>>>>>>> for truncate and overwrite operations that only partially overlap the
>>>>>>> largest extent.
>>>>>>>
>>>>>>> Signed-off-by: Yongpeng Yang <[email protected]>
>>>>>>> ---
>>>>>>>   fs/f2fs/extent_cache.c | 31 ++++++++++++++++++++++++-------
>>>>>>>   1 file changed, 24 insertions(+), 7 deletions(-)
>>>>>>>
>>>>>>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>>>>>>> index f8d94db60dc6..82d84c4e98b2 100644
>>>>>>> --- a/fs/f2fs/extent_cache.c
>>>>>>> +++ b/fs/f2fs/extent_cache.c
>>>>>>> @@ -397,14 +397,31 @@ static unsigned int
>>>>>>> __free_extent_tree(struct f2fs_sb_info *sbi,
>>>>>>>       return count;
>>>>>>>   }
>>>>>>> -static void __drop_largest_extent(struct extent_tree *et,
>>>>>>> +static void __punch_largest_extent(struct extent_tree *et,
>>>>>>>                       pgoff_t fofs, unsigned int len)
>>>>>>>   {
>>>>>>> -    if (fofs < (pgoff_t)et->largest.fofs + et->largest.len &&
>>>>>>> -            fofs + len > et->largest.fofs) {
>>>>>>> -        et->largest.len = 0;
>>>>>>> -        et->largest_updated = true;
>>>>>>> +    unsigned int largest_end, punch_end;
>>>>>>> +    unsigned int left_len, right_len;
>>>>>>> +
>>>>>>> +    if (fofs >= (pgoff_t)et->largest.fofs + et->largest.len ||
>>>>>>> +            fofs + len <= et->largest.fofs)
>>>>>>> +        return;
>>>>>>> +
>>>>>>> +    /* Punch [fofs, fofs + len) from largest extent. */
>>>>>>> +    largest_end = et->largest.fofs + et->largest.len;
>>>>>>> +    punch_end = fofs + len;
>>>>>>> +
>>>>>>> +    left_len = fofs > et->largest.fofs ? fofs - et-
>>>>>>>> largest.fofs : 0;
>>>>>>> +    right_len = largest_end > punch_end ? largest_end -
>>>>>>> punch_end : 0;
>>>>>>> +
>>>>>>> +    if (left_len >= right_len) {
>>>>>>> +        et->largest.len = left_len;
>>>>>>> +    } else {
>>>>>>> +        et->largest.blk += punch_end - et->largest.fofs;
>>>>>>> +        et->largest.fofs = punch_end;
>>>>>>> +        et->largest.len = right_len;
>>>>>>>       }
>>>>>>> +    et->largest_updated = true;
>>>>>>>   }
>>>>>>>   void f2fs_init_read_extent_tree(struct inode *inode, struct
>>>>>>> folio *ifolio)
>>>>>>> @@ -680,10 +697,10 @@ static void
>>>>>>> __update_extent_tree_range(struct inode *inode,
>>>>>>>           dei.len = 0;
>>>>>>>           /*
>>>>>>> -         * drop largest extent before lookup, in case it's already
>>>>>>> +         * punch largest extent before lookup, in case it's already
>>>>>>>            * been shrunk from extent tree
>>>>>>>            */
>>>>>>> -        __drop_largest_extent(et, fofs, len);
>>>>>>> +        __punch_largest_extent(et, fofs, len);
>>>>>>>       }
>>>>>>>       if (et->largest.len != 0 &&
>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> 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
> 



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