Received-SPF: pass (google.com: domain of linux-kernel+bounces-90015-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <1709292976-13118-1-git-send-email-zhiguo.niu@unisoc.com> <7dc371ad-2448-4dd4-9551-8caef0a00d48@kernel.org>
In-Reply-To: <7dc371ad-2448-4dd4-9551-8caef0a00d48@kernel.org>
From: Zhiguo Niu <niuzhiguo84@gmail.com>
Date: Mon, 4 Mar 2024 11:33:02 +0800
Message-ID: <CAHJ8P3+ZBuBu9Sdpm_6fxhY2sVyu97dXFKKLdtEF4shi_3Fs1Q@mail.gmail.com>
Subject: Re: [PATCH] f2fs: fix to check result of new_curseg in f2fs_allocate_segment_for_resize
To: Chao Yu <chao@kernel.org>
Cc: Zhiguo Niu <zhiguo.niu@unisoc.com>, jaegeuk@kernel.org, 
	linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, 
	ke.wang@unisoc.com, hongyu.jin@unisoc.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Mar 4, 2024 at 11:19=E2=80=AFAM Chao Yu <chao@kernel.org> wrote:
>
> On 2024/3/1 19:36, Zhiguo Niu wrote:
> > new_curseg may return error if get_new_segment fail, so its result
> > should be check in its caller f2fs_allocate_segment_for_resize,
> > alos pass this results to free_segment_range.
>
> Zhiguo,
>
> What about handling all error paths of new_curseg() and change_curseg()
> in one patch?
Dear Chao,

Do you mean to merge it with the previous patch =E2=80=9Cf2fs: fix to check
return value of f2fs_gc_range=E2=80=9D?
Because in addition to new_curseg/change_curseg error handling, there
are some other changes in the previous patch.
besides, I searched for new related codes, and there should be the
only place left without error handling about new_curseg/
change_curseg .

thanks!
>
> Thanks,
>
> >
> > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com>
> > ---
> >   fs/f2fs/f2fs.h    | 2 +-
> >   fs/f2fs/gc.c      | 7 +++++--
> >   fs/f2fs/segment.c | 9 +++++++--
> >   3 files changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> > index 4331012..39dda7d 100644
> > --- a/fs/f2fs/f2fs.h
> > +++ b/fs/f2fs/f2fs.h
> > @@ -3701,7 +3701,7 @@ void f2fs_clear_prefree_segments(struct f2fs_sb_i=
nfo *sbi,
> >   void f2fs_init_inmem_curseg(struct f2fs_sb_info *sbi);
> >   void f2fs_save_inmem_curseg(struct f2fs_sb_info *sbi);
> >   void f2fs_restore_inmem_curseg(struct f2fs_sb_info *sbi);
> > -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int ty=
pe,
> > +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int typ=
e,
> >                                       unsigned int start, unsigned int =
end);
> >   int f2fs_allocate_new_section(struct f2fs_sb_info *sbi, int type, boo=
l force);
> >   int f2fs_allocate_pinning_section(struct f2fs_sb_info *sbi);
> > diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> > index c60b747..7a458fa 100644
> > --- a/fs/f2fs/gc.c
> > +++ b/fs/f2fs/gc.c
> > @@ -2037,8 +2037,11 @@ static int free_segment_range(struct f2fs_sb_inf=
o *sbi,
> >       mutex_unlock(&DIRTY_I(sbi)->seglist_lock);
> >
> >       /* Move out cursegs from the target range */
> > -     for (type =3D CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; typ=
e++)
> > -             f2fs_allocate_segment_for_resize(sbi, type, start, end);
> > +     for (type =3D CURSEG_HOT_DATA; type < NR_CURSEG_PERSIST_TYPE; typ=
e++) {
> > +             err =3D f2fs_allocate_segment_for_resize(sbi, type, start=
, end);
> > +             if (err)
> > +                     goto out;
> > +     }
> >
> >       /* do GC to move out valid blocks in the range */
> >       err =3D f2fs_gc_range(sbi, start, end, dry_run, 0);
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 1bb3019..2a07b9d 100644
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -3071,11 +3071,12 @@ static bool need_new_seg(struct f2fs_sb_info *s=
bi, int type)
> >       return false;
> >   }
> >
> > -void f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int ty=
pe,
> > +int f2fs_allocate_segment_for_resize(struct f2fs_sb_info *sbi, int typ=
e,
> >                                       unsigned int start, unsigned int =
end)
> >   {
> >       struct curseg_info *curseg =3D CURSEG_I(sbi, type);
> >       unsigned int segno;
> > +     int err =3D 0;
> >
> >       f2fs_down_read(&SM_I(sbi)->curseg_lock);
> >       mutex_lock(&curseg->curseg_mutex);
> > @@ -3089,7 +3090,10 @@ void f2fs_allocate_segment_for_resize(struct f2f=
s_sb_info *sbi, int type,
> >               change_curseg(sbi, type);
> >       else
> >               new_curseg(sbi, type, true);
> > -
> > +     if (curseg->segno =3D=3D NULL_SEGNO) {
> > +             err =3D -ENOSPC;
> > +             goto unlock;
> > +     }
> >       stat_inc_seg_type(sbi, curseg);
> >
> >       locate_dirty_segment(sbi, segno);
> > @@ -3102,6 +3106,7 @@ void f2fs_allocate_segment_for_resize(struct f2fs=
_sb_info *sbi, int type,
> >
> >       mutex_unlock(&curseg->curseg_mutex);
> >       f2fs_up_read(&SM_I(sbi)->curseg_lock);
> > +     return err;
> >   }
> >
> >   static int __allocate_new_segment(struct f2fs_sb_info *sbi, int type,