| Message ID | 1715945202-30045-1-git-send-email-zhiguo.niu@unisoc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [f2fs-dev] f2fs: fix to check return value of f2fs_allocate_new_section | expand |
…
> missed this allocated path, fix it.
* Will another imperative wording be desirable for an improved change description?
* Would you like to add the tag “Fixes”?
* How do you think about to append parentheses to the function name
in the summary phrase?
Regards,
Markus
On 2024/5/17 19:26, Zhiguo Niu wrote: > commit 245930617c9b ("f2fs: fix to handle error paths of {new,change}_curseg()") > missed this allocated path, fix it. > > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> Reviewed-by: Chao Yu <chao@kernel.org> Thanks,
On 05/17, Zhiguo Niu wrote: > commit 245930617c9b ("f2fs: fix to handle error paths of {new,change}_curseg()") > missed this allocated path, fix it. > > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > --- > fs/f2fs/segment.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index a0ce3d0..71dc8042 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -5190,7 +5190,9 @@ static int fix_curseg_write_pointer(struct f2fs_sb_info *sbi, int type) > if (cs->next_blkoff) { > unsigned int old_segno = cs->segno, old_blkoff = cs->next_blkoff; > > - f2fs_allocate_new_section(sbi, type, true); > + err = f2fs_allocate_new_section(sbi, type, true); > + if (err) > + return err; I hesitate to apply this, since this may give mount failures forever. Do you see a real issue with this? > f2fs_notice(sbi, "Assign new section to curseg[%d]: " > "[0x%x,0x%x] -> [0x%x,0x%x]", > type, old_segno, old_blkoff, > -- > 1.9.1
On Tue, May 21, 2024 at 3:52 AM Jaegeuk Kim <jaegeuk@kernel.org> wrote: > > On 05/17, Zhiguo Niu wrote: > > commit 245930617c9b ("f2fs: fix to handle error paths of {new,change}_curseg()") > > missed this allocated path, fix it. > > > > Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> > > --- > > fs/f2fs/segment.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > > index a0ce3d0..71dc8042 100644 > > --- a/fs/f2fs/segment.c > > +++ b/fs/f2fs/segment.c > > @@ -5190,7 +5190,9 @@ static int fix_curseg_write_pointer(struct f2fs_sb_info *sbi, int type) > > if (cs->next_blkoff) { > > unsigned int old_segno = cs->segno, old_blkoff = cs->next_blkoff; > > > > - f2fs_allocate_new_section(sbi, type, true); > > + err = f2fs_allocate_new_section(sbi, type, true); > > + if (err) > > + return err; > > I hesitate to apply this, since this may give mount failures forever. Do you see > a real issue with this? Hi Jaegeuk, No I did not have a real issue related to this, just found it by code review. it is ok not to apply this for me . thanks a lot. > > > f2fs_notice(sbi, "Assign new section to curseg[%d]: " > > "[0x%x,0x%x] -> [0x%x,0x%x]", > > type, old_segno, old_blkoff, > > -- > > 1.9.1
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index a0ce3d0..71dc8042 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -5190,7 +5190,9 @@ static int fix_curseg_write_pointer(struct f2fs_sb_info *sbi, int type) if (cs->next_blkoff) { unsigned int old_segno = cs->segno, old_blkoff = cs->next_blkoff; - f2fs_allocate_new_section(sbi, type, true); + err = f2fs_allocate_new_section(sbi, type, true); + if (err) + return err; f2fs_notice(sbi, "Assign new section to curseg[%d]: " "[0x%x,0x%x] -> [0x%x,0x%x]", type, old_segno, old_blkoff,
commit 245930617c9b ("f2fs: fix to handle error paths of {new,change}_curseg()") missed this allocated path, fix it. Signed-off-by: Zhiguo Niu <zhiguo.niu@unisoc.com> --- fs/f2fs/segment.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)