On 2023/4/20 04:03, Andrew Morton wrote: > On Wed, 19 Apr 2023 09:20:07 +0000 Haifeng Xu <haifeng.xu@xxxxxxxxxx> wrote: > >> There are three places, vmpressure_prio(), shrink_node_memcgs() and >> shrink_node(), which invoke vmpressure(). But only shrink_node_memcgs() >> sets tree to false and the memcg used in it is not NULL, so we don't >> check it again in vmpressure(). >> >> ... >> >> --- a/mm/vmpressure.c >> +++ b/mm/vmpressure.c >> @@ -284,7 +284,7 @@ void vmpressure(gfp_t gfp, struct mem_cgroup *memcg, bool tree, >> enum vmpressure_levels level; >> >> /* For now, no users for root-level efficiency */ >> - if (!memcg || mem_cgroup_is_root(memcg)) >> + if (mem_cgroup_is_root(memcg)) >> return; >> >> spin_lock(&vmpr->sr_lock); > > try_to_free_pages()-> > do_try_to_free_pages()-> > vmpressure_prio()-> > vmpressure()-> > crash > > what am I missing here? > > vmpressure_prio() set tree to true, so the crash won't happen. > It does appear that vmpressure() could be simplified with > > if (!memcg) > memcg = root_mem_cgroup; > > so the test you identified goes away and the memcg_to_vmpressure() call > becomes simpler. But that's such a small change it doesn't seem worth > the effort. memcg_to_vmpressure() has taken this case that the memcg is NULL into account and we do not need to change it. The test I identified is in the branch that the tree is set to false. In this branch, memcg can never be NULL because only shrink_node_memcgs() will step into it.
