| Message ID | 20230210213250.3471246-1-jaegeuk@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [f2fs-dev] f2fs: fix wrong segment count | expand |
On 2023/2/11 5:32, Jaegeuk Kim wrote: > MAIN_SEGS is for data area, while TOTAL_SEGS includes data and metadata. Good catch! Could you please add fixes line? Thanks, > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > --- > fs/f2fs/segment.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h > index 8ee5e5db9287..6003fbaf4b7d 100644 > --- a/fs/f2fs/segment.h > +++ b/fs/f2fs/segment.h > @@ -720,7 +720,7 @@ static inline unsigned char curseg_alloc_type(struct f2fs_sb_info *sbi, > > static inline void check_seg_range(struct f2fs_sb_info *sbi, unsigned int segno) > { > - f2fs_bug_on(sbi, segno > TOTAL_SEGS(sbi) - 1); > + f2fs_bug_on(sbi, segno > MAIN_SEGS(sbi) - 1); > } > > static inline void verify_fio_blkaddr(struct f2fs_io_info *fio) > @@ -775,7 +775,7 @@ static inline int check_block_count(struct f2fs_sb_info *sbi, > > /* check segment usage, and check boundary of a given segment number */ > if (unlikely(GET_SIT_VBLOCKS(raw_sit) > usable_blks_per_seg > - || segno > TOTAL_SEGS(sbi) - 1)) { > + || segno > MAIN_SEGS(sbi) - 1)) { > f2fs_err(sbi, "Wrong valid blocks %d or segno %u", > GET_SIT_VBLOCKS(raw_sit), segno); > set_sbi_flag(sbi, SBI_NEED_FSCK);
On 02/13, Chao Yu wrote: > On 2023/2/11 5:32, Jaegeuk Kim wrote: > > MAIN_SEGS is for data area, while TOTAL_SEGS includes data and metadata. > > Good catch! > > Could you please add fixes line? It seems this is not a bug case, and exisits from the first F2FS patch. :) > > Thanks, > > > > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > > --- > > fs/f2fs/segment.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h > > index 8ee5e5db9287..6003fbaf4b7d 100644 > > --- a/fs/f2fs/segment.h > > +++ b/fs/f2fs/segment.h > > @@ -720,7 +720,7 @@ static inline unsigned char curseg_alloc_type(struct f2fs_sb_info *sbi, > > static inline void check_seg_range(struct f2fs_sb_info *sbi, unsigned int segno) > > { > > - f2fs_bug_on(sbi, segno > TOTAL_SEGS(sbi) - 1); > > + f2fs_bug_on(sbi, segno > MAIN_SEGS(sbi) - 1); > > } > > static inline void verify_fio_blkaddr(struct f2fs_io_info *fio) > > @@ -775,7 +775,7 @@ static inline int check_block_count(struct f2fs_sb_info *sbi, > > /* check segment usage, and check boundary of a given segment number */ > > if (unlikely(GET_SIT_VBLOCKS(raw_sit) > usable_blks_per_seg > > - || segno > TOTAL_SEGS(sbi) - 1)) { > > + || segno > MAIN_SEGS(sbi) - 1)) { > > f2fs_err(sbi, "Wrong valid blocks %d or segno %u", > > GET_SIT_VBLOCKS(raw_sit), segno); > > set_sbi_flag(sbi, SBI_NEED_FSCK);
MAIN_SEGS is for data area, while TOTAL_SEGS includes data and metadata.
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
Change log from v1:
- replace check_seg_range with valid_main_segno to avoid confusion.
fs/f2fs/segment.h | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 92c8be00d396..efdb7fc3b797 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -719,9 +719,10 @@ static inline unsigned char curseg_alloc_type(struct f2fs_sb_info *sbi,
return curseg->alloc_type;
}
-static inline void check_seg_range(struct f2fs_sb_info *sbi, unsigned int segno)
+static inline bool valid_main_segno(struct f2fs_sb_info *sbi,
+ unsigned int segno)
{
- f2fs_bug_on(sbi, segno > TOTAL_SEGS(sbi) - 1);
+ return segno <= (MAIN_SEGS(sbi) - 1);
}
static inline void verify_fio_blkaddr(struct f2fs_io_info *fio)
@@ -776,7 +777,7 @@ static inline int check_block_count(struct f2fs_sb_info *sbi,
/* check segment usage, and check boundary of a given segment number */
if (unlikely(GET_SIT_VBLOCKS(raw_sit) > usable_blks_per_seg
- || segno > TOTAL_SEGS(sbi) - 1)) {
+ || !valid_main_segno(sbi, segno))) {
f2fs_err(sbi, "Wrong valid blocks %d or segno %u",
GET_SIT_VBLOCKS(raw_sit), segno);
set_sbi_flag(sbi, SBI_NEED_FSCK);
@@ -793,7 +794,7 @@ static inline pgoff_t current_sit_addr(struct f2fs_sb_info *sbi,
unsigned int offset = SIT_BLOCK_OFFSET(start);
block_t blk_addr = sit_i->sit_base_addr + offset;
- check_seg_range(sbi, start);
+ f2fs_bug_on(sbi, !valid_main_segno(sbi, start));
#ifdef CONFIG_F2FS_CHECK_FS
if (f2fs_test_bit(offset, sit_i->sit_bitmap) !=
On 2023/2/14 1:48, Jaegeuk Kim wrote: > On 02/13, Chao Yu wrote: >> On 2023/2/11 5:32, Jaegeuk Kim wrote: >>> MAIN_SEGS is for data area, while TOTAL_SEGS includes data and metadata. >> >> Good catch! >> >> Could you please add fixes line? > > It seems this is not a bug case, and exisits from the first F2FS patch. :) Alright, anyway, it looks good to me. Reviewed-by: Chao Yu <chao@kernel.org> Thanks, > >> >> Thanks, >> >>> >>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> >>> --- >>> fs/f2fs/segment.h | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h >>> index 8ee5e5db9287..6003fbaf4b7d 100644 >>> --- a/fs/f2fs/segment.h >>> +++ b/fs/f2fs/segment.h >>> @@ -720,7 +720,7 @@ static inline unsigned char curseg_alloc_type(struct f2fs_sb_info *sbi, >>> static inline void check_seg_range(struct f2fs_sb_info *sbi, unsigned int segno) >>> { >>> - f2fs_bug_on(sbi, segno > TOTAL_SEGS(sbi) - 1); >>> + f2fs_bug_on(sbi, segno > MAIN_SEGS(sbi) - 1); >>> } >>> static inline void verify_fio_blkaddr(struct f2fs_io_info *fio) >>> @@ -775,7 +775,7 @@ static inline int check_block_count(struct f2fs_sb_info *sbi, >>> /* check segment usage, and check boundary of a given segment number */ >>> if (unlikely(GET_SIT_VBLOCKS(raw_sit) > usable_blks_per_seg >>> - || segno > TOTAL_SEGS(sbi) - 1)) { >>> + || segno > MAIN_SEGS(sbi) - 1)) { >>> f2fs_err(sbi, "Wrong valid blocks %d or segno %u", >>> GET_SIT_VBLOCKS(raw_sit), segno); >>> set_sbi_flag(sbi, SBI_NEED_FSCK);
On 02/14, Chao Yu wrote: > On 2023/2/14 1:48, Jaegeuk Kim wrote: > > On 02/13, Chao Yu wrote: > > > On 2023/2/11 5:32, Jaegeuk Kim wrote: > > > > MAIN_SEGS is for data area, while TOTAL_SEGS includes data and metadata. > > > > > > Good catch! > > > > > > Could you please add fixes line? > > > > It seems this is not a bug case, and exisits from the first F2FS patch. :) > > Alright, anyway, it looks good to me. > > Reviewed-by: Chao Yu <chao@kernel.org> I assumed this for v2. Let me know if you have other concern. > > Thanks, > > > > > > > > > Thanks, > > > > > > > > > > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > > > > --- > > > > fs/f2fs/segment.h | 4 ++-- > > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h > > > > index 8ee5e5db9287..6003fbaf4b7d 100644 > > > > --- a/fs/f2fs/segment.h > > > > +++ b/fs/f2fs/segment.h > > > > @@ -720,7 +720,7 @@ static inline unsigned char curseg_alloc_type(struct f2fs_sb_info *sbi, > > > > static inline void check_seg_range(struct f2fs_sb_info *sbi, unsigned int segno) > > > > { > > > > - f2fs_bug_on(sbi, segno > TOTAL_SEGS(sbi) - 1); > > > > + f2fs_bug_on(sbi, segno > MAIN_SEGS(sbi) - 1); > > > > } > > > > static inline void verify_fio_blkaddr(struct f2fs_io_info *fio) > > > > @@ -775,7 +775,7 @@ static inline int check_block_count(struct f2fs_sb_info *sbi, > > > > /* check segment usage, and check boundary of a given segment number */ > > > > if (unlikely(GET_SIT_VBLOCKS(raw_sit) > usable_blks_per_seg > > > > - || segno > TOTAL_SEGS(sbi) - 1)) { > > > > + || segno > MAIN_SEGS(sbi) - 1)) { > > > > f2fs_err(sbi, "Wrong valid blocks %d or segno %u", > > > > GET_SIT_VBLOCKS(raw_sit), segno); > > > > set_sbi_flag(sbi, SBI_NEED_FSCK);
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h index 8ee5e5db9287..6003fbaf4b7d 100644 --- a/fs/f2fs/segment.h +++ b/fs/f2fs/segment.h @@ -720,7 +720,7 @@ static inline unsigned char curseg_alloc_type(struct f2fs_sb_info *sbi, static inline void check_seg_range(struct f2fs_sb_info *sbi, unsigned int segno) { - f2fs_bug_on(sbi, segno > TOTAL_SEGS(sbi) - 1); + f2fs_bug_on(sbi, segno > MAIN_SEGS(sbi) - 1); } static inline void verify_fio_blkaddr(struct f2fs_io_info *fio) @@ -775,7 +775,7 @@ static inline int check_block_count(struct f2fs_sb_info *sbi, /* check segment usage, and check boundary of a given segment number */ if (unlikely(GET_SIT_VBLOCKS(raw_sit) > usable_blks_per_seg - || segno > TOTAL_SEGS(sbi) - 1)) { + || segno > MAIN_SEGS(sbi) - 1)) { f2fs_err(sbi, "Wrong valid blocks %d or segno %u", GET_SIT_VBLOCKS(raw_sit), segno); set_sbi_flag(sbi, SBI_NEED_FSCK);
MAIN_SEGS is for data area, while TOTAL_SEGS includes data and metadata. Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> --- fs/f2fs/segment.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)