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

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