Re: [PATCH] f2fs: fix FG GC failure when file in victim is pinned

[Jiucheng Xu <jiucheng.xu@amlogic.com>]

On 6/25/2026 3:09 PM, Chao Yu wrote:
> [Some people who received this message don't often get email from chao@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> [ EXTERNAL EMAIL ]
> 
> On 6/22/26 18:40, Jiucheng Xu wrote:
>>
>> Regarding "why the pinfile is fragmented" you said, we use v5.15 + android U. Is it that v5.15 lacks some patches for special handling of pinfiles?
> 
> Ah, I think you can unpin log file created by logcat, the flag is added
> in aosp/1014260 to avoid fragmentation in filesystem (), but it's gone in
> aosp/43c6d76a
> 
> Thanks,

Thanks Chao, I think unpinning the log file can avoid this issue.

Let me ask a bit more.

I wonder why gc_control.nr_free_secs must be 0 in f2fs_expand_inode_data().

static int f2fs_expand_inode_data(struct inode *inode, loff_t offset,
                                         loff_t len, int mode)
{
         struct f2fs_gc_control gc_control = { .victim_segno = NULL_SEGNO,
                         .init_gc_type = FG_GC,
                         .should_migrate_blocks = false,
                         .err_gc_skipped = true,
                         .nr_free_secs = 0 };


Since it has reached such an urgent situation that FG GC needs to be 
triggered, I think the GC should try the second most suitable victim 
instead of returning -EAGAIN.

int f2fs_gc(struct f2fs_sb_info *sbi, struct f2fs_gc_control 
*gc_control)
{
         if (gc_type == FG_GC) {
                 sbi->cur_victim_sec = NULL_SEGNO;

                 if (has_enough_free_secs(sbi, sec_freed, 0)) {
                         if (!gc_control->no_bg_gc &&
                             total_sec_freed < gc_control->nr_free_secs) 
//If nr_free_secs = 1, GC can try the second most suitable victim.

                                 goto go_gc_more;
                         goto stop;
                 }

Is there any risk if it is set to 1?

If I'm not mistaken, for pinned files, the allocation in 
f2fs_expand_inode_data() is also carried out in units of sections. 
Therefore, it should be reasonable to set nr_free_secs to 1 here.

Welcome your feedback!

Thanks,

>>>> Signed-off-by: Jiucheng Xu <jiucheng.xu@amlogic.com>
>>>> ---
>>>>    fs/f2fs/file.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>> index 8acdd94272a0ced448e0ba21635d702cfec10682..3e49a73bbf3a184a314e97bff9509a66c27eac00 100644
>>>> --- a/fs/f2fs/file.c
>>>> +++ b/fs/f2fs/file.c
>>>> @@ -1883,7 +1883,7 @@ static int f2fs_expand_inode_data(struct inode *inode, loff_t offset,
>>>>                        .init_gc_type = FG_GC,
>>>>                        .should_migrate_blocks = false,
>>>>                        .err_gc_skipped = true,
>>>> -                     .nr_free_secs = 0 };
>>>> +                     .nr_free_secs = 1 };
>>>>        pgoff_t pg_start, pg_end;
>>>>        loff_t new_size;
>>>>        loff_t off_end;
>>>>
>>>> ---
>>>> base-commit: b51f606aa323d553d786ed681a213f134dc688d6
>>>> change-id: 20260620-origin-dev-99cdccc83800
>>>>
>>>> Best regards,
>>>
>>
>