[1/2] mm: memcg: rename and document global_reclaim()

Commit Message

Yosry Ahmed June 21, 2023, 2:30 a.m. UTC

Evidently, global_reclaim() can be a confusing name. Especially that it
used to exist before with a subtly different definition (removed by
commit b5ead35e7e1d ("mm: vmscan: naming fixes: global_reclaim() and
sane_reclaim()"). It can be interpreted as non-cgroup reclaim, even
though it returns true for cgroup reclaim on the root memcg
(through memory.reclaim).

Rename it to root_reclaim() in an attempt to make it less ambiguous, and
add documentation to it as well as cgroup_reclaim.

Reported-by: Johannes Weiner <[e-mail]>
Closes: https://lore.kernel.org/lkml/[e-mail]/
Signed-off-by: Yosry Ahmed <[e-mail]>
---
 mm/vmscan.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

Comments

Yu Zhao June 23, 2023, 8:39 p.m. UTC | #1

On Tue, Jun 20, 2023 at 8:30 PM Yosry Ahmed <[e-mail]> wrote:
>
> Evidently, global_reclaim() can be a confusing name. Especially that it
> used to exist before with a subtly different definition (removed by
> commit b5ead35e7e1d ("mm: vmscan: naming fixes: global_reclaim() and
> sane_reclaim()"). It can be interpreted as non-cgroup reclaim, even
> though it returns true for cgroup reclaim on the root memcg
> (through memory.reclaim).
>
> Rename it to root_reclaim() in an attempt to make it less ambiguous, and
> add documentation to it as well as cgroup_reclaim.
>
> Reported-by: Johannes Weiner <[e-mail]>
> Closes: https://lore.kernel.org/lkml/[e-mail]/
> Signed-off-by: Yosry Ahmed <[e-mail]>

Acked-by: Yu Zhao <[e-mail]>

Patch

diff --git a/mm/vmscan.c b/mm/vmscan.c
index e305c11ec8fc..0dbbf718c53e 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -445,12 +445,17 @@  void reparent_shrinker_deferred(struct mem_cgroup *memcg)
 	mutex_unlock(&shrinker_mutex);
 }
 
+/* Returns true for reclaim through cgroup limits or cgroup interfaces. */
 static bool cgroup_reclaim(struct scan_control *sc)
 {
 	return sc->target_mem_cgroup;
 }
 
-static bool global_reclaim(struct scan_control *sc)
+/*
+ * Returns true for reclaim on the root cgroup. This is true for direct
+ * allocator reclaim and reclaim through cgroup interfaces on the root cgroup.
+ */
+static bool root_reclaim(struct scan_control *sc)
 {
 	return !sc->target_mem_cgroup || mem_cgroup_is_root(sc->target_mem_cgroup);
 }
@@ -505,7 +510,7 @@  static bool cgroup_reclaim(struct scan_control *sc)
 	return false;
 }
 
-static bool global_reclaim(struct scan_control *sc)
+static bool root_reclaim(struct scan_control *sc)
 {
 	return true;
 }
@@ -562,7 +567,7 @@  static void flush_reclaim_state(struct scan_control *sc)
 	 * memcg reclaim, to make reporting more accurate and reduce
 	 * underestimation, but it's probably not worth the complexity for now.
 	 */
-	if (current->reclaim_state && global_reclaim(sc)) {
+	if (current->reclaim_state && root_reclaim(sc)) {
 		sc->nr_reclaimed += current->reclaim_state->reclaimed;
 		current->reclaim_state->reclaimed = 0;
 	}
@@ -5339,7 +5344,7 @@  static long get_nr_to_scan(struct lruvec *lruvec, struct scan_control *sc, bool
 static unsigned long get_nr_to_reclaim(struct scan_control *sc)
 {
 	/* don't abort memcg reclaim to ensure fairness */
-	if (!global_reclaim(sc))
+	if (!root_reclaim(sc))
 		return -1;
 
 	return max(sc->nr_to_reclaim, compact_gap(sc->order));
@@ -5491,7 +5496,7 @@  static void lru_gen_shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc
 {
 	struct blk_plug plug;
 
-	VM_WARN_ON_ONCE(global_reclaim(sc));
+	VM_WARN_ON_ONCE(root_reclaim(sc));
 	VM_WARN_ON_ONCE(!sc->may_writepage || !sc->may_unmap);
 
 	lru_add_drain();
@@ -5552,7 +5557,7 @@  static void lru_gen_shrink_node(struct pglist_data *pgdat, struct scan_control *
 	struct blk_plug plug;
 	unsigned long reclaimed = sc->nr_reclaimed;
 
-	VM_WARN_ON_ONCE(!global_reclaim(sc));
+	VM_WARN_ON_ONCE(!root_reclaim(sc));
 
 	/*
 	 * Unmapped clean folios are already prioritized. Scanning for more of
@@ -6274,7 +6279,7 @@  static void shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
 	bool proportional_reclaim;
 	struct blk_plug plug;
 
-	if (lru_gen_enabled() && !global_reclaim(sc)) {
+	if (lru_gen_enabled() && !root_reclaim(sc)) {
 		lru_gen_shrink_lruvec(lruvec, sc);
 		return;
 	}
@@ -6515,7 +6520,7 @@  static void shrink_node(pg_data_t *pgdat, struct scan_control *sc)
 	struct lruvec *target_lruvec;
 	bool reclaimable = false;
 
-	if (lru_gen_enabled() && global_reclaim(sc)) {
+	if (lru_gen_enabled() && root_reclaim(sc)) {
 		lru_gen_shrink_node(pgdat, sc);
 		return;
 	}