Messages in this thread | | | Date | Wed, 1 Jun 2022 17:00:44 -0700 | From | Tadeusz Struk <> | Subject | Re: [PATCH 2/2] cgroup: Use separate work structs on css release path |
| |
On 6/1/22 16:43, Tejun Heo wrote: > On Wed, Jun 01, 2022 at 04:37:17PM -0700, Tadeusz Struk wrote: >> Yes, but as far as I can see the percpu_ref_kill_and_confirm(&css->refcnt, css_killed_ref_fn) >> doesn't change the value of the refcnt, it just causes the css_killed_ref_fn() to be called > > Yeah, the base ref is special for percpu_ref. > >> on it. Only css_get() & css_put() modify the refcnt value. >> And for the "free the thing" the css_killed_work_fn() does that. >> It calls offline_css(css) and css_put(css) for the whole css hierarchy. > > Yeah, the freeing path depends on the css_put(css) invoking css_release() > which schedules the work item which actually frees. Am I misunderstanding > something here?
What I'm trying to say is that it's not really a ref imbalance problem. I think once the kill_css() has been called on a css, and it is enqueued to be killed, we shouldn't call css_put(css) on it anymore outside of the "killed call flow path". It will all be handled by the css_killed_ref_fn() function.
The fact the css_release() is called (via cgroup_kn_unlock()) just after kill_css() causes the css->destroy_work to be enqueued twice on the same WQ (cgroup_destroy_wq), just with different function. This results in the BUG: corrupted list in insert_work issue.
-- Thanks, Tadeusz
| |