2024-01-11 12:25:43

by zhiguojiang

[permalink] [raw]
Subject: [PATCH v1 2/2] mm:vmscan: fix shrink sc parameters issue

The values of struct scan_control's members obtained by
prepare_scan_control() are always from root_mem_cgroup, rather than
the memcg where the shrinked folio is actually located, such as
sc->anon_cost, sc->file_cost, sc->may_deactivate and so on. This is
an inaccurate sc values for the actual situation of the current
shrinking memcg. so we need fix the values of struct scan_control's
members are corresponding to the current shrinking memcg.

Signed-off-by: Zhiguo Jiang <[email protected]>
---
mm/vmscan.c | 35 ++++++++++++++++++-----------------
1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 8a1fbdaca042..31001123e8f1 100755
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2229,29 +2229,30 @@ enum scan_balance {
SCAN_FILE,
};

-static void prepare_scan_control(pg_data_t *pgdat, struct scan_control *sc)
+static void prepare_scan_control(pg_data_t *pgdat, struct scan_control *sc,
+ struct mem_cgroup *memcg)
{
unsigned long file;
- struct lruvec *target_lruvec;
+ struct lruvec *lruvec;

if (lru_gen_enabled())
return;

- target_lruvec = mem_cgroup_lruvec(sc->target_mem_cgroup, pgdat);
+ lruvec = mem_cgroup_lruvec(memcg, pgdat);

/*
* Flush the memory cgroup stats, so that we read accurate per-memcg
* lruvec stats for heuristics.
*/
- mem_cgroup_flush_stats(sc->target_mem_cgroup);
+ mem_cgroup_flush_stats(memcg);

/*
* Determine the scan balance between anon and file LRUs.
*/
- spin_lock_irq(&target_lruvec->lru_lock);
- sc->anon_cost = target_lruvec->anon_cost;
- sc->file_cost = target_lruvec->file_cost;
- spin_unlock_irq(&target_lruvec->lru_lock);
+ spin_lock_irq(&lruvec->lru_lock);
+ sc->anon_cost = lruvec->anon_cost;
+ sc->file_cost = lruvec->file_cost;
+ spin_unlock_irq(&lruvec->lru_lock);

/*
* Target desirable inactive:active list ratios for the anon
@@ -2265,18 +2266,18 @@ static void prepare_scan_control(pg_data_t *pgdat, struct scan_control *sc)
* workingset is being established. Deactivate to get
* rid of any stale active pages quickly.
*/
- refaults = lruvec_page_state(target_lruvec,
+ refaults = lruvec_page_state(lruvec,
WORKINGSET_ACTIVATE_ANON);
- if (refaults != target_lruvec->refaults[WORKINGSET_ANON] ||
- inactive_is_low(target_lruvec, LRU_INACTIVE_ANON))
+ if (refaults != lruvec->refaults[WORKINGSET_ANON] ||
+ inactive_is_low(lruvec, LRU_INACTIVE_ANON))
sc->may_deactivate |= DEACTIVATE_ANON;
else
sc->may_deactivate &= ~DEACTIVATE_ANON;

- refaults = lruvec_page_state(target_lruvec,
+ refaults = lruvec_page_state(lruvec,
WORKINGSET_ACTIVATE_FILE);
- if (refaults != target_lruvec->refaults[WORKINGSET_FILE] ||
- inactive_is_low(target_lruvec, LRU_INACTIVE_FILE))
+ if (refaults != lruvec->refaults[WORKINGSET_FILE] ||
+ inactive_is_low(lruvec, LRU_INACTIVE_FILE))
sc->may_deactivate |= DEACTIVATE_FILE;
else
sc->may_deactivate &= ~DEACTIVATE_FILE;
@@ -2288,7 +2289,7 @@ static void prepare_scan_control(pg_data_t *pgdat, struct scan_control *sc)
* thrashing, try to reclaim those first before touching
* anonymous pages.
*/
- file = lruvec_page_state(target_lruvec, NR_INACTIVE_FILE);
+ file = lruvec_page_state(lruvec, NR_INACTIVE_FILE);
if (file >> sc->priority && !(sc->may_deactivate & DEACTIVATE_FILE))
sc->cache_trim_mode = 1;
else
@@ -5885,6 +5886,8 @@ static void shrink_node_memcgs(pg_data_t *pgdat, struct scan_control *sc)
reclaimed = sc->nr_reclaimed;
scanned = sc->nr_scanned;

+ prepare_scan_control(pgdat, sc, memcg);
+
shrink_lruvec(lruvec, sc);

shrink_slab(sc->gfp_mask, pgdat->node_id, memcg,
@@ -5918,8 +5921,6 @@ static void shrink_node(pg_data_t *pgdat, struct scan_control *sc)
nr_reclaimed = sc->nr_reclaimed;
nr_scanned = sc->nr_scanned;

- prepare_scan_control(pgdat, sc);
-
shrink_node_memcgs(pgdat, sc);

flush_reclaim_state(sc);
--
2.39.0



2024-01-11 13:43:49

by Johannes Weiner

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] mm:vmscan: fix shrink sc parameters issue

On Thu, Jan 11, 2024 at 08:24:51PM +0800, Zhiguo Jiang wrote:
> The values of struct scan_control's members obtained by
> prepare_scan_control() are always from root_mem_cgroup, rather than
> the memcg where the shrinked folio is actually located, such as
> sc->anon_cost, sc->file_cost, sc->may_deactivate and so on. This is
> an inaccurate sc values for the actual situation of the current
> shrinking memcg. so we need fix the values of struct scan_control's
> members are corresponding to the current shrinking memcg.

You don't seem to understand how cgroup reclaim works.

Nacked-by: Johannes Weiner <[email protected]>

2024-01-12 01:28:14

by zhiguojiang

[permalink] [raw]
Subject: Re: [PATCH v1 2/2] mm:vmscan: fix shrink sc parameters issue



在 2024/1/11 21:43, Johannes Weiner 写道:
> On Thu, Jan 11, 2024 at 08:24:51PM +0800, Zhiguo Jiang wrote:
>> The values of struct scan_control's members obtained by
>> prepare_scan_control() are always from root_mem_cgroup, rather than
>> the memcg where the shrinked folio is actually located, such as
>> sc->anon_cost, sc->file_cost, sc->may_deactivate and so on. This is
>> an inaccurate sc values for the actual situation of the current
>> shrinking memcg. so we need fix the values of struct scan_control's
>> members are corresponding to the current shrinking memcg.
> You don't seem to understand how cgroup reclaim works.
It seems that the actual memcg's lruvec->anon_cost/file_cost values are
not used in code exclude target_memcg ?
>
> Nacked-by: Johannes Weiner <[email protected]>