diff mbox series

[v6,1/6] mm/vmscan: make active/inactive ratio as 1:1 for anon lru

Message ID 1592371583-30672-2-git-send-email-iamjoonsoo.kim@lge.com (mailing list archive)
State New, archived
Headers show
Series workingset protection/detection on the anonymous LRU list | expand

Commit Message

Joonsoo Kim June 17, 2020, 5:26 a.m. UTC
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Current implementation of LRU management for anonymous page has some
problems. Most important one is that it doesn't protect the workingset,
that is, pages on the active LRU list. Although, this problem will be
fixed in the following patchset, the preparation is required and
this patch does it.

What following patchset does is to restore workingset protection. In this
case, newly created or swap-in pages are started their lifetime on the
inactive list. If inactive list is too small, there is not enough chance
to be referenced and the page cannot become the workingset.

In order to provide enough chance to the newly anonymous pages, this patch
makes active/inactive LRU ratio as 1:1.

Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
---
 mm/vmscan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Vlastimil Babka June 30, 2020, 5:27 p.m. UTC | #1
On 6/17/20 7:26 AM, js1304@gmail.com wrote:
> From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> 
> Current implementation of LRU management for anonymous page has some
> problems. Most important one is that it doesn't protect the workingset,
> that is, pages on the active LRU list. Although, this problem will be
> fixed in the following patchset, the preparation is required and
> this patch does it.
> 
> What following patchset does is to restore workingset protection. In this

"Restore" sounds as if the protection used to be there and then it was removed.
If it's the case, it should be said what commit did that. Otherwise I would say
"implement", not "restore"?

> case, newly created or swap-in pages are started their lifetime on the

I would rephrase it: "After the following patch, newly created or swap-in pages
will start their lifetime... "

> inactive list. If inactive list is too small, there is not enough chance
> to be referenced and the page cannot become the workingset.
> 
> In order to provide enough chance to the newly anonymous pages, this patch

"In order to provide the newly anonymous pages enough chance to be referenced
again..."

> makes active/inactive LRU ratio as 1:1.

Here I would add:

This is just a temporary measure. Later patch in the series introduces
workingset detection for anonymous LRU that will be used to better decide if
pages should start on the active and inactive list. Afterwards this patch is
effectively reverted.

> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>

Acked-by: Vlastimil Babka <vbabka@suse.cz>

> ---
>  mm/vmscan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 749d239..9f940c4 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2212,7 +2212,7 @@ static bool inactive_is_low(struct lruvec *lruvec, enum lru_list inactive_lru)
>  	active = lruvec_page_state(lruvec, NR_LRU_BASE + active_lru);
>  
>  	gb = (inactive + active) >> (30 - PAGE_SHIFT);
> -	if (gb)
> +	if (gb && is_file_lru(inactive_lru))
>  		inactive_ratio = int_sqrt(10 * gb);
>  	else
>  		inactive_ratio = 1;
>
Joonsoo Kim July 1, 2020, 6:18 a.m. UTC | #2
2020년 7월 1일 (수) 오전 2:27, Vlastimil Babka <vbabka@suse.cz>님이 작성:
>
> On 6/17/20 7:26 AM, js1304@gmail.com wrote:
> > From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> >
> > Current implementation of LRU management for anonymous page has some
> > problems. Most important one is that it doesn't protect the workingset,
> > that is, pages on the active LRU list. Although, this problem will be
> > fixed in the following patchset, the preparation is required and
> > this patch does it.
> >
> > What following patchset does is to restore workingset protection. In this
>
> "Restore" sounds as if the protection used to be there and then it was removed.
> If it's the case, it should be said what commit did that. Otherwise I would say
> "implement", not "restore"?
>
> > case, newly created or swap-in pages are started their lifetime on the
>
> I would rephrase it: "After the following patch, newly created or swap-in pages
> will start their lifetime... "
>
> > inactive list. If inactive list is too small, there is not enough chance
> > to be referenced and the page cannot become the workingset.
> >
> > In order to provide enough chance to the newly anonymous pages, this patch
>
> "In order to provide the newly anonymous pages enough chance to be referenced
> again..."
>
> > makes active/inactive LRU ratio as 1:1.
>
> Here I would add:
>
> This is just a temporary measure. Later patch in the series introduces
> workingset detection for anonymous LRU that will be used to better decide if
> pages should start on the active and inactive list. Afterwards this patch is
> effectively reverted.
>
> > Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> > Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>

Thanks!

I will change the commit description as you suggested.

Thanks.
diff mbox series

Patch

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 749d239..9f940c4 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2212,7 +2212,7 @@  static bool inactive_is_low(struct lruvec *lruvec, enum lru_list inactive_lru)
 	active = lruvec_page_state(lruvec, NR_LRU_BASE + active_lru);
 
 	gb = (inactive + active) >> (30 - PAGE_SHIFT);
-	if (gb)
+	if (gb && is_file_lru(inactive_lru))
 		inactive_ratio = int_sqrt(10 * gb);
 	else
 		inactive_ratio = 1;