2021-04-01 18:52:58

by Dave Hansen

[permalink] [raw]
Subject: [PATCH 07/10] mm/vmscan: add helper for querying ability to age anonymous pages


From: Dave Hansen <[email protected]>

Anonymous pages are kept on their own LRU(s). These lists could
theoretically always be scanned and maintained. But, without swap,
there is currently nothing the kernel can *do* with the results of a
scanned, sorted LRU for anonymous pages.

A check for '!total_swap_pages' currently serves as a valid check as
to whether anonymous LRUs should be maintained. However, another
method will be added shortly: page demotion.

Abstract out the 'total_swap_pages' checks into a helper, give it a
logically significant name, and check for the possibility of page
demotion.

Signed-off-by: Dave Hansen <[email protected]>
Reviewed-by: Yang Shi <[email protected]>
Reviewed-by: Greg Thelen <[email protected]>
Cc: Wei Xu <[email protected]>
Cc: David Rientjes <[email protected]>
Cc: Huang Ying <[email protected]>
Cc: Dan Williams <[email protected]>
Cc: David Hildenbrand <[email protected]>
Cc: osalvador <[email protected]>
---

b/mm/vmscan.c | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)

diff -puN mm/vmscan.c~mm-vmscan-anon-can-be-aged mm/vmscan.c
--- a/mm/vmscan.c~mm-vmscan-anon-can-be-aged 2021-03-31 15:17:18.325000245 -0700
+++ b/mm/vmscan.c 2021-03-31 15:17:18.333000245 -0700
@@ -2508,6 +2508,26 @@ out:
}
}

+/*
+ * Anonymous LRU management is a waste if there is
+ * ultimately no way to reclaim the memory.
+ */
+bool anon_should_be_aged(struct lruvec *lruvec)
+{
+ struct pglist_data *pgdat = lruvec_pgdat(lruvec);
+
+ /* Aging the anon LRU is valuable if swap is present: */
+ if (total_swap_pages > 0)
+ return true;
+
+ /* Also valuable if anon pages can be demoted: */
+ if (next_demotion_node(pgdat->node_id) >= 0)
+ return true;
+
+ /* No way to reclaim anon pages. Should not age anon LRUs: */
+ return false;
+}
+
static void shrink_lruvec(struct lruvec *lruvec, struct scan_control *sc)
{
unsigned long nr[NR_LRU_LISTS];
@@ -2617,7 +2637,8 @@ static void shrink_lruvec(struct lruvec
* Even if we did not try to evict anon pages at all, we want to
* rebalance the anon lru active/inactive ratio.
*/
- if (total_swap_pages && inactive_is_low(lruvec, LRU_INACTIVE_ANON))
+ if (anon_should_be_aged(lruvec) &&
+ inactive_is_low(lruvec, LRU_INACTIVE_ANON))
shrink_active_list(SWAP_CLUSTER_MAX, lruvec,
sc, LRU_ACTIVE_ANON);
}
@@ -3446,10 +3467,11 @@ static void age_active_anon(struct pglis
struct mem_cgroup *memcg;
struct lruvec *lruvec;

- if (!total_swap_pages)
+ lruvec = mem_cgroup_lruvec(NULL, pgdat);
+
+ if (!anon_should_be_aged(lruvec))
return;

- lruvec = mem_cgroup_lruvec(NULL, pgdat);
if (!inactive_is_low(lruvec, LRU_INACTIVE_ANON))
return;

_


2021-04-07 21:54:16

by Wei Xu

[permalink] [raw]
Subject: Re: [PATCH 07/10] mm/vmscan: add helper for querying ability to age anonymous pages

> +/*
> + * Anonymous LRU management is a waste if there is
> + * ultimately no way to reclaim the memory.
> + */
> +bool anon_should_be_aged(struct lruvec *lruvec)
> +{
> + struct pglist_data *pgdat = lruvec_pgdat(lruvec);
> +
> + /* Aging the anon LRU is valuable if swap is present: */
> + if (total_swap_pages > 0)
> + return true;
> +
> + /* Also valuable if anon pages can be demoted: */
> + if (next_demotion_node(pgdat->node_id) >= 0)
> + return true;
> +
> + /* No way to reclaim anon pages. Should not age anon LRUs: */
> + return false;
> +}

anon_should_be_aged() doesn't really need "lruvec". It essentially
answers whether the pages of the given node can be swapped or demoted.
So it would be clearer and less confusing if anon_should_be_aged()
takes "pgdat" instead of "lruvec" as the argument. The call to
mem_cgroup_lruvec(NULL, pgdat) in age_active_anon() can then be removed
as well.

2021-04-09 08:35:55

by Oscar Salvador

[permalink] [raw]
Subject: Re: [PATCH 07/10] mm/vmscan: add helper for querying ability to age anonymous pages

On Wed, Apr 07, 2021 at 11:40:13AM -0700, Wei Xu wrote:
> anon_should_be_aged() doesn't really need "lruvec". It essentially
> answers whether the pages of the given node can be swapped or demoted.
> So it would be clearer and less confusing if anon_should_be_aged()
> takes "pgdat" instead of "lruvec" as the argument. The call to
> mem_cgroup_lruvec(NULL, pgdat) in age_active_anon() can then be removed
> as well.

I tend to agree with this, and I would go one step further with the naming.
For me, taking into account the nature of the function that tells us whether
we have any means to age those pages, a better fit would be something like
anon_can_be_aged(). IIUC, the "should age" part would be inactive_is_low().


--
Oscar Salvador
SUSE L3