* Re: [f2fs-dev] [PATCH V2] fsck.f2fs: allow fsck to fix issues with online resize due to SPO
[not found] <1583466746-17445-1-git-send-email-stummala@codeaurora.org>
@ 2020-03-16 3:19 ` Chao Yu
2020-03-19 9:15 ` Sahitya Tummala
0 siblings, 1 reply; 4+ messages in thread
From: Chao Yu @ 2020-03-16 3:19 UTC (permalink / raw)
To: Sahitya Tummala, Jaegeuk Kim, linux-f2fs-devel
On 2020/3/6 11:52, Sahitya Tummala wrote:
> Add support for new CP flag CP_RESIZEFS_FLAG set during online
> resize FS. If SPO happens after SB is updated but CP isn't, then
> allow fsck to fix it.
>
> fsck errors without this fix -
> Info: CKPT version = 6ed7bccb
> Wrong user_block_count(2233856)
> [f2fs_do_mount:3365] Checkpoint is polluted
>
> the subsequent mount failure without this fix -
> [ 11.294650] F2FS-fs (sda8): Wrong user_block_count: 2233856
> [ 11.300272] F2FS-fs (sda8): Failed to get valid F2FS checkpoint
>
> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
> ---
> v2:
> - fix even if CP_FSCK_FLAG is set for backward compatibility
> - update print_cp_state()
>
> fsck/mount.c | 34 +++++++++++++++++++++++++++++++---
> include/f2fs_fs.h | 1 +
> 2 files changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/fsck/mount.c b/fsck/mount.c
> index e4ba048..8d32e41 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -429,6 +429,8 @@ void print_cp_state(u32 flag)
> MSG(0, "%s", " orphan_inodes");
> if (flag & CP_DISABLED_FLAG)
> MSG(0, "%s", " disabled");
> + if (flag & CP_RESIZEFS_FLAG)
> + MSG(0, "%s", " resizefs");
> if (flag & CP_UMOUNT_FLAG)
> MSG(0, "%s", " unmount");
> else
> @@ -1128,6 +1130,7 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi)
> unsigned int total, fsmeta;
> struct f2fs_super_block *sb = F2FS_RAW_SUPER(sbi);
> struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
> + unsigned int flag = get_cp(ckpt_flags);
> unsigned int ovp_segments, reserved_segments;
> unsigned int main_segs, blocks_per_seg;
> unsigned int sit_segs, nat_segs;
> @@ -1164,7 +1167,32 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi)
> log_blocks_per_seg = get_sb(log_blocks_per_seg);
> if (!user_block_count || user_block_count >=
> segment_count_main << log_blocks_per_seg) {
> - MSG(0, "\tWrong user_block_count(%u)\n", user_block_count);
> + if (flag & (CP_FSCK_FLAG | CP_RESIZEFS_FLAG)) {
> + u32 valid_user_block_cnt;
> + u32 seg_cnt_main = get_sb(segment_count) -
> + (get_sb(segment_count_ckpt) +
> + get_sb(segment_count_sit) +
> + get_sb(segment_count_nat) +
> + get_sb(segment_count_ssa));
> +
> + /* validate segment_count_main in sb first */
> + if (seg_cnt_main != get_sb(segment_count_main)) {
> + MSG(0, "\tWrong user_block_count(%u) and inconsistent segment_cnt_main %u\n",
> + user_block_count,
> + segment_count_main << log_blocks_per_seg);
> + return 1;
> + }
> + valid_user_block_cnt = ((get_sb(segment_count_main) -
> + get_cp(overprov_segment_count)) * c.blks_per_seg);
> + MSG(0, "\tInfo: Fix wrong user_block_count in CP: (%u) -> (%u)\n",
> + user_block_count, valid_user_block_cnt);
By default, we should only fix such bug if c.fix_on is true, something
like this:
ASSERT_MSG("\tWrong user_block_count(%u)\n", user_block_count);
if (!c.fix_on)
return 1;
valid_user_block_cnt = ((get_sb(segment_count_main) -
get_cp(overprov_segment_count)) * c.blks_per_seg);
MSG(0, "\tInfo: Fix wrong user_block_count in CP: (%u) -> (%u)\n",
user_block_count, valid_user_block_cnt);
Thanks,
> + set_cp(user_block_count, valid_user_block_cnt);
> + c.fix_on = 1;
> + c.bug_on = 1;
> + } else {
> + MSG(0, "\tWrong user_block_count(%u)\n", user_block_count);
> + return 1;
> + }
> return 1;
> }
>
> @@ -3361,6 +3389,8 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
> return -1;
> }
>
> + c.bug_on = 0;
> +
> if (sanity_check_ckpt(sbi)) {
> ERR_MSG("Checkpoint is polluted\n");
> return -1;
> @@ -3380,8 +3410,6 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
> c.fix_on = 1;
> }
>
> - c.bug_on = 0;
> -
> if (tune_sb_features(sbi))
> return -1;
>
> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> index af31bc5..265f50c 100644
> --- a/include/f2fs_fs.h
> +++ b/include/f2fs_fs.h
> @@ -678,6 +678,7 @@ struct f2fs_super_block {
> /*
> * For checkpoint
> */
> +#define CP_RESIZEFS_FLAG 0x00004000
> #define CP_DISABLED_FLAG 0x00001000
> #define CP_QUOTA_NEED_FSCK_FLAG 0x00000800
> #define CP_LARGE_NAT_BITMAP_FLAG 0x00000400
>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [f2fs-dev] [PATCH V2] fsck.f2fs: allow fsck to fix issues with online resize due to SPO
2020-03-16 3:19 ` [f2fs-dev] [PATCH V2] fsck.f2fs: allow fsck to fix issues with online resize due to SPO Chao Yu
@ 2020-03-19 9:15 ` Sahitya Tummala
2020-03-19 10:47 ` Chao Yu
0 siblings, 1 reply; 4+ messages in thread
From: Sahitya Tummala @ 2020-03-19 9:15 UTC (permalink / raw)
To: Chao Yu; +Cc: Jaegeuk Kim, linux-f2fs-devel
Hi Chao,
On Mon, Mar 16, 2020 at 11:19:11AM +0800, Chao Yu wrote:
> On 2020/3/6 11:52, Sahitya Tummala wrote:
> > Add support for new CP flag CP_RESIZEFS_FLAG set during online
> > resize FS. If SPO happens after SB is updated but CP isn't, then
> > allow fsck to fix it.
> >
> > fsck errors without this fix -
> > Info: CKPT version = 6ed7bccb
> > Wrong user_block_count(2233856)
> > [f2fs_do_mount:3365] Checkpoint is polluted
> >
> > the subsequent mount failure without this fix -
> > [ 11.294650] F2FS-fs (sda8): Wrong user_block_count: 2233856
> > [ 11.300272] F2FS-fs (sda8): Failed to get valid F2FS checkpoint
> >
> > Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
> > ---
> > v2:
> > - fix even if CP_FSCK_FLAG is set for backward compatibility
> > - update print_cp_state()
> >
> > fsck/mount.c | 34 +++++++++++++++++++++++++++++++---
> > include/f2fs_fs.h | 1 +
> > 2 files changed, 32 insertions(+), 3 deletions(-)
> >
> > diff --git a/fsck/mount.c b/fsck/mount.c
> > index e4ba048..8d32e41 100644
> > --- a/fsck/mount.c
> > +++ b/fsck/mount.c
> > @@ -429,6 +429,8 @@ void print_cp_state(u32 flag)
> > MSG(0, "%s", " orphan_inodes");
> > if (flag & CP_DISABLED_FLAG)
> > MSG(0, "%s", " disabled");
> > + if (flag & CP_RESIZEFS_FLAG)
> > + MSG(0, "%s", " resizefs");
> > if (flag & CP_UMOUNT_FLAG)
> > MSG(0, "%s", " unmount");
> > else
> > @@ -1128,6 +1130,7 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi)
> > unsigned int total, fsmeta;
> > struct f2fs_super_block *sb = F2FS_RAW_SUPER(sbi);
> > struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
> > + unsigned int flag = get_cp(ckpt_flags);
> > unsigned int ovp_segments, reserved_segments;
> > unsigned int main_segs, blocks_per_seg;
> > unsigned int sit_segs, nat_segs;
> > @@ -1164,7 +1167,32 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi)
> > log_blocks_per_seg = get_sb(log_blocks_per_seg);
> > if (!user_block_count || user_block_count >=
> > segment_count_main << log_blocks_per_seg) {
> > - MSG(0, "\tWrong user_block_count(%u)\n", user_block_count);
> > + if (flag & (CP_FSCK_FLAG | CP_RESIZEFS_FLAG)) {
> > + u32 valid_user_block_cnt;
> > + u32 seg_cnt_main = get_sb(segment_count) -
> > + (get_sb(segment_count_ckpt) +
> > + get_sb(segment_count_sit) +
> > + get_sb(segment_count_nat) +
> > + get_sb(segment_count_ssa));
> > +
> > + /* validate segment_count_main in sb first */
> > + if (seg_cnt_main != get_sb(segment_count_main)) {
> > + MSG(0, "\tWrong user_block_count(%u) and inconsistent segment_cnt_main %u\n",
> > + user_block_count,
> > + segment_count_main << log_blocks_per_seg);
> > + return 1;
> > + }
> > + valid_user_block_cnt = ((get_sb(segment_count_main) -
> > + get_cp(overprov_segment_count)) * c.blks_per_seg);
> > + MSG(0, "\tInfo: Fix wrong user_block_count in CP: (%u) -> (%u)\n",
> > + user_block_count, valid_user_block_cnt);
>
> By default, we should only fix such bug if c.fix_on is true, something
> like this:
>
> ASSERT_MSG("\tWrong user_block_count(%u)\n", user_block_count);
>
> if (!c.fix_on)
> return 1;
>
> valid_user_block_cnt = ((get_sb(segment_count_main) -
> get_cp(overprov_segment_count)) * c.blks_per_seg);
>
> MSG(0, "\tInfo: Fix wrong user_block_count in CP: (%u) -> (%u)\n",
> user_block_count, valid_user_block_cnt);
>
Since this is a fatal error which fails the basic mount itself, I thought it
must be fixed by default with fsck independent of -f option. Can we do so for
such critical bugs?
Thanks,
> Thanks,
>
>
> > + set_cp(user_block_count, valid_user_block_cnt);
> > + c.fix_on = 1;
> > + c.bug_on = 1;
> > + } else {
> > + MSG(0, "\tWrong user_block_count(%u)\n", user_block_count);
> > + return 1;
> > + }
> > return 1;
> > }
> >
> > @@ -3361,6 +3389,8 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
> > return -1;
> > }
> >
> > + c.bug_on = 0;
> > +
> > if (sanity_check_ckpt(sbi)) {
> > ERR_MSG("Checkpoint is polluted\n");
> > return -1;
> > @@ -3380,8 +3410,6 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
> > c.fix_on = 1;
> > }
> >
> > - c.bug_on = 0;
> > -
> > if (tune_sb_features(sbi))
> > return -1;
> >
> > diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> > index af31bc5..265f50c 100644
> > --- a/include/f2fs_fs.h
> > +++ b/include/f2fs_fs.h
> > @@ -678,6 +678,7 @@ struct f2fs_super_block {
> > /*
> > * For checkpoint
> > */
> > +#define CP_RESIZEFS_FLAG 0x00004000
> > #define CP_DISABLED_FLAG 0x00001000
> > #define CP_QUOTA_NEED_FSCK_FLAG 0x00000800
> > #define CP_LARGE_NAT_BITMAP_FLAG 0x00000400
> >
--
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [f2fs-dev] [PATCH V2] fsck.f2fs: allow fsck to fix issues with online resize due to SPO
2020-03-19 9:15 ` Sahitya Tummala
@ 2020-03-19 10:47 ` Chao Yu
2020-03-19 15:16 ` Sahitya Tummala
0 siblings, 1 reply; 4+ messages in thread
From: Chao Yu @ 2020-03-19 10:47 UTC (permalink / raw)
To: Sahitya Tummala; +Cc: Jaegeuk Kim, linux-f2fs-devel
Hi Sahitya,
On 2020/3/19 17:15, Sahitya Tummala wrote:
> Hi Chao,
>
> On Mon, Mar 16, 2020 at 11:19:11AM +0800, Chao Yu wrote:
>> On 2020/3/6 11:52, Sahitya Tummala wrote:
>>> Add support for new CP flag CP_RESIZEFS_FLAG set during online
>>> resize FS. If SPO happens after SB is updated but CP isn't, then
>>> allow fsck to fix it.
>>>
>>> fsck errors without this fix -
>>> Info: CKPT version = 6ed7bccb
>>> Wrong user_block_count(2233856)
>>> [f2fs_do_mount:3365] Checkpoint is polluted
>>>
>>> the subsequent mount failure without this fix -
>>> [ 11.294650] F2FS-fs (sda8): Wrong user_block_count: 2233856
>>> [ 11.300272] F2FS-fs (sda8): Failed to get valid F2FS checkpoint
>>>
>>> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
>>> ---
>>> v2:
>>> - fix even if CP_FSCK_FLAG is set for backward compatibility
>>> - update print_cp_state()
>>>
>>> fsck/mount.c | 34 +++++++++++++++++++++++++++++++---
>>> include/f2fs_fs.h | 1 +
>>> 2 files changed, 32 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/fsck/mount.c b/fsck/mount.c
>>> index e4ba048..8d32e41 100644
>>> --- a/fsck/mount.c
>>> +++ b/fsck/mount.c
>>> @@ -429,6 +429,8 @@ void print_cp_state(u32 flag)
>>> MSG(0, "%s", " orphan_inodes");
>>> if (flag & CP_DISABLED_FLAG)
>>> MSG(0, "%s", " disabled");
>>> + if (flag & CP_RESIZEFS_FLAG)
>>> + MSG(0, "%s", " resizefs");
>>> if (flag & CP_UMOUNT_FLAG)
>>> MSG(0, "%s", " unmount");
>>> else
>>> @@ -1128,6 +1130,7 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi)
>>> unsigned int total, fsmeta;
>>> struct f2fs_super_block *sb = F2FS_RAW_SUPER(sbi);
>>> struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
>>> + unsigned int flag = get_cp(ckpt_flags);
>>> unsigned int ovp_segments, reserved_segments;
>>> unsigned int main_segs, blocks_per_seg;
>>> unsigned int sit_segs, nat_segs;
>>> @@ -1164,7 +1167,32 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi)
>>> log_blocks_per_seg = get_sb(log_blocks_per_seg);
>>> if (!user_block_count || user_block_count >=
>>> segment_count_main << log_blocks_per_seg) {
>>> - MSG(0, "\tWrong user_block_count(%u)\n", user_block_count);
>>> + if (flag & (CP_FSCK_FLAG | CP_RESIZEFS_FLAG)) {
>>> + u32 valid_user_block_cnt;
>>> + u32 seg_cnt_main = get_sb(segment_count) -
>>> + (get_sb(segment_count_ckpt) +
>>> + get_sb(segment_count_sit) +
>>> + get_sb(segment_count_nat) +
>>> + get_sb(segment_count_ssa));
>>> +
>>> + /* validate segment_count_main in sb first */
>>> + if (seg_cnt_main != get_sb(segment_count_main)) {
>>> + MSG(0, "\tWrong user_block_count(%u) and inconsistent segment_cnt_main %u\n",
>>> + user_block_count,
>>> + segment_count_main << log_blocks_per_seg);
>>> + return 1;
>>> + }
>>> + valid_user_block_cnt = ((get_sb(segment_count_main) -
>>> + get_cp(overprov_segment_count)) * c.blks_per_seg);
>>> + MSG(0, "\tInfo: Fix wrong user_block_count in CP: (%u) -> (%u)\n",
>>> + user_block_count, valid_user_block_cnt);
>>
>> By default, we should only fix such bug if c.fix_on is true, something
>> like this:
>>
>> ASSERT_MSG("\tWrong user_block_count(%u)\n", user_block_count);
>>
>> if (!c.fix_on)
>> return 1;
>>
>> valid_user_block_cnt = ((get_sb(segment_count_main) -
>> get_cp(overprov_segment_count)) * c.blks_per_seg);
>>
>> MSG(0, "\tInfo: Fix wrong user_block_count in CP: (%u) -> (%u)\n",
>> user_block_count, valid_user_block_cnt);
>>
> Since this is a fatal error which fails the basic mount itself, I thought it
> must be fixed by default with fsck independent of -f option. Can we do so for
> such critical bugs?
I suggest to follow the options' meaning, e.g. if user specified --dry-run,
however we change any bits on that image, it looks that break that option's
semantics, and it may violate user's will.... so IMO, if user don't want to
repair the image, it will be better to keep the image as it is.
Thoughts?
To Jaegeuk, thoughts?
Thanks,
>
> Thanks,
>
>> Thanks,
>>
>>
>>> + set_cp(user_block_count, valid_user_block_cnt);
>>> + c.fix_on = 1;
>>> + c.bug_on = 1;
>>> + } else {
>>> + MSG(0, "\tWrong user_block_count(%u)\n", user_block_count);
>>> + return 1;
>>> + }
>>> return 1;
>>> }
>>>
>>> @@ -3361,6 +3389,8 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
>>> return -1;
>>> }
>>>
>>> + c.bug_on = 0;
>>> +
>>> if (sanity_check_ckpt(sbi)) {
>>> ERR_MSG("Checkpoint is polluted\n");
>>> return -1;
>>> @@ -3380,8 +3410,6 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
>>> c.fix_on = 1;
>>> }
>>>
>>> - c.bug_on = 0;
>>> -
>>> if (tune_sb_features(sbi))
>>> return -1;
>>>
>>> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
>>> index af31bc5..265f50c 100644
>>> --- a/include/f2fs_fs.h
>>> +++ b/include/f2fs_fs.h
>>> @@ -678,6 +678,7 @@ struct f2fs_super_block {
>>> /*
>>> * For checkpoint
>>> */
>>> +#define CP_RESIZEFS_FLAG 0x00004000
>>> #define CP_DISABLED_FLAG 0x00001000
>>> #define CP_QUOTA_NEED_FSCK_FLAG 0x00000800
>>> #define CP_LARGE_NAT_BITMAP_FLAG 0x00000400
>>>
>
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [f2fs-dev] [PATCH V2] fsck.f2fs: allow fsck to fix issues with online resize due to SPO
2020-03-19 10:47 ` Chao Yu
@ 2020-03-19 15:16 ` Sahitya Tummala
0 siblings, 0 replies; 4+ messages in thread
From: Sahitya Tummala @ 2020-03-19 15:16 UTC (permalink / raw)
To: Chao Yu; +Cc: Jaegeuk Kim, linux-f2fs-devel
On Thu, Mar 19, 2020 at 06:47:07PM +0800, Chao Yu wrote:
> Hi Sahitya,
>
> On 2020/3/19 17:15, Sahitya Tummala wrote:
> > Hi Chao,
> >
> > On Mon, Mar 16, 2020 at 11:19:11AM +0800, Chao Yu wrote:
> >> On 2020/3/6 11:52, Sahitya Tummala wrote:
> >>> Add support for new CP flag CP_RESIZEFS_FLAG set during online
> >>> resize FS. If SPO happens after SB is updated but CP isn't, then
> >>> allow fsck to fix it.
> >>>
> >>> fsck errors without this fix -
> >>> Info: CKPT version = 6ed7bccb
> >>> Wrong user_block_count(2233856)
> >>> [f2fs_do_mount:3365] Checkpoint is polluted
> >>>
> >>> the subsequent mount failure without this fix -
> >>> [ 11.294650] F2FS-fs (sda8): Wrong user_block_count: 2233856
> >>> [ 11.300272] F2FS-fs (sda8): Failed to get valid F2FS checkpoint
> >>>
> >>> Signed-off-by: Sahitya Tummala <stummala@codeaurora.org>
> >>> ---
> >>> v2:
> >>> - fix even if CP_FSCK_FLAG is set for backward compatibility
> >>> - update print_cp_state()
> >>>
> >>> fsck/mount.c | 34 +++++++++++++++++++++++++++++++---
> >>> include/f2fs_fs.h | 1 +
> >>> 2 files changed, 32 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/fsck/mount.c b/fsck/mount.c
> >>> index e4ba048..8d32e41 100644
> >>> --- a/fsck/mount.c
> >>> +++ b/fsck/mount.c
> >>> @@ -429,6 +429,8 @@ void print_cp_state(u32 flag)
> >>> MSG(0, "%s", " orphan_inodes");
> >>> if (flag & CP_DISABLED_FLAG)
> >>> MSG(0, "%s", " disabled");
> >>> + if (flag & CP_RESIZEFS_FLAG)
> >>> + MSG(0, "%s", " resizefs");
> >>> if (flag & CP_UMOUNT_FLAG)
> >>> MSG(0, "%s", " unmount");
> >>> else
> >>> @@ -1128,6 +1130,7 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi)
> >>> unsigned int total, fsmeta;
> >>> struct f2fs_super_block *sb = F2FS_RAW_SUPER(sbi);
> >>> struct f2fs_checkpoint *cp = F2FS_CKPT(sbi);
> >>> + unsigned int flag = get_cp(ckpt_flags);
> >>> unsigned int ovp_segments, reserved_segments;
> >>> unsigned int main_segs, blocks_per_seg;
> >>> unsigned int sit_segs, nat_segs;
> >>> @@ -1164,7 +1167,32 @@ int sanity_check_ckpt(struct f2fs_sb_info *sbi)
> >>> log_blocks_per_seg = get_sb(log_blocks_per_seg);
> >>> if (!user_block_count || user_block_count >=
> >>> segment_count_main << log_blocks_per_seg) {
> >>> - MSG(0, "\tWrong user_block_count(%u)\n", user_block_count);
> >>> + if (flag & (CP_FSCK_FLAG | CP_RESIZEFS_FLAG)) {
> >>> + u32 valid_user_block_cnt;
> >>> + u32 seg_cnt_main = get_sb(segment_count) -
> >>> + (get_sb(segment_count_ckpt) +
> >>> + get_sb(segment_count_sit) +
> >>> + get_sb(segment_count_nat) +
> >>> + get_sb(segment_count_ssa));
> >>> +
> >>> + /* validate segment_count_main in sb first */
> >>> + if (seg_cnt_main != get_sb(segment_count_main)) {
> >>> + MSG(0, "\tWrong user_block_count(%u) and inconsistent segment_cnt_main %u\n",
> >>> + user_block_count,
> >>> + segment_count_main << log_blocks_per_seg);
> >>> + return 1;
> >>> + }
> >>> + valid_user_block_cnt = ((get_sb(segment_count_main) -
> >>> + get_cp(overprov_segment_count)) * c.blks_per_seg);
> >>> + MSG(0, "\tInfo: Fix wrong user_block_count in CP: (%u) -> (%u)\n",
> >>> + user_block_count, valid_user_block_cnt);
> >>
> >> By default, we should only fix such bug if c.fix_on is true, something
> >> like this:
> >>
> >> ASSERT_MSG("\tWrong user_block_count(%u)\n", user_block_count);
> >>
> >> if (!c.fix_on)
> >> return 1;
> >>
> >> valid_user_block_cnt = ((get_sb(segment_count_main) -
> >> get_cp(overprov_segment_count)) * c.blks_per_seg);
> >>
> >> MSG(0, "\tInfo: Fix wrong user_block_count in CP: (%u) -> (%u)\n",
> >> user_block_count, valid_user_block_cnt);
> >>
> > Since this is a fatal error which fails the basic mount itself, I thought it
> > must be fixed by default with fsck independent of -f option. Can we do so for
> > such critical bugs?
>
> I suggest to follow the options' meaning, e.g. if user specified --dry-run,
> however we change any bits on that image, it looks that break that option's
> semantics, and it may violate user's will.... so IMO, if user don't want to
> repair the image, it will be better to keep the image as it is.
>
> Thoughts?
Sure Chao. I will update the patch to fix it as per your suggestion.
Thanks,
>
> To Jaegeuk, thoughts?
>
> Thanks,
>
> >
> > Thanks,
> >
> >> Thanks,
> >>
> >>
> >>> + set_cp(user_block_count, valid_user_block_cnt);
> >>> + c.fix_on = 1;
> >>> + c.bug_on = 1;
> >>> + } else {
> >>> + MSG(0, "\tWrong user_block_count(%u)\n", user_block_count);
> >>> + return 1;
> >>> + }
> >>> return 1;
> >>> }
> >>>
> >>> @@ -3361,6 +3389,8 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
> >>> return -1;
> >>> }
> >>>
> >>> + c.bug_on = 0;
> >>> +
> >>> if (sanity_check_ckpt(sbi)) {
> >>> ERR_MSG("Checkpoint is polluted\n");
> >>> return -1;
> >>> @@ -3380,8 +3410,6 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
> >>> c.fix_on = 1;
> >>> }
> >>>
> >>> - c.bug_on = 0;
> >>> -
> >>> if (tune_sb_features(sbi))
> >>> return -1;
> >>>
> >>> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> >>> index af31bc5..265f50c 100644
> >>> --- a/include/f2fs_fs.h
> >>> +++ b/include/f2fs_fs.h
> >>> @@ -678,6 +678,7 @@ struct f2fs_super_block {
> >>> /*
> >>> * For checkpoint
> >>> */
> >>> +#define CP_RESIZEFS_FLAG 0x00004000
> >>> #define CP_DISABLED_FLAG 0x00001000
> >>> #define CP_QUOTA_NEED_FSCK_FLAG 0x00000800
> >>> #define CP_LARGE_NAT_BITMAP_FLAG 0x00000400
> >>>
> >
--
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-19 15:17 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1583466746-17445-1-git-send-email-stummala@codeaurora.org>
2020-03-16 3:19 ` [f2fs-dev] [PATCH V2] fsck.f2fs: allow fsck to fix issues with online resize due to SPO Chao Yu
2020-03-19 9:15 ` Sahitya Tummala
2020-03-19 10:47 ` Chao Yu
2020-03-19 15:16 ` Sahitya Tummala
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.