On 12/26/25 11:07, Chao Yu via Linux-f2fs-devel wrote:
> On 12/23/2025 8:02 PM, Yongpeng Yang wrote:
>>
>> On 11/10/25 17:32, Chao Yu via Linux-f2fs-devel wrote:
>>> On 11/10/25 17:20, Yongpeng Yang wrote:
>>>> On 11/8/25 11:11, Chao Yu via Linux-f2fs-devel wrote:
>>>>> Yunlei,
>>>>>
>>>>> On 2025/11/7 14:29, Yunlei He wrote:
>>>>>> From: Yunlei He <[email protected]>
>>>>>>
>>>>>> GC move fbe data block will add some non uptodate page, we'd
>>>>>> better release it at the end.
>>>>>
>>>>> This is just for saving memory, right?
>>>>>
>>>>
>>>> Yes, move_data_block() doesn’t read any data to folio, and the GC
>>>> usually selects cold data. Therefore, this folio is typically not
>>>> uptodate, and there’s no need for it to occupy the page cache.
>>>>
>>>>>>
>>>>>> Signed-off-by: Yunlei He <[email protected]>
>>>>>> Signed-off-by: Yongpeng Yang <[email protected]>
>>>>>> ---
>>>>>>    fs/f2fs/gc.c | 5 +++++
>>>>>>    1 file changed, 5 insertions(+)
>>>>>>
>>>>>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>>>>>> index 8abf521530ff..09b65e6ea853 100644
>>>>>> --- a/fs/f2fs/gc.c
>>>>>> +++ b/fs/f2fs/gc.c
>>>>>> @@ -1315,6 +1315,7 @@ static int move_data_block(struct inode
>>>>>> *inode, block_t bidx,
>>>>>>        struct node_info ni;
>>>>>>        struct folio *folio, *mfolio;
>>>>>>        block_t newaddr;
>>>>>> +    bool need_invalidate = true;
>>>>>>        int err = 0;
>>>>>>        bool lfs_mode = f2fs_lfs_mode(fio.sbi);
>>>>>>        int type = fio.sbi->am.atgc_enabled && (gc_type == BG_GC) &&
>>>>>> @@ -1450,7 +1451,11 @@ static int move_data_block(struct inode
>>>>>> *inode, block_t bidx,
>>>>>>    put_out:
>>>>>>        f2fs_put_dnode(&dn);
>>>>>>    out:
>>>>>> +    if (folio_test_uptodate(folio))
>>>>>> +        need_invalidate = false;
>>>>>
>>>>> How about dropping such folio under its lock?
>>>>>
>>>>> if (!folio_test_uptodate())
>>>>>       truncate_inode_partial_folio()
>>>>>
>>>>
>>>> truncate_inode_partial_folio() is more efficient since it avoids
>>>> looking
>>>> up the folio again, but it’s declared in mm/internal.h, so it cannot be
>>>> called directly.
>>>
>>> Yeah, or generic_error_remove_folio(), not sure.
>>>
>>> I just take a look to check whether there is a better alternative
>>> scheme.
>>
>> How about the following modification? The folio is marked with
>> PG_dropbehind using __folio_set_dropbehind(), and is subsequently
>> removed from the page cache through folio_end_dropbehind().
>>
>> Thanks,
>> Yongpeng
>>
>> --- a/fs/f2fs/gc.c
>> +++ b/fs/f2fs/gc.c
>> @@ -1326,6 +1326,7 @@ static int move_data_block(struct inode *inode,
>> block_t bidx,
>>          folio = f2fs_grab_cache_folio(mapping, bidx, false);
>>          if (IS_ERR(folio))
>>                  return PTR_ERR(folio);
>> +       __folio_set_dropbehind(folio);
>>
>>          if (!check_valid_map(F2FS_I_SB(inode), segno, off)) {
>>                  err = -ENOENT;
>> @@ -1453,7 +1454,10 @@ static int move_data_block(struct inode *inode,
>> block_t bidx,
>>   put_out:
>>          f2fs_put_dnode(&dn);
>>   out:
>> -       f2fs_folio_put(folio, true);
>> +       folio_unlock(folio);
>> +       folio_end_dropbehind(folio);
>> +       folio_put(folio);
> 
> Maybe:
> 
> out:
>     if (!folio_test_uptodate)
>         folio_set_dropbehind
>     folio_unlock
>     folio_end_dropbehind
>     folio_test_clear_dropbehind // make sure to clear the flag

Oh, I miss this, folio_end_dropbehind->folio_trylock may fail. I'll fix
this and send v2 patch.

Thanks
Yongpeng,

>     folio_put
> 
> Thanks,
> 
>>          return err;
>>   }
>>
>>>
>>> Thanks,
>>>
>>>>
>>>> Yongpeng,
>>>>
>>>>>>        f2fs_folio_put(folio, true);
>>>>>> +    if (need_invalidate)
>>>>>> +        invalidate_mapping_pages(mapping, bidx, bidx);
>>>>>>        return err;
>>>>>>    }
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> 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

Reply via email to