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

[Jiucheng Xu]

On 6/25/2026 3:09 PM, Chao Yu wrote:
> [Some people who received this message don't often get email from chao@xxxxxxxxxx. 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@xxxxxxxxxxx>
> > > > ---
> > > >    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,