On Fri, Oct 19, 2018 at 07:29:26AM +0800, Qu Wenruo wrote:
>
>
> On 2018/10/19 上午12:20, David Sterba wrote:
> > On Thu, Oct 18, 2018 at 07:17:27PM +0800, Qu Wenruo wrote:
> >> +void btrfs_qgroup_clean_swapped_blocks(struct btrfs_root *root)
> >> +{
> >> + struct btrfs_qgroup_swapped_blocks *swapped_blocks;
> >> + struct btrfs_qgroup_swapped_block *cur, *next;
> >> + int i;
> >> +
> >> + swapped_blocks = &root->swapped_blocks;
> >> +
> >> + spin_lock(&swapped_blocks->lock);
> >> + if (!swapped_blocks->swapped)
> >> + goto out;
> >> + for (i = 0; i < BTRFS_MAX_LEVEL; i++) {
> >> + struct rb_root *cur_root = &swapped_blocks->blocks[i];
> >> +
> >> + rbtree_postorder_for_each_entry_safe(cur, next, cur_root,
> >> + node) {
> >> + rb_erase(&cur->node, cur_root);
> >
> > https://elixir.bootlin.com/linux/latest/source/include/linux/rbtree.h#L141
> >
> > rb_erase must not be used on the same pointer that
> > rbtree_postorder_for_each_entry_safe iterates, here it's cur.
>
> Oh, thanks for pointing this out.
>
> So we must go the old while(rb_first()) loop...
I just realized the postorder iterator can be used here. The forbidden
pattern is
rbtree_postorder_for_each_entry_safe() {
rb_erase();
}
But you also do kfree right after the erase, and the iterator is fine
with that and rb_erase is pointless for an entry that's being freed
anyway.
