Re: [f2fs-dev] [PATCH] Revert "f2fs: stop allocating pinned sections if EAGAIN happens"

[Wu Bo]

On Fri, Sep 06, 2024 at 10:52:47PM +0000, Jaegeuk Kim via Linux-f2fs-devel wrote:
> On 09/06, Chao Yu wrote:
> > On 2024/9/6 16:31, Wu Bo wrote:
> > > On Tue, Feb 20, 2024 at 02:50:11PM +0800, Chao Yu wrote:
> > > > On 2024/2/8 16:11, Wu Bo wrote:
> > > > > On 2024/2/5 11:54, Chao Yu wrote:
> > > > > > How about calling f2fs_balance_fs() to double check and make sure there is
> > > > > > enough free space for following allocation.
> > > > > > 
> > > > > >          if (has_not_enough_free_secs(sbi, 0,
> > > > > >              GET_SEC_FROM_SEG(sbi, overprovision_segments(sbi)))) {
> > > > > >              f2fs_down_write(&sbi->gc_lock);
> > > > > >              stat_inc_gc_call_count(sbi, FOREGROUND);
> > > > > >              err = f2fs_gc(sbi, &gc_control);
> > > > > >              if (err == -EAGAIN)
> > > > > >                  f2fs_balance_fs(sbi, true);
> > > > > >              if (err && err != -ENODATA)
> > > > > >                  goto out_err;
> > > > > >          }
> > > > > > 
> > > > > > Thanks,
> > > > > 
> > > > > f2fs_balance_fs() here will not change procedure branch and may just trigger another GC.
> > > > > 
> > > > > I'm afraid this is a bit redundant.
> > > > 
> > > > Okay.
> > > > 
> > > > I guess maybe Jaegeuk has concern which is the reason to commit
> > > > 2e42b7f817ac ("f2fs: stop allocating pinned sections if EAGAIN happens").
> > > 
> > > Hi Jaegeuk,
> > > 
> > > We occasionally receive user complaints about OTA failures caused by this issue.
> > > Please consider merging this patch.
> 
> What about adding a retry logic here, as it's literally EAGAIN?

In this scenario, the remaining reclaimable sections has a block been pinned. As
a result, the user sees that there is enough free space, but fallocate still
fails. This happens because the GC triggered by fallocate cannot recycle the
section that has been pinned. No matter how many times it’s attempted, it will
continue to fail. I included steps to reproduce this scenario in my previous
proposal patch:
https://lore.kernel.org/linux-f2fs-devel/20231030094024.263707-1-bo.wu@xxxxxxxx/t/#u

However, this issue can't be reproduced on latest kernel.
Seems this commit prevents the creation of non-segment size pinned files and
also avoids this scenario.
3fdd89b452c2 (f2fs: prevent writing without fallocate() for pinned files)

> 
> > 
> > I'm fine w/ this patch, but one another quick fix will be triggering
> > background GC via f2fs ioctl after fallocate() failure, once
> > has_not_enough_free_secs(, ovp_segs) returns false, fallocate() will
> > succeed.
> 
> > 
> > Reviewed-by: Chao Yu <chao@xxxxxxxxxx>
> > 
> > Thanks,
> > 
> > > 
> > > Thanks
> > > 
> > > > 
> > > > Thanks,
> > > > 
> > > > > 
> > > > > > 
> > > > 
> > > > 
> > > > _______________________________________________
> > > > Linux-f2fs-devel mailing list
> > > > Linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx
> > > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel