On 2020/2/12 上午5:55, Josef Bacik wrote:
> On 2/8/20 5:07 AM, Qu Wenruo wrote:
...
>> But that would cause ASSERT() if CONFIG_BTRFS_FS_CHECK_INTEGRITY is
>> enabled.
>> As described, the offending tree block is completely empty during log
>> tree rebalance, if it's COWed for extent tree, tree-checker won't
>> allow empty extent tree at all, thus we will hit ASSERT() in
>> btrfs_mark_buffer_dirty().
>>
>> I'm not sure if we should go that direction. So I only go the
>> last-safe-net method.
>
> Yeah I can't think of a better scenario here. We're dealing with a
> corrupt fs, and trying to work around the corruption in this case isn't
> super clear-cut. If there was flaw in how we were handling blocks then
> I'd be more inclined to fixing the problem more directly, but this is
> just wonky and we can't trust we'll make the right decision for every
> type of corruption.
Another idea is, to check the eb->refs at btrfs_init_new_buffer().
As we're expecting a completely new buffer not shared by anyone else.
And it follows my old idea of rejecting corruption as early as possible.
But IIRC I have submitted similar patch before but didn't end up too
well, so I'll leave that as another patchset.
...
>> + * Now such dirty tree block will not be cleaned by any dirty
>> + * extent io tree. Thus we don't want to submit such wild eb
>> + * if transaction is aborted.
>> + */
>> + if (!test_bit(BTRFS_FS_STATE_TRANS_ABORTED, &fs_info->fs_state)) {
>
> This needs to be BTRFS_FS_STATE_ERROR, because we could have tripped
> over some other error where we didn't have a transaction and never
> gotten TRANS_ABORTED. Thanks,
Thanks for that hint. Indeed makes sense.
Hopes next version will be the last version.
Thanks,
Qu
>
> Josef
Attachment:
signature.asc
Description: OpenPGP digital signature
