[f2fs-dev] [PATCH] f2fs: Fix type of section block count variables

[f2fs-dev] [PATCH] f2fs: Fix type of section block count variables

[Shin'ichiro Kawasaki]

Commit da52f8ade40b ("f2fs: get the right gc victim section when section
has several segments") added code to count blocks of each section using
variables with type 'unsigned short', which has 2 bytes size in many
systems. However, the counts can be larger than the 2 bytes range and
type conversion results in wrong values. Especially when the f2fs
sections have blocks as many as USHRT_MAX, the count is handled as zero.
This triggers eternal loop in init_dirty_segmap() at mount system call.
Fix this by changing the type of the variables to block_t.

Fixes: da52f8ade40b ("f2fs: get the right gc victim section when section has several segments")
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
---
 fs/f2fs/segment.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index a65d357f89a9..e247a5ef3713 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -799,7 +799,7 @@ static void __locate_dirty_segment(struct f2fs_sb_info *sbi, unsigned int segno,
 
 		if (__is_large_section(sbi)) {
 			unsigned int secno = GET_SEC_FROM_SEG(sbi, segno);
-			unsigned short valid_blocks =
+			block_t valid_blocks =
 				get_valid_blocks(sbi, segno, true);
 
 			f2fs_bug_on(sbi, unlikely(!valid_blocks ||
@@ -815,7 +815,7 @@ static void __remove_dirty_segment(struct f2fs_sb_info *sbi, unsigned int segno,
 		enum dirty_type dirty_type)
 {
 	struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
-	unsigned short valid_blocks;
+	block_t valid_blocks;
 
 	if (test_and_clear_bit(segno, dirty_i->dirty_segmap[dirty_type]))
 		dirty_i->nr_dirty[dirty_type]--;
@@ -4316,8 +4316,8 @@ static void init_dirty_segmap(struct f2fs_sb_info *sbi)
 	struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
 	struct free_segmap_info *free_i = FREE_I(sbi);
 	unsigned int segno = 0, offset = 0, secno;
-	unsigned short valid_blocks;
-	unsigned short blks_per_sec = BLKS_PER_SEC(sbi);
+	block_t valid_blocks;
+	block_t blks_per_sec = BLKS_PER_SEC(sbi);
 
 	while (1) {
 		/* find dirty segment based on free segmap */
-- 
2.26.2



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

Re: [f2fs-dev] [PATCH] f2fs: Fix type of section block count variables

[Chao Yu]

On 2020/8/19 9:34, Shin'ichiro Kawasaki wrote:
> Commit da52f8ade40b ("f2fs: get the right gc victim section when section
> has several segments") added code to count blocks of each section using
> variables with type 'unsigned short', which has 2 bytes size in many
> systems. However, the counts can be larger than the 2 bytes range and
> type conversion results in wrong values. Especially when the f2fs
> sections have blocks as many as USHRT_MAX, the count is handled as zero.
> This triggers eternal loop in init_dirty_segmap() at mount system call.
> Fix this by changing the type of the variables to block_t.
> 
> Fixes: da52f8ade40b ("f2fs: get the right gc victim section when section has several segments")
> Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,


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

Re: [f2fs-dev] [PATCH] f2fs: Fix type of section block count variables

[Shinichiro Kawasaki]

On Aug 19, 2020 / 10:34, Shin'ichiro Kawasaki wrote:
> Commit da52f8ade40b ("f2fs: get the right gc victim section when section
> has several segments") added code to count blocks of each section using
> variables with type 'unsigned short', which has 2 bytes size in many
> systems. However, the counts can be larger than the 2 bytes range and
> type conversion results in wrong values. Especially when the f2fs
> sections have blocks as many as USHRT_MAX, the count is handled as zero.

I made a mistake in the commit message line above. Not 'USHRT_MAX' but
'UHSRT_MAX + 1' is the number to be zero after type cast to unsinged short.

Jaegeuk, if the patch is good enough for you to pick up, could you help to
edit the line as follows?

> sections have blocks as many as USHRT_MAX + 1, the count is handled as 0.
>                                          ^^^^                          ^

In case this patch needs update to v2, I will update the commit message.
Sorry about this confusion.

-- 
Best Regards,
Shin'ichiro Kawasaki

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

Re: [f2fs-dev] [PATCH] f2fs: Fix type of section block count variables

[Jaegeuk Kim]

On 08/19, Shinichiro Kawasaki wrote:
> On Aug 19, 2020 / 10:34, Shin'ichiro Kawasaki wrote:
> > Commit da52f8ade40b ("f2fs: get the right gc victim section when section
> > has several segments") added code to count blocks of each section using
> > variables with type 'unsigned short', which has 2 bytes size in many
> > systems. However, the counts can be larger than the 2 bytes range and
> > type conversion results in wrong values. Especially when the f2fs
> > sections have blocks as many as USHRT_MAX, the count is handled as zero.
> 
> I made a mistake in the commit message line above. Not 'USHRT_MAX' but
> 'UHSRT_MAX + 1' is the number to be zero after type cast to unsinged short.
> 
> Jaegeuk, if the patch is good enough for you to pick up, could you help to
> edit the line as follows?
> 
> > sections have blocks as many as USHRT_MAX + 1, the count is handled as 0.
> >                                          ^^^^                          ^
> 
> In case this patch needs update to v2, I will update the commit message.
> Sorry about this confusion.

No worries. I applied the above change. Thanks,

> 
> -- 
> Best Regards,
> Shin'ichiro Kawasaki


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