[PATCH] f2fs: fix incorrect return value in f2fs_sanity_check_ckpt()

[PATCH] f2fs: fix incorrect return value in f2fs_sanity_check_ckpt()

[Chao Yu]

As Pavel Machek reported in [1]

This code looks quite confused: part of function returns 1 on
corruption, part returns -errno. The problem is not stable-specific.

[1] https://lkml.org/lkml/2021/9/19/207

Let's fix to make 'insane cp_payload case' to return 1 rater than
EFSCORRUPTED, so that return value can be kept consistent for all
error cases, it can avoid confusion of code logic.

Fixes: 65ddf6564843 ("f2fs: fix to do sanity check for sb/cp fields correctly")
Reported-by: Pavel Machek <pavel@denx.de>
Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 8d6d0657a470..e3975cb8e3e8 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3487,7 +3487,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
 		NR_CURSEG_PERSIST_TYPE + nat_bits_blocks >= blocks_per_seg)) {
 		f2fs_warn(sbi, "Insane cp_payload: %u, nat_bits_blocks: %u)",
 			  cp_payload, nat_bits_blocks);
-		return -EFSCORRUPTED;
+		return 1;
 	}
 
 	if (unlikely(f2fs_cp_error(sbi))) {
-- 
2.32.0

[f2fs-dev] [PATCH] f2fs: fix incorrect return value in f2fs_sanity_check_ckpt()

[Chao Yu]

As Pavel Machek reported in [1]

This code looks quite confused: part of function returns 1 on
corruption, part returns -errno. The problem is not stable-specific.

[1] https://lkml.org/lkml/2021/9/19/207

Let's fix to make 'insane cp_payload case' to return 1 rater than
EFSCORRUPTED, so that return value can be kept consistent for all
error cases, it can avoid confusion of code logic.

Fixes: 65ddf6564843 ("f2fs: fix to do sanity check for sb/cp fields correctly")
Reported-by: Pavel Machek <pavel@denx.de>
Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/super.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 8d6d0657a470..e3975cb8e3e8 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -3487,7 +3487,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
 		NR_CURSEG_PERSIST_TYPE + nat_bits_blocks >= blocks_per_seg)) {
 		f2fs_warn(sbi, "Insane cp_payload: %u, nat_bits_blocks: %u)",
 			  cp_payload, nat_bits_blocks);
-		return -EFSCORRUPTED;
+		return 1;
 	}
 
 	if (unlikely(f2fs_cp_error(sbi))) {
-- 
2.32.0



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

Re: [PATCH] f2fs: fix incorrect return value in f2fs_sanity_check_ckpt()

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1308 bytes --]

Hi!

> This code looks quite confused: part of function returns 1 on
> corruption, part returns -errno. The problem is not stable-specific.
> 
> [1] https://lkml.org/lkml/2021/9/19/207
> 
> Let's fix to make 'insane cp_payload case' to return 1 rater than
> EFSCORRUPTED, so that return value can be kept consistent for all
> error cases, it can avoid confusion of code logic.
> 
> Fixes: 65ddf6564843 ("f2fs: fix to do sanity check for sb/cp fields correctly")
> Reported-by: Pavel Machek <pavel@denx.de>
> Signed-off-by: Chao Yu <chao@kernel.org>

Reviewed-by: Pavel Machek <pavel@denx.de>

(This is good minimal fix, but eventually I believe the function
should switch to 0/-errno... for consistency with rest of kernel).

Thank you,
								Pavel
								
> +++ b/fs/f2fs/super.c
> @@ -3487,7 +3487,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
>  		NR_CURSEG_PERSIST_TYPE + nat_bits_blocks >= blocks_per_seg)) {
>  		f2fs_warn(sbi, "Insane cp_payload: %u, nat_bits_blocks: %u)",
>  			  cp_payload, nat_bits_blocks);
> -		return -EFSCORRUPTED;
> +		return 1;
>  	}
>  
>  	if (unlikely(f2fs_cp_error(sbi))) {

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH] f2fs: fix incorrect return value in f2fs_sanity_check_ckpt()

[Chao Yu]

Jaegeuk,

Missed to apply this patch?

Thanks,

On 2021/9/24 17:50, Pavel Machek wrote:
> Hi!
> 
>> This code looks quite confused: part of function returns 1 on
>> corruption, part returns -errno. The problem is not stable-specific.
>>
>> [1] https://lkml.org/lkml/2021/9/19/207
>>
>> Let's fix to make 'insane cp_payload case' to return 1 rater than
>> EFSCORRUPTED, so that return value can be kept consistent for all
>> error cases, it can avoid confusion of code logic.
>>
>> Fixes: 65ddf6564843 ("f2fs: fix to do sanity check for sb/cp fields correctly")
>> Reported-by: Pavel Machek <pavel@denx.de>
>> Signed-off-by: Chao Yu <chao@kernel.org>
> 
> Reviewed-by: Pavel Machek <pavel@denx.de>
> 
> (This is good minimal fix, but eventually I believe the function
> should switch to 0/-errno... for consistency with rest of kernel).
> 
> Thank you,
> 								Pavel
> 								
>> +++ b/fs/f2fs/super.c
>> @@ -3487,7 +3487,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
>>   		NR_CURSEG_PERSIST_TYPE + nat_bits_blocks >= blocks_per_seg)) {
>>   		f2fs_warn(sbi, "Insane cp_payload: %u, nat_bits_blocks: %u)",
>>   			  cp_payload, nat_bits_blocks);
>> -		return -EFSCORRUPTED;
>> +		return 1;
>>   	}
>>   
>>   	if (unlikely(f2fs_cp_error(sbi))) {
> 

Re: [f2fs-dev] [PATCH] f2fs: fix incorrect return value in f2fs_sanity_check_ckpt()

[Chao Yu]

Jaegeuk,

Missed to apply this patch?

Thanks,

On 2021/9/24 17:50, Pavel Machek wrote:
> Hi!
> 
>> This code looks quite confused: part of function returns 1 on
>> corruption, part returns -errno. The problem is not stable-specific.
>>
>> [1] https://lkml.org/lkml/2021/9/19/207
>>
>> Let's fix to make 'insane cp_payload case' to return 1 rater than
>> EFSCORRUPTED, so that return value can be kept consistent for all
>> error cases, it can avoid confusion of code logic.
>>
>> Fixes: 65ddf6564843 ("f2fs: fix to do sanity check for sb/cp fields correctly")
>> Reported-by: Pavel Machek <pavel@denx.de>
>> Signed-off-by: Chao Yu <chao@kernel.org>
> 
> Reviewed-by: Pavel Machek <pavel@denx.de>
> 
> (This is good minimal fix, but eventually I believe the function
> should switch to 0/-errno... for consistency with rest of kernel).
> 
> Thank you,
> 								Pavel
> 								
>> +++ b/fs/f2fs/super.c
>> @@ -3487,7 +3487,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
>>   		NR_CURSEG_PERSIST_TYPE + nat_bits_blocks >= blocks_per_seg)) {
>>   		f2fs_warn(sbi, "Insane cp_payload: %u, nat_bits_blocks: %u)",
>>   			  cp_payload, nat_bits_blocks);
>> -		return -EFSCORRUPTED;
>> +		return 1;
>>   	}
>>   
>>   	if (unlikely(f2fs_cp_error(sbi))) {
> 


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

Re: [PATCH] f2fs: fix incorrect return value in f2fs_sanity_check_ckpt()

[Jaegeuk Kim]

Could you post the patch again? I don't see this in my box.

On 10/27, Chao Yu wrote:
> Jaegeuk,
> 
> Missed to apply this patch?
> 
> Thanks,
> 
> On 2021/9/24 17:50, Pavel Machek wrote:
> > Hi!
> > 
> > > This code looks quite confused: part of function returns 1 on
> > > corruption, part returns -errno. The problem is not stable-specific.
> > > 
> > > [1] https://lkml.org/lkml/2021/9/19/207
> > > 
> > > Let's fix to make 'insane cp_payload case' to return 1 rater than
> > > EFSCORRUPTED, so that return value can be kept consistent for all
> > > error cases, it can avoid confusion of code logic.
> > > 
> > > Fixes: 65ddf6564843 ("f2fs: fix to do sanity check for sb/cp fields correctly")
> > > Reported-by: Pavel Machek <pavel@denx.de>
> > > Signed-off-by: Chao Yu <chao@kernel.org>
> > 
> > Reviewed-by: Pavel Machek <pavel@denx.de>
> > 
> > (This is good minimal fix, but eventually I believe the function
> > should switch to 0/-errno... for consistency with rest of kernel).
> > 
> > Thank you,
> > 								Pavel
> > 								
> > > +++ b/fs/f2fs/super.c
> > > @@ -3487,7 +3487,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> > >   		NR_CURSEG_PERSIST_TYPE + nat_bits_blocks >= blocks_per_seg)) {
> > >   		f2fs_warn(sbi, "Insane cp_payload: %u, nat_bits_blocks: %u)",
> > >   			  cp_payload, nat_bits_blocks);
> > > -		return -EFSCORRUPTED;
> > > +		return 1;
> > >   	}
> > >   	if (unlikely(f2fs_cp_error(sbi))) {
> > 

Re: [f2fs-dev] [PATCH] f2fs: fix incorrect return value in f2fs_sanity_check_ckpt()

[Jaegeuk Kim]

Could you post the patch again? I don't see this in my box.

On 10/27, Chao Yu wrote:
> Jaegeuk,
> 
> Missed to apply this patch?
> 
> Thanks,
> 
> On 2021/9/24 17:50, Pavel Machek wrote:
> > Hi!
> > 
> > > This code looks quite confused: part of function returns 1 on
> > > corruption, part returns -errno. The problem is not stable-specific.
> > > 
> > > [1] https://lkml.org/lkml/2021/9/19/207
> > > 
> > > Let's fix to make 'insane cp_payload case' to return 1 rater than
> > > EFSCORRUPTED, so that return value can be kept consistent for all
> > > error cases, it can avoid confusion of code logic.
> > > 
> > > Fixes: 65ddf6564843 ("f2fs: fix to do sanity check for sb/cp fields correctly")
> > > Reported-by: Pavel Machek <pavel@denx.de>
> > > Signed-off-by: Chao Yu <chao@kernel.org>
> > 
> > Reviewed-by: Pavel Machek <pavel@denx.de>
> > 
> > (This is good minimal fix, but eventually I believe the function
> > should switch to 0/-errno... for consistency with rest of kernel).
> > 
> > Thank you,
> > 								Pavel
> > 								
> > > +++ b/fs/f2fs/super.c
> > > @@ -3487,7 +3487,7 @@ int f2fs_sanity_check_ckpt(struct f2fs_sb_info *sbi)
> > >   		NR_CURSEG_PERSIST_TYPE + nat_bits_blocks >= blocks_per_seg)) {
> > >   		f2fs_warn(sbi, "Insane cp_payload: %u, nat_bits_blocks: %u)",
> > >   			  cp_payload, nat_bits_blocks);
> > > -		return -EFSCORRUPTED;
> > > +		return 1;
> > >   	}
> > >   	if (unlikely(f2fs_cp_error(sbi))) {
> > 


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