Hello, On Fri, Jan 13, 2023 at 09:25:11AM +0800, Yu Kuai wrote: > I think hold the lock in blkg_free_workfn() is too late, pd_free_fn() > for parent from blkcg_deactivate_policy() can be called first. > > t1: remove cgroup t1/t2 > blkcg_destroy_blkgs > blkg_destroy > percpu_ref_kill(&blkg->refcnt) > blkg_release > blkg_free > schedule_work(&blkg->free_work) > // t1 is done > > t2: handle t1 from removing device > blkcg_deactivate_policy > pd_free_fn > // free parent > t3: from t1 > blkg_free_workfn > pd_free_fn > // free child As we discussed before, you'd have to order the actual freeing by shifting the ref puts into the free_work. If you move `blkg_put(blkg->parent)` and `list_del_init(&blkg->q_node)` to blkg_free_workfn() (this will require adjustments as these things are used from other places too), the free work items will be ordered and the blkg would remain iterable - IOW, deactivate_policy would be able to see it allowing the two paths to synchronize, right? Thanks. -- tejun
