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

Jiucheng Xu via Linux-f2fs-devel <[email protected]>
Newsgroups net.sourceforge.lists.linux-f2fs-devel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>

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



_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.