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 <[email protected]>
Signed-off-by: Chao Yu <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: Chao Yu <[email protected]>
Reviewed-by: Pavel Machek <[email protected]>
(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
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 <[email protected]>
>> Signed-off-by: Chao Yu <[email protected]>
>
> Reviewed-by: Pavel Machek <[email protected]>
>
> (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))) {
>
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 <[email protected]>
> > > Signed-off-by: Chao Yu <[email protected]>
> >
> > Reviewed-by: Pavel Machek <[email protected]>
> >
> > (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))) {
> >