linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Yafang Shao <laoar.shao@gmail.com>
To: mhocko@kernel.org, hannes@cmpxchg.org, vdavydov.dev@gmail.com,
	akpm@linux-foundation.org
Cc: linux-mm@kvack.org, Yafang Shao <laoar.shao@gmail.com>,
	David Hildenbrand <david@redhat.com>
Subject: [PATCH v3] mm, memcg: fix the stupid OOM killer when shrinking memcg hard limit
Date: Wed, 27 Nov 2019 23:17:13 -0500	[thread overview]
Message-ID: <1574914633-2020-1-git-send-email-laoar.shao@gmail.com> (raw)

When there are no more processes in a memcg (e.g., due to OOM
group), we can still have file pages in the page cache.

If these pages are protected by memory.min, they can't be reclaimed.
Especially if there won't be another process in this memcg and the memcg
is kept online, we do want to drop these pages from the page cache.

By dropping these page caches we can avoid reclaimers (e.g., kswapd or
direct) to scan and reclaim pages from all memcgs in the system -
because the reclaimers will try to fairly reclaim pages from all memcgs
in the system when under memory pressure.

By setting the hard limit of such a memcg to 0, we allow to drop the
page cache of such memcgs. Unfortunately, this may invoke the OOM killer
and generate a lot of output. The OOM output is not expected by an admin
who wants to drop these caches and knows that there are no processes in
this memcg anymore.

Therefore, if a memcg is not populated, we should not invoke the OOM
killer - there is nothing to kill. The next time a new process is
started in the memcg and the "max" is still below usage, the OOM killer
will be invoked and the new process will be killed.

[ Above commit log is contributed by David ]

What's worse about this issue is that when there're killable tasks and the
OOM killer killed the last task, and what will happen then ? As nr_reclaims
is already 0 and drained is alreay true, the OOM killer will try to kill
nothing (because he knows he has killed the last task), what's a stupid
behavior.

Someone may worry that the admins may not going to see that the memcg was
OOM due to the limit change. But this is not a issue, because the admins
changes the limit and then the admins must check the result of his change
- by checking memory.{max, current, stat} he can get all he wants.

Cc: David Hildenbrand <david@redhat.com>
Nacked-by: Michal Hocko <mhocko@suse.com>
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>

---
Changes since v2: Refresh the subject and commit log. The original
subject is "mm, memcg: avoid oom if cgroup is not populated"
---
 mm/memcontrol.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 1c4c08b..e936f1b 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -6139,9 +6139,20 @@ static ssize_t memory_max_write(struct kernfs_open_file *of,
 			continue;
 		}
 
-		memcg_memory_event(memcg, MEMCG_OOM);
-		if (!mem_cgroup_out_of_memory(memcg, GFP_KERNEL, 0))
+		/* If there's no procesess, we don't need to invoke the OOM
+		 * killer. Then next time when you try to start a process
+		 * in this memcg, the max may still bellow usage, and then
+		 * this OOM killer will be invoked. This can be considered
+		 * as lazy OOM, that is we have been always doing in the
+		 * kernel. Pls. Michal, that is really consistency.
+		 */
+		if (cgroup_is_populated(memcg->css.cgroup)) {
+			memcg_memory_event(memcg, MEMCG_OOM);
+			if (!mem_cgroup_out_of_memory(memcg, GFP_KERNEL, 0))
+				break;
+		} else  {
 			break;
+		}
 	}
 
 	memcg_wb_domain_size_changed(memcg);
-- 
1.8.3.1



             reply	other threads:[~2019-11-28  4:17 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-28  4:17 Yafang Shao [this message]
2019-11-28  4:28 ` [PATCH v3] mm, memcg: fix the stupid OOM killer when shrinking memcg hard limit Qian Cai
2019-11-28  4:59   ` Yafang Shao

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=1574914633-2020-1-git-send-email-laoar.shao@gmail.com \
    --to=laoar.shao@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=david@redhat.com \
    --cc=hannes@cmpxchg.org \
    --cc=linux-mm@kvack.org \
    --cc=mhocko@kernel.org \
    --cc=vdavydov.dev@gmail.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;
as well as URLs for NNTP newsgroup(s).