Message ID | 20220513145928.29766-2-gaoyahu19@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | block,iocost: fix potential kernel NULL | expand |
Hello, On Fri, May 13, 2022 at 10:59:28PM +0800, Yahu Gao wrote: > From: Yahu Gao <yahugao@didiglobal.com> > > Some inode pinned dying memory cgroup and its parent destroyed at first. > The parent's pd of iocost won't be allocated during function > blkcg_activate_policy. > Ignore the DYING CSS to avoid kernel NULL during iocost policy data init. Thanks for the analysis and patch but I'm not quite sure the analysis is correct. When a cgroup goes down, its blkgs are destroyed blkcg_destroy_blkgs() which is invoked by blkcg_unpin_online() when its online_pin reaches zero which is incremented and decremented recursively, so an ancestor's blkgs should be destroyed before a descendant's if the code is working as intended. Can you guys dig a bit deeper and why we're losing ancestor blkgs before descendants? Thanks.
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index a91f8ae18b49..32472de2e61d 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -1418,6 +1418,9 @@ int blkcg_activate_policy(struct request_queue *q, list_for_each_entry_reverse(blkg, &q->blkg_list, q_node) { struct blkg_policy_data *pd; + if (blkg->blkcg->css.flags & CSS_DYING) + continue; + if (blkg->pd[pol->plid]) continue; @@ -1459,8 +1462,11 @@ int blkcg_activate_policy(struct request_queue *q, /* all allocated, init in the same order */ if (pol->pd_init_fn) - list_for_each_entry_reverse(blkg, &q->blkg_list, q_node) + list_for_each_entry_reverse(blkg, &q->blkg_list, q_node) { + if (blkg->blkcg->css.flags & CSS_DYING) + continue; pol->pd_init_fn(blkg->pd[pol->plid]); + } __set_bit(pol->plid, q->blkcg_pols); ret = 0;