On 2023/9/23 07:22, Roman Gushchin wrote: > On Fri, Sep 22, 2023 at 07:05:29AM +0000, Haifeng Xu wrote: >> Only when memcg oom killer is disabled, the task which triggers memecg >> oom handling will sleep on a waitqueue. Except this case, the waitqueue >> is empty though under_oom is true. There is no need to step into wake >> up path when resolve the oom situation. So add a check that whether the >> waitqueue is empty. >> >> Signed-off-by: Haifeng Xu <haifeng.xu@xxxxxxxxxx> >> --- >> mm/memcontrol.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/mm/memcontrol.c b/mm/memcontrol.c >> index 0b6ed63504ca..2bb98ff5be3d 100644 >> --- a/mm/memcontrol.c >> +++ b/mm/memcontrol.c >> @@ -1918,7 +1918,7 @@ static void memcg_oom_recover(struct mem_cgroup *memcg) >> * achieved by invoking mem_cgroup_mark_under_oom() before >> * triggering notification. >> */ >> - if (memcg && memcg->under_oom) >> + if (memcg && memcg->under_oom && !list_empty(&memcg_oom_waitq.head)) >> __wake_up(&memcg_oom_waitq, TASK_NORMAL, 0, memcg); > > This change looks questionable to me: > 1) it's not obvious that this racy check is fine. can an oom event be > missed because of a race here? why not? The oom event can't be missed, because the OOM task has been put on the waitqueue before marking the hierarchy as under OOM(can be seen in mem_cgroup_oom_synchronize()). > 2) is there any measurable impact? No. it's not a hot path, so I'd keep it > simple. ok, thanks. > > Thanks!
