Re: [PATCH 09/12] btrfs: clear defragmented inodes using postorder in btrfs_cleanup_defrag_inodes()

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2024/8/28 08:48, David Sterba 写道:
> On Wed, Aug 28, 2024 at 08:29:23AM +0930, Qu Wenruo wrote:
>>
>>
>> 在 2024/8/28 07:25, David Sterba 写道:
>>> btrfs_cleanup_defrag_inodes() is not called frequently, only in remount
>>> or unmount, but the way it frees the inodes in fs_info->defrag_inodes
>>> is inefficient. Each time it needs to locate first node, remove it,
>>> potentially rebalance tree until it's done. This allows to do a
>>> conditional reschedule.
>>>
>>> For cleanups the rbtree_postorder_for_each_entry_safe() iterator is
>>> convenient but if the reschedule happens and unlocks fs_info->defrag_inodes_lock
>>> we can't be sure that the tree is in the same state. If that happens,
>>> restart the iteration from the beginning.
>>
>> In that case, isn't the rbtree itself in an inconsistent state, and
>> restarting will only cause invalid memory access?
>>
>> So in this particular case, since we can be interrupted, the full tree
>> balance looks like the only safe way we can go?
>
> You're right, the nodes get freed so even if the iteration is restarted
> it would touch freed memory, IOW rbtree_postorder_for_each_entry_safe()
> can't be interrupted. I can drop the reschedule, with the same argument
> that it should be relatively fast even for thousands of entries, this
> should not hurt for remouunt/umount context.
>

Considering the autodefrag is only triggered for certain writes, and at
remount (to RO) or unmount time, there should be no more writes, the
solution looks fine.

Thanks,
Qu