All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jaegeuk Kim via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>
To: Chao Yu <chao@kernel.org>
Cc: Wu Bo <wubo.oduw@gmail.com>,
	stable@vger.kernel.org, linux-kernel@vger.kernel.org,
	Wu Bo <bo.wu@vivo.com>,
	linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH] Revert "f2fs: stop allocating pinned sections if EAGAIN happens"
Date: Fri, 6 Sep 2024 22:52:47 +0000	[thread overview]
Message-ID: <ZtuHv9ZbCxLmzuZp@google.com> (raw)
In-Reply-To: <d5505e7f-19db-44dd-8c3f-5b43cfff6b29@kernel.org>

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?

> 
> 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@kernel.org>
> 
> Thanks,
> 
> > 
> > Thanks
> > 
> > > 
> > > Thanks,
> > > 
> > > > 
> > > > > 
> > > 
> > > 
> > > _______________________________________________
> > > Linux-f2fs-devel mailing list
> > > Linux-f2fs-devel@lists.sourceforge.net
> > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

WARNING: multiple messages have this Message-ID (diff)
From: Jaegeuk Kim <jaegeuk@kernel.org>
To: Chao Yu <chao@kernel.org>
Cc: Wu Bo <bo.wu@vivo.com>, Wu Bo <wubo.oduw@gmail.com>,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net
Subject: Re: [f2fs-dev] [PATCH] Revert "f2fs: stop allocating pinned sections if EAGAIN happens"
Date: Fri, 6 Sep 2024 22:52:47 +0000	[thread overview]
Message-ID: <ZtuHv9ZbCxLmzuZp@google.com> (raw)
In-Reply-To: <d5505e7f-19db-44dd-8c3f-5b43cfff6b29@kernel.org>

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?

> 
> 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@kernel.org>
> 
> Thanks,
> 
> > 
> > Thanks
> > 
> > > 
> > > Thanks,
> > > 
> > > > 
> > > > > 
> > > 
> > > 
> > > _______________________________________________
> > > Linux-f2fs-devel mailing list
> > > Linux-f2fs-devel@lists.sourceforge.net
> > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

  reply	other threads:[~2024-09-06 22:53 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-05  3:14 [f2fs-dev] [PATCH] Revert "f2fs: stop allocating pinned sections if EAGAIN happens" Wu Bo via Linux-f2fs-devel
2024-02-05  3:14 ` Wu Bo
2024-02-05  3:54 ` [f2fs-dev] " Chao Yu
2024-02-05  3:54   ` Chao Yu
2024-02-08  8:11   ` [f2fs-dev] " Wu Bo
2024-02-08  8:11     ` Wu Bo
2024-02-20  6:50     ` [f2fs-dev] " Chao Yu
2024-02-20  6:50       ` Chao Yu
2024-09-06  8:31       ` [f2fs-dev] " Wu Bo via Linux-f2fs-devel
2024-09-06  8:31         ` Wu Bo
2024-09-06 10:07         ` Chao Yu via Linux-f2fs-devel
2024-09-06 10:07           ` Chao Yu
2024-09-06 22:52           ` Jaegeuk Kim via Linux-f2fs-devel [this message]
2024-09-06 22:52             ` Jaegeuk Kim
2024-09-10  8:33             ` Wu Bo via Linux-f2fs-devel
2024-09-10  8:33               ` Wu Bo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZtuHv9ZbCxLmzuZp@google.com \
    --to=linux-f2fs-devel@lists.sourceforge.net \
    --cc=bo.wu@vivo.com \
    --cc=chao@kernel.org \
    --cc=jaegeuk@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=wubo.oduw@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.