From: Chris Down <chris@chrisdown.name>
To: Johannes Weiner <hannes@cmpxchg.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Leon Yang <lnyng@fb.com>, Roman Gushchin <guro@fb.com>,
Michal Hocko <mhocko@suse.com>,
linux-mm@kvack.org, cgroups@vger.kernel.org,
linux-kernel@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH] mm: memcontrol: fix occasional OOMs due to proportional memory.low reclaim
Date: Wed, 18 Aug 2021 21:18:10 +0100 [thread overview]
Message-ID: <YR1rAoFNegl6CrPz@chrisdown.name> (raw)
In-Reply-To: <20210817180506.220056-1-hannes@cmpxchg.org>
Johannes Weiner writes:
>We've noticed occasional OOM killing when memory.low settings are in
>effect for cgroups. This is unexpected and undesirable as memory.low
>is supposed to express non-OOMing memory priorities between cgroups.
>
>The reason for this is proportional memory.low reclaim. When cgroups
>are below their memory.low threshold, reclaim passes them over in the
>first round, and then retries if it couldn't find pages anywhere else.
>But when cgroups are slighly above their memory.low setting, page scan
>force is scaled down and diminished in proportion to the overage, to
>the point where it can cause reclaim to fail as well - only in that
>case we currently don't retry, and instead trigger OOM.
>
>To fix this, hook proportional reclaim into the same retry logic we
>have in place for when cgroups are skipped entirely. This way if
>reclaim fails and some cgroups were scanned with dimished pressure,
>we'll try another full-force cycle before giving up and OOMing.
>
>Reported-by: Leon Yang <lnyng@fb.com>
>Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
Thanks for tracking this down! Agreed that this looks like a good stable
candidate.
Acked-by: Chris Down <chris@chrisdown.name>
>---
> include/linux/memcontrol.h | 29 +++++++++++++++--------------
> mm/vmscan.c | 27 +++++++++++++++++++--------
> 2 files changed, 34 insertions(+), 22 deletions(-)
>
>diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
>index bfe5c486f4ad..24797929d8a1 100644
>--- a/include/linux/memcontrol.h
>+++ b/include/linux/memcontrol.h
>@@ -612,12 +612,15 @@ static inline bool mem_cgroup_disabled(void)
> return !cgroup_subsys_enabled(memory_cgrp_subsys);
> }
>
>-static inline unsigned long mem_cgroup_protection(struct mem_cgroup *root,
>- struct mem_cgroup *memcg,
>- bool in_low_reclaim)
>+static inline void mem_cgroup_protection(struct mem_cgroup *root,
>+ struct mem_cgroup *memcg,
>+ unsigned long *min,
>+ unsigned long *low)
> {
>+ *min = *low = 0;
>+
> if (mem_cgroup_disabled())
>- return 0;
>+ return;
>
> /*
> * There is no reclaim protection applied to a targeted reclaim.
>@@ -653,13 +656,10 @@ static inline unsigned long mem_cgroup_protection(struct mem_cgroup *root,
> *
> */
> if (root == memcg)
>- return 0;
>-
>- if (in_low_reclaim)
>- return READ_ONCE(memcg->memory.emin);
>+ return;
>
>- return max(READ_ONCE(memcg->memory.emin),
>- READ_ONCE(memcg->memory.elow));
>+ *min = READ_ONCE(memcg->memory.emin);
>+ *low = READ_ONCE(memcg->memory.elow);
> }
>
> void mem_cgroup_calculate_protection(struct mem_cgroup *root,
>@@ -1147,11 +1147,12 @@ static inline void memcg_memory_event_mm(struct mm_struct *mm,
> {
> }
>
>-static inline unsigned long mem_cgroup_protection(struct mem_cgroup *root,
>- struct mem_cgroup *memcg,
>- bool in_low_reclaim)
>+static inline void mem_cgroup_protection(struct mem_cgroup *root,
>+ struct mem_cgroup *memcg,
>+ unsigned long *min,
>+ unsigned long *low)
> {
>- return 0;
>+ *min = *low = 0;
> }
>
> static inline void mem_cgroup_calculate_protection(struct mem_cgroup *root,
>diff --git a/mm/vmscan.c b/mm/vmscan.c
>index 4620df62f0ff..701106e1829c 100644
>--- a/mm/vmscan.c
>+++ b/mm/vmscan.c
>@@ -100,9 +100,12 @@ struct scan_control {
> unsigned int may_swap:1;
>
> /*
>- * Cgroups are not reclaimed below their configured memory.low,
>- * unless we threaten to OOM. If any cgroups are skipped due to
>- * memory.low and nothing was reclaimed, go back for memory.low.
>+ * Cgroup memory below memory.low is protected as long as we
>+ * don't threaten to OOM. If any cgroup is reclaimed at
>+ * reduced force or passed over entirely due to its memory.low
>+ * setting (memcg_low_skipped), and nothing is reclaimed as a
>+ * result, then go back back for one more cycle that reclaims
>+ * the protected memory (memcg_low_reclaim) to avert OOM.
> */
> unsigned int memcg_low_reclaim:1;
> unsigned int memcg_low_skipped:1;
>@@ -2537,15 +2540,14 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
> for_each_evictable_lru(lru) {
> int file = is_file_lru(lru);
> unsigned long lruvec_size;
>+ unsigned long low, min;
> unsigned long scan;
>- unsigned long protection;
>
> lruvec_size = lruvec_lru_size(lruvec, lru, sc->reclaim_idx);
>- protection = mem_cgroup_protection(sc->target_mem_cgroup,
>- memcg,
>- sc->memcg_low_reclaim);
>+ mem_cgroup_protection(sc->target_mem_cgroup, memcg,
>+ &min, &low);
>
>- if (protection) {
>+ if (min || low) {
> /*
> * Scale a cgroup's reclaim pressure by proportioning
> * its current usage to its memory.low or memory.min
>@@ -2576,6 +2578,15 @@ static void get_scan_count(struct lruvec *lruvec, struct scan_control *sc,
> * hard protection.
> */
> unsigned long cgroup_size = mem_cgroup_size(memcg);
>+ unsigned long protection;
>+
>+ /* memory.low scaling, make sure we retry before OOM */
>+ if (!sc->memcg_low_reclaim && low > min) {
>+ protection = low;
>+ sc->memcg_low_skipped = 1;
>+ } else {
>+ protection = min;
>+ }
>
> /* Avoid TOCTOU with earlier protection check */
> cgroup_size = max(cgroup_size, protection);
>--
>2.32.0
>
next prev parent reply other threads:[~2021-08-18 20:18 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-17 18:05 [PATCH] mm: memcontrol: fix occasional OOMs due to proportional memory.low reclaim Johannes Weiner
2021-08-17 18:44 ` Rik van Riel
2021-08-17 19:10 ` Shakeel Butt
2021-08-18 14:16 ` Johannes Weiner
2021-08-17 19:14 ` Andrew Morton
2021-08-17 19:45 ` Roman Gushchin
2021-08-18 14:15 ` Johannes Weiner
2021-08-18 20:18 ` Chris Down [this message]
2021-08-19 15:01 ` Michal Hocko
2021-08-19 20:38 ` Johannes Weiner
2021-08-20 15:44 ` Michal Hocko
2021-08-23 16:09 ` Michal Koutný
2021-08-23 17:48 ` Johannes Weiner
2021-08-24 13:01 ` Michal Koutný
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=YR1rAoFNegl6CrPz@chrisdown.name \
--to=chris@chrisdown.name \
--cc=akpm@linux-foundation.org \
--cc=cgroups@vger.kernel.org \
--cc=guro@fb.com \
--cc=hannes@cmpxchg.org \
--cc=kernel-team@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=lnyng@fb.com \
--cc=mhocko@suse.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).