Re: [f2fs-dev] [PATCH v4 2/2] fsck.f2fs: Check write pointer consistency with current segments

[Chao Yu <yuchao0@huawei.com>]

On 2019/9/25 16:05, Shinichiro Kawasaki wrote:
> On Sep 21, 2019 / 17:42, Chao Yu wrote:
>> On 2019/9/18 11:07, Shinichiro Kawasaki wrote:
>>> Thanks for the comments. I read f2fs code further, and think still the
>>> SIT vs write pointer check can be implemented and meaningful.
>>>
>>> F2fs ensures consistency of SIT using two CP areas, two SIT areas and
>>> sit_bitmap in CP. These metadata are in the conventional zone that not
>>> affected by write pointer control logic. My current scope is to ensure
>>> write pointer control logic correctness for zoned block device. From this
>>> scope and the f2fs SIT consistency feature, I would like to assume that
>>> SIT entries built in kernel after f2fs mount is correct for the write
>>> pointer position check.
>>
>> SIT may be broken due to software bug or hardware flaw, we'd better not consider
>> it as a consistent metadata.
> 
> I see. I found that kernel checks SIT valid blocks with check_block_count()
> function, and if mismatch happens it requests fsck. I guess SIT consistency is
> not ensured during kernel run, but after fsck run. Is this correct? If so, I
> think the write pointer consistency check with SIT valid block bitmaps should
> not be done by kernel, but by fsck after SIT rewrite.

IMO, consistency check can be done in both kernel and fsck.

If SIT is corrupted, but kernel doesn't aware of it (check_block_count() doesn't
report any inconsistency), write_pointer consistency check may give some clues
for filesystem corruption.

> 
> As for non-open zones (curseg do not point to), the other check is needed: if
> the zone does not have valid blocks and fsync data, need to reset its write
> pointer. My understanding is that the valid blocks count comes from SIT. Then
> this fix also should be done not by kernel but by fsck after SIT rewrite. I
> think kernel just should do this check at mount time to avoid unaligned write
> error, and if inconsistency found it should request fsck to recheck and fix.

Yeah, I agree with this.

> 
> I assume kernel can check and fix write pointer inconsistency with cursegs in
> CP. CP (and SB) have checksum guard and their consistency looks ensured during
> kernel run.
> 
>>
>>>
>>> Fsck does additional SIT consistency check in fsck_chk_meta(). It would be
>>> good to do the write pointer position check at the end of fsck_chk_meta().
>>
>> SIT can be changed later? e.g. SIT bitmap says one block address is valid,
>> however fsck found there is no entry can link to it, then it needs to be
>> deleted? it may affect write_pointer repair, right?
>>
>> So we'd better look into all SIT update cases in fsck.
> 
> Ah, yes, when fsck updates SIT, write pointer consistency check should be done
> after that. I found SIT is rewritten at the end of fsck_verify() by
> rewrite_sit_area_bitmap() and fix_curseg_info() calls. I guess write pointer
> consistency check with SIT (and write pointer reset if required) should be
> done after those function calls to reflect all SIT changes by fsck. I'll try
> to create a patch with this approach.

Quota file repair may grab and write the block which can change SIT status,
please notice that case as well.

Thanks,

> 
> Thanks again for your comments!
> 
> --
> 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