[PATCH] vmscan: do not force-scan file lru if its absolute size is small

[PATCH] vmscan: do not force-scan file lru if its absolute size is small

[Vladimir Davydov]

We assume there is enough inactive page cache if the size of inactive
file lru is greater than the size of active file lru, in which case we
force-scan file lru ignoring anonymous pages. While this logic works
fine when there are plenty of page cache pages, it fails if the size of
file lru is small (several MB): in this case (lru_size >> prio) will be
0 for normal scan priorities, as a result, if inactive file lru happens
to be larger than active file lru, anonymous pages of a cgroup will
never get evicted unless the system experiences severe memory pressure,
even if there are gigabytes of unused anonymous memory there, which is
unfair in respect to other cgroups, whose workloads might be page cache
oriented.

This patch attempts to fix this by elaborating the "enough inactive page
cache" check: it makes it not only check that inactive lru size > active
lru size, but also that we will scan something from the cgroup at the
current scan priority. If these conditions do not hold, we proceed to
SCAN_FRACT as usual.

Signed-off-by: Vladimir Davydov <vdavydov@virtuozzo.com>
---
 mm/vmscan.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index bd2918e6391a..2c9b2a053987 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2046,7 +2046,8 @@ static void get_scan_count(struct lruvec *lruvec, int swappiness,
 	 * There is enough inactive page cache, do not reclaim
 	 * anything from the anonymous working set right now.
 	 */
-	if (!inactive_file_is_low(lruvec)) {
+	if (!inactive_file_is_low(lruvec) &&
+	    get_lru_size(lruvec, LRU_INACTIVE_FILE) >> sc->priority > 0) {
 		scan_balance = SCAN_FILE;
 		goto out;
 	}
-- 
2.1.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] vmscan: do not force-scan file lru if its absolute size is small

[Johannes Weiner]

On Fri, Nov 20, 2015 at 08:02:56PM +0300, Vladimir Davydov wrote:
> We assume there is enough inactive page cache if the size of inactive
> file lru is greater than the size of active file lru, in which case we
> force-scan file lru ignoring anonymous pages. While this logic works
> fine when there are plenty of page cache pages, it fails if the size of
> file lru is small (several MB): in this case (lru_size >> prio) will be
> 0 for normal scan priorities, as a result, if inactive file lru happens
> to be larger than active file lru, anonymous pages of a cgroup will
> never get evicted unless the system experiences severe memory pressure,
> even if there are gigabytes of unused anonymous memory there, which is
> unfair in respect to other cgroups, whose workloads might be page cache
> oriented.
> 
> This patch attempts to fix this by elaborating the "enough inactive page
> cache" check: it makes it not only check that inactive lru size > active
> lru size, but also that we will scan something from the cgroup at the
> current scan priority. If these conditions do not hold, we proceed to
> SCAN_FRACT as usual.
> 
> Signed-off-by: Vladimir Davydov <vdavydov@virtuozzo.com>

This makes sense, the inactive:active ratio of the file list alone
does not give the full picture to decide whether to skip anonymous.

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

> @@ -2046,7 +2046,8 @@ static void get_scan_count(struct lruvec *lruvec, int swappiness,
>  	 * There is enough inactive page cache, do not reclaim
>  	 * anything from the anonymous working set right now.
>  	 */
> -	if (!inactive_file_is_low(lruvec)) {
> +	if (!inactive_file_is_low(lruvec) &&
> +	    get_lru_size(lruvec, LRU_INACTIVE_FILE) >> sc->priority > 0) {

The > 0 seems unnecessary, no? There are too many > in this line :-)

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] vmscan: do not force-scan file lru if its absolute size is small

[Andrew Morton]

On Fri, 20 Nov 2015 13:37:07 -0500 Johannes Weiner <hannes@cmpxchg.org> wrote:

> On Fri, Nov 20, 2015 at 08:02:56PM +0300, Vladimir Davydov wrote:
> > We assume there is enough inactive page cache if the size of inactive
> > file lru is greater than the size of active file lru, in which case we
> > force-scan file lru ignoring anonymous pages. While this logic works
> > fine when there are plenty of page cache pages, it fails if the size of
> > file lru is small (several MB): in this case (lru_size >> prio) will be
> > 0 for normal scan priorities, as a result, if inactive file lru happens
> > to be larger than active file lru, anonymous pages of a cgroup will
> > never get evicted unless the system experiences severe memory pressure,
> > even if there are gigabytes of unused anonymous memory there, which is
> > unfair in respect to other cgroups, whose workloads might be page cache
> > oriented.
> > 
> > This patch attempts to fix this by elaborating the "enough inactive page
> > cache" check: it makes it not only check that inactive lru size > active
> > lru size, but also that we will scan something from the cgroup at the
> > current scan priority. If these conditions do not hold, we proceed to
> > SCAN_FRACT as usual.
> > 
> > Signed-off-by: Vladimir Davydov <vdavydov@virtuozzo.com>
> 
> This makes sense, the inactive:active ratio of the file list alone
> does not give the full picture to decide whether to skip anonymous.
> 
> Acked-by: Johannes Weiner <hannes@cmpxchg.org>
> 
> > @@ -2046,7 +2046,8 @@ static void get_scan_count(struct lruvec *lruvec, int swappiness,
> >  	 * There is enough inactive page cache, do not reclaim
> >  	 * anything from the anonymous working set right now.
> >  	 */
> > -	if (!inactive_file_is_low(lruvec)) {
> > +	if (!inactive_file_is_low(lruvec) &&
> > +	    get_lru_size(lruvec, LRU_INACTIVE_FILE) >> sc->priority > 0) {
> 
> The > 0 seems unnecessary, no? There are too many > in this line :-)

And an update to the code comment would be helpful.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH v2] vmscan: do not force-scan file lru if its absolute size is small

[Vladimir Davydov]

We assume there is enough inactive page cache if the size of inactive
file lru is greater than the size of active file lru, in which case we
force-scan file lru ignoring anonymous pages. While this logic works
fine when there are plenty of page cache pages, it fails if the size of
file lru is small (several MB): in this case (lru_size >> prio) will be
0 for normal scan priorities, as a result, if inactive file lru happens
to be larger than active file lru, anonymous pages of a cgroup will
never get evicted unless the system experiences severe memory pressure,
even if there are gigabytes of unused anonymous memory there, which is
unfair in respect to other cgroups, whose workloads might be page cache
oriented.

This patch attempts to fix this by elaborating the "enough inactive page
cache" check: it makes it not only check that inactive lru size > active
lru size, but also that we will scan something from the cgroup at the
current scan priority. If these conditions do not hold, we proceed to
SCAN_FRACT as usual.

Signed-off-by: Vladimir Davydov <vdavydov@virtuozzo.com>
---
Changes in v2:
 - remove unnecessary > 0 (Johannes)
 - elaborate on the comment (Andrew)

 mm/vmscan.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index bd2918e6391a..97ba9e1cde09 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -2043,10 +2043,16 @@ static void get_scan_count(struct lruvec *lruvec, int swappiness,
 	}
 
 	/*
-	 * There is enough inactive page cache, do not reclaim
-	 * anything from the anonymous working set right now.
+	 * If there is enough inactive page cache, i.e. if the size of the
+	 * inactive list is greater than that of the active list *and* the
+	 * inactive list actually has some pages to scan on this priority, we
+	 * do not reclaim anything from the anonymous working set right now.
+	 * Without the second condition we could end up never scanning an
+	 * lruvec even if it has plenty of old anonymous pages unless the
+	 * system is under heavy pressure.
 	 */
-	if (!inactive_file_is_low(lruvec)) {
+	if (!inactive_file_is_low(lruvec) &&
+	    get_lru_size(lruvec, LRU_INACTIVE_FILE) >> sc->priority) {
 		scan_balance = SCAN_FILE;
 		goto out;
 	}
-- 
2.1.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] vmscan: do not force-scan file lru if its absolute size is small

[Michal Hocko]

On Mon 23-11-15 13:39:33, Vladimir Davydov wrote:
> We assume there is enough inactive page cache if the size of inactive
> file lru is greater than the size of active file lru, in which case we
> force-scan file lru ignoring anonymous pages. While this logic works
> fine when there are plenty of page cache pages, it fails if the size of
> file lru is small (several MB): in this case (lru_size >> prio) will be
> 0 for normal scan priorities, as a result, if inactive file lru happens
> to be larger than active file lru, anonymous pages of a cgroup will
> never get evicted unless the system experiences severe memory pressure,
> even if there are gigabytes of unused anonymous memory there, which is
> unfair in respect to other cgroups, whose workloads might be page cache
> oriented.
> 
> This patch attempts to fix this by elaborating the "enough inactive page
> cache" check: it makes it not only check that inactive lru size > active
> lru size, but also that we will scan something from the cgroup at the
> current scan priority. If these conditions do not hold, we proceed to
> SCAN_FRACT as usual.

Yes this makes sense. FWIW I have a similar patch waiting for feedback
from testing which catches the other extreme case when we force anon
pages scan without any progress from the kswapd context (I hope I get to
post it soon).

get_scan_count is getting more and more convoluted :/

> Signed-off-by: Vladimir Davydov <vdavydov@virtuozzo.com>

Acked-by: Michal Hocko <mhocko@suse.com>

Thanks!

> ---
> Changes in v2:
>  - remove unnecessary > 0 (Johannes)
>  - elaborate on the comment (Andrew)
> 
>  mm/vmscan.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index bd2918e6391a..97ba9e1cde09 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -2043,10 +2043,16 @@ static void get_scan_count(struct lruvec *lruvec, int swappiness,
>  	}
>  
>  	/*
> -	 * There is enough inactive page cache, do not reclaim
> -	 * anything from the anonymous working set right now.
> +	 * If there is enough inactive page cache, i.e. if the size of the
> +	 * inactive list is greater than that of the active list *and* the
> +	 * inactive list actually has some pages to scan on this priority, we
> +	 * do not reclaim anything from the anonymous working set right now.
> +	 * Without the second condition we could end up never scanning an
> +	 * lruvec even if it has plenty of old anonymous pages unless the
> +	 * system is under heavy pressure.
>  	 */
> -	if (!inactive_file_is_low(lruvec)) {
> +	if (!inactive_file_is_low(lruvec) &&
> +	    get_lru_size(lruvec, LRU_INACTIVE_FILE) >> sc->priority) {
>  		scan_balance = SCAN_FILE;
>  		goto out;
>  	}
> -- 
> 2.1.4
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2] vmscan: do not force-scan file lru if its absolute size is small

[Johannes Weiner]

On Mon, Nov 23, 2015 at 01:39:33PM +0300, Vladimir Davydov wrote:
> We assume there is enough inactive page cache if the size of inactive
> file lru is greater than the size of active file lru, in which case we
> force-scan file lru ignoring anonymous pages. While this logic works
> fine when there are plenty of page cache pages, it fails if the size of
> file lru is small (several MB): in this case (lru_size >> prio) will be
> 0 for normal scan priorities, as a result, if inactive file lru happens
> to be larger than active file lru, anonymous pages of a cgroup will
> never get evicted unless the system experiences severe memory pressure,
> even if there are gigabytes of unused anonymous memory there, which is
> unfair in respect to other cgroups, whose workloads might be page cache
> oriented.
> 
> This patch attempts to fix this by elaborating the "enough inactive page
> cache" check: it makes it not only check that inactive lru size > active
> lru size, but also that we will scan something from the cgroup at the
> current scan priority. If these conditions do not hold, we proceed to
> SCAN_FRACT as usual.
> 
> Signed-off-by: Vladimir Davydov <vdavydov@virtuozzo.com>

Acked-by: Johannes Weiner <hannes@cmpxchg.org>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>