From: Haifeng Xu <haifeng.xu@shopee.com>
To: Roman Gushchin <roman.gushchin@linux.dev>
Cc: mhocko@kernel.org, hannes@cmpxchg.org, shakeelb@google.com,
cgroups@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH 2/2] memcg, oom: do not wake up memcg_oom_waitq if waitqueue is empty
Date: Sat, 23 Sep 2023 16:01:15 +0800 [thread overview]
Message-ID: <94c2a462-1d4e-e6e8-0e31-babcbc459836@shopee.com> (raw)
In-Reply-To: <ZQ4hsQRp31RXMOfv@P9FQF9L96D.corp.robot.car>
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@shopee.com>
>> ---
>> 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!
prev parent reply other threads:[~2023-09-23 8:01 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-22 7:05 Haifeng Xu
2023-09-22 23:22 ` Roman Gushchin
2023-09-23 8:01 ` Haifeng Xu [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=94c2a462-1d4e-e6e8-0e31-babcbc459836@shopee.com \
--to=haifeng.xu@shopee.com \
--cc=cgroups@vger.kernel.org \
--cc=hannes@cmpxchg.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=roman.gushchin@linux.dev \
--cc=shakeelb@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox