[patch 1/2] mm: vmscan: fix force-scanning small targets without swap

[patch 1/2] mm: vmscan: fix force-scanning small targets without swap

[Johannes Weiner]

Without swap, anonymous pages are not scanned.  As such, they should
not count when considering force-scanning a small target if there is
no swap.

Otherwise, targets are not force-scanned even when their effective
scan number is zero and the other conditions--kswapd/memcg--apply.

Signed-off-by: Johannes Weiner <jweiner@redhat.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Ying Han <yinghan@google.com>
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
Cc: Mel Gorman <mel@csn.ul.ie>
---
 mm/vmscan.c |   27 ++++++++++++---------------
 1 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 3153729..96061d7 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1830,23 +1830,15 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 	u64 fraction[2], denominator;
 	enum lru_list l;
 	int noswap = 0;
-	int force_scan = 0;
+	bool force_scan = false;
 	unsigned long nr_force_scan[2];
 
-
-	anon  = zone_nr_lru_pages(zone, sc, LRU_ACTIVE_ANON) +
-		zone_nr_lru_pages(zone, sc, LRU_INACTIVE_ANON);
-	file  = zone_nr_lru_pages(zone, sc, LRU_ACTIVE_FILE) +
-		zone_nr_lru_pages(zone, sc, LRU_INACTIVE_FILE);
-
-	if (((anon + file) >> priority) < SWAP_CLUSTER_MAX) {
-		/* kswapd does zone balancing and need to scan this zone */
-		if (scanning_global_lru(sc) && current_is_kswapd())
-			force_scan = 1;
-		/* memcg may have small limit and need to avoid priority drop */
-		if (!scanning_global_lru(sc))
-			force_scan = 1;
-	}
+	/* kswapd does zone balancing and need to scan this zone */
+	if (scanning_global_lru(sc) && current_is_kswapd())
+		force_scan = true;
+	/* memcg may have small limit and need to avoid priority drop */
+	if (!scanning_global_lru(sc))
+		force_scan = true;
 
 	/* If we have no swap space, do not bother scanning anon pages. */
 	if (!sc->may_swap || (nr_swap_pages <= 0)) {
@@ -1859,6 +1851,11 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 		goto out;
 	}
 
+	anon  = zone_nr_lru_pages(zone, sc, LRU_ACTIVE_ANON) +
+		zone_nr_lru_pages(zone, sc, LRU_INACTIVE_ANON);
+	file  = zone_nr_lru_pages(zone, sc, LRU_ACTIVE_FILE) +
+		zone_nr_lru_pages(zone, sc, LRU_INACTIVE_FILE);
+
 	if (scanning_global_lru(sc)) {
 		free  = zone_page_state(zone, NR_FREE_PAGES);
 		/* If we have very few page cache pages,
-- 
1.7.6

[patch 1/2] mm: vmscan: fix force-scanning small targets without swap

[Johannes Weiner]

Without swap, anonymous pages are not scanned.  As such, they should
not count when considering force-scanning a small target if there is
no swap.

Otherwise, targets are not force-scanned even when their effective
scan number is zero and the other conditions--kswapd/memcg--apply.

Signed-off-by: Johannes Weiner <jweiner@redhat.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Ying Han <yinghan@google.com>
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
Cc: Mel Gorman <mel@csn.ul.ie>
---
 mm/vmscan.c |   27 ++++++++++++---------------
 1 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 3153729..96061d7 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1830,23 +1830,15 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 	u64 fraction[2], denominator;
 	enum lru_list l;
 	int noswap = 0;
-	int force_scan = 0;
+	bool force_scan = false;
 	unsigned long nr_force_scan[2];
 
-
-	anon  = zone_nr_lru_pages(zone, sc, LRU_ACTIVE_ANON) +
-		zone_nr_lru_pages(zone, sc, LRU_INACTIVE_ANON);
-	file  = zone_nr_lru_pages(zone, sc, LRU_ACTIVE_FILE) +
-		zone_nr_lru_pages(zone, sc, LRU_INACTIVE_FILE);
-
-	if (((anon + file) >> priority) < SWAP_CLUSTER_MAX) {
-		/* kswapd does zone balancing and need to scan this zone */
-		if (scanning_global_lru(sc) && current_is_kswapd())
-			force_scan = 1;
-		/* memcg may have small limit and need to avoid priority drop */
-		if (!scanning_global_lru(sc))
-			force_scan = 1;
-	}
+	/* kswapd does zone balancing and need to scan this zone */
+	if (scanning_global_lru(sc) && current_is_kswapd())
+		force_scan = true;
+	/* memcg may have small limit and need to avoid priority drop */
+	if (!scanning_global_lru(sc))
+		force_scan = true;
 
 	/* If we have no swap space, do not bother scanning anon pages. */
 	if (!sc->may_swap || (nr_swap_pages <= 0)) {
@@ -1859,6 +1851,11 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 		goto out;
 	}
 
+	anon  = zone_nr_lru_pages(zone, sc, LRU_ACTIVE_ANON) +
+		zone_nr_lru_pages(zone, sc, LRU_INACTIVE_ANON);
+	file  = zone_nr_lru_pages(zone, sc, LRU_ACTIVE_FILE) +
+		zone_nr_lru_pages(zone, sc, LRU_INACTIVE_FILE);
+
 	if (scanning_global_lru(sc)) {
 		free  = zone_page_state(zone, NR_FREE_PAGES);
 		/* If we have very few page cache pages,
-- 
1.7.6

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch 2/2] mm: vmscan: drop nr_force_scan[] from get_scan_count

[Johannes Weiner]

The nr_force_scan[] tuple holds the effective scan numbers for anon
and file pages in case the situation called for a forced scan and the
regularly calculated scan numbers turned out zero.

However, the effective scan number can always be assumed to be
SWAP_CLUSTER_MAX right before the division into anon and file.  The
numerators and denominator are properly set up for all cases, be it
force scan for just file, just anon, or both, to do the right thing.

Signed-off-by: Johannes Weiner <jweiner@redhat.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Ying Han <yinghan@google.com>
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
Cc: Mel Gorman <mel@csn.ul.ie>
---
 mm/vmscan.c |   24 ++----------------------
 1 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 96061d7..45f0986 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1831,7 +1831,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 	enum lru_list l;
 	int noswap = 0;
 	bool force_scan = false;
-	unsigned long nr_force_scan[2];
 
 	/* kswapd does zone balancing and need to scan this zone */
 	if (scanning_global_lru(sc) && current_is_kswapd())
@@ -1846,8 +1845,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 		fraction[0] = 0;
 		fraction[1] = 1;
 		denominator = 1;
-		nr_force_scan[0] = 0;
-		nr_force_scan[1] = SWAP_CLUSTER_MAX;
 		goto out;
 	}
 
@@ -1864,8 +1861,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 			fraction[0] = 1;
 			fraction[1] = 0;
 			denominator = 1;
-			nr_force_scan[0] = SWAP_CLUSTER_MAX;
-			nr_force_scan[1] = 0;
 			goto out;
 		}
 	}
@@ -1914,11 +1909,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 	fraction[0] = ap;
 	fraction[1] = fp;
 	denominator = ap + fp + 1;
-	if (force_scan) {
-		unsigned long scan = SWAP_CLUSTER_MAX;
-		nr_force_scan[0] = div64_u64(scan * ap, denominator);
-		nr_force_scan[1] = div64_u64(scan * fp, denominator);
-	}
 out:
 	for_each_evictable_lru(l) {
 		int file = is_file_lru(l);
@@ -1927,20 +1917,10 @@ out:
 		scan = zone_nr_lru_pages(zone, sc, l);
 		if (priority || noswap) {
 			scan >>= priority;
+			if (!scan && force_scan)
+				scan = SWAP_CLUSTER_MAX;
 			scan = div64_u64(scan * fraction[file], denominator);
 		}
-
-		/*
-		 * If zone is small or memcg is small, nr[l] can be 0.
-		 * This results no-scan on this priority and priority drop down.
-		 * For global direct reclaim, it can visit next zone and tend
-		 * not to have problems. For global kswapd, it's for zone
-		 * balancing and it need to scan a small amounts. When using
-		 * memcg, priority drop can cause big latency. So, it's better
-		 * to scan small amount. See may_noscan above.
-		 */
-		if (!scan && force_scan)
-			scan = nr_force_scan[file];
 		nr[l] = scan;
 	}
 }
-- 
1.7.6

[patch 2/2] mm: vmscan: drop nr_force_scan[] from get_scan_count

[Johannes Weiner]

The nr_force_scan[] tuple holds the effective scan numbers for anon
and file pages in case the situation called for a forced scan and the
regularly calculated scan numbers turned out zero.

However, the effective scan number can always be assumed to be
SWAP_CLUSTER_MAX right before the division into anon and file.  The
numerators and denominator are properly set up for all cases, be it
force scan for just file, just anon, or both, to do the right thing.

Signed-off-by: Johannes Weiner <jweiner@redhat.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Ying Han <yinghan@google.com>
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
Cc: Mel Gorman <mel@csn.ul.ie>
---
 mm/vmscan.c |   24 ++----------------------
 1 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 96061d7..45f0986 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1831,7 +1831,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 	enum lru_list l;
 	int noswap = 0;
 	bool force_scan = false;
-	unsigned long nr_force_scan[2];
 
 	/* kswapd does zone balancing and need to scan this zone */
 	if (scanning_global_lru(sc) && current_is_kswapd())
@@ -1846,8 +1845,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 		fraction[0] = 0;
 		fraction[1] = 1;
 		denominator = 1;
-		nr_force_scan[0] = 0;
-		nr_force_scan[1] = SWAP_CLUSTER_MAX;
 		goto out;
 	}
 
@@ -1864,8 +1861,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 			fraction[0] = 1;
 			fraction[1] = 0;
 			denominator = 1;
-			nr_force_scan[0] = SWAP_CLUSTER_MAX;
-			nr_force_scan[1] = 0;
 			goto out;
 		}
 	}
@@ -1914,11 +1909,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 	fraction[0] = ap;
 	fraction[1] = fp;
 	denominator = ap + fp + 1;
-	if (force_scan) {
-		unsigned long scan = SWAP_CLUSTER_MAX;
-		nr_force_scan[0] = div64_u64(scan * ap, denominator);
-		nr_force_scan[1] = div64_u64(scan * fp, denominator);
-	}
 out:
 	for_each_evictable_lru(l) {
 		int file = is_file_lru(l);
@@ -1927,20 +1917,10 @@ out:
 		scan = zone_nr_lru_pages(zone, sc, l);
 		if (priority || noswap) {
 			scan >>= priority;
+			if (!scan && force_scan)
+				scan = SWAP_CLUSTER_MAX;
 			scan = div64_u64(scan * fraction[file], denominator);
 		}
-
-		/*
-		 * If zone is small or memcg is small, nr[l] can be 0.
-		 * This results no-scan on this priority and priority drop down.
-		 * For global direct reclaim, it can visit next zone and tend
-		 * not to have problems. For global kswapd, it's for zone
-		 * balancing and it need to scan a small amounts. When using
-		 * memcg, priority drop can cause big latency. So, it's better
-		 * to scan small amount. See may_noscan above.
-		 */
-		if (!scan && force_scan)
-			scan = nr_force_scan[file];
 		nr[l] = scan;
 	}
 }
-- 
1.7.6

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/2] mm: vmscan: fix force-scanning small targets without swap

[Minchan Kim]

On Fri, Aug 12, 2011 at 5:31 AM, Johannes Weiner <jweiner@redhat.com> wrote:
> Without swap, anonymous pages are not scanned.  As such, they should
> not count when considering force-scanning a small target if there is
> no swap.
>
> Otherwise, targets are not force-scanned even when their effective
> scan number is zero and the other conditions--kswapd/memcg--apply.
>
> Signed-off-by: Johannes Weiner <jweiner@redhat.com>
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

Good catch!


-- 
Kind regards,
Minchan Kim

Re: [patch 1/2] mm: vmscan: fix force-scanning small targets without swap

[Minchan Kim]

On Fri, Aug 12, 2011 at 5:31 AM, Johannes Weiner <jweiner@redhat.com> wrote:
> Without swap, anonymous pages are not scanned.  As such, they should
> not count when considering force-scanning a small target if there is
> no swap.
>
> Otherwise, targets are not force-scanned even when their effective
> scan number is zero and the other conditions--kswapd/memcg--apply.
>
> Signed-off-by: Johannes Weiner <jweiner@redhat.com>
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

Good catch!


-- 
Kind regards,
Minchan Kim

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] mm: vmscan: drop nr_force_scan[] from get_scan_count

[Minchan Kim]

On Fri, Aug 12, 2011 at 5:31 AM, Johannes Weiner <jweiner@redhat.com> wrote:
> The nr_force_scan[] tuple holds the effective scan numbers for anon
> and file pages in case the situation called for a forced scan and the
> regularly calculated scan numbers turned out zero.
>
> However, the effective scan number can always be assumed to be
> SWAP_CLUSTER_MAX right before the division into anon and file.  The
> numerators and denominator are properly set up for all cases, be it
> force scan for just file, just anon, or both, to do the right thing.
>
> Signed-off-by: Johannes Weiner <jweiner@redhat.com>

Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

There is a nitpick at below.

> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: Ying Han <yinghan@google.com>
> Cc: Balbir Singh <bsingharora@gmail.com>
> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Cc: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> Cc: Mel Gorman <mel@csn.ul.ie>
> ---
>  mm/vmscan.c |   24 ++----------------------
>  1 files changed, 2 insertions(+), 22 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 96061d7..45f0986 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1831,7 +1831,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
>        enum lru_list l;
>        int noswap = 0;
>        bool force_scan = false;
> -       unsigned long nr_force_scan[2];
>
>        /* kswapd does zone balancing and need to scan this zone */
>        if (scanning_global_lru(sc) && current_is_kswapd())
> @@ -1846,8 +1845,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
>                fraction[0] = 0;
>                fraction[1] = 1;
>                denominator = 1;
> -               nr_force_scan[0] = 0;
> -               nr_force_scan[1] = SWAP_CLUSTER_MAX;
>                goto out;
>        }
>
> @@ -1864,8 +1861,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
>                        fraction[0] = 1;
>                        fraction[1] = 0;
>                        denominator = 1;
> -                       nr_force_scan[0] = SWAP_CLUSTER_MAX;
> -                       nr_force_scan[1] = 0;
>                        goto out;
>                }
>        }
> @@ -1914,11 +1909,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
>        fraction[0] = ap;
>        fraction[1] = fp;
>        denominator = ap + fp + 1;
> -       if (force_scan) {
> -               unsigned long scan = SWAP_CLUSTER_MAX;
> -               nr_force_scan[0] = div64_u64(scan * ap, denominator);
> -               nr_force_scan[1] = div64_u64(scan * fp, denominator);
> -       }
>  out:
>        for_each_evictable_lru(l) {
>                int file = is_file_lru(l);
> @@ -1927,20 +1917,10 @@ out:
>                scan = zone_nr_lru_pages(zone, sc, l);
>                if (priority || noswap) {
>                        scan >>= priority;
> +                       if (!scan && force_scan)
> +                               scan = SWAP_CLUSTER_MAX;
>                        scan = div64_u64(scan * fraction[file], denominator);
>                }
> -
> -               /*
> -                * If zone is small or memcg is small, nr[l] can be 0.
> -                * This results no-scan on this priority and priority drop down.
> -                * For global direct reclaim, it can visit next zone and tend
> -                * not to have problems. For global kswapd, it's for zone
> -                * balancing and it need to scan a small amounts. When using
> -                * memcg, priority drop can cause big latency. So, it's better
> -                * to scan small amount. See may_noscan above.
> -                */

Please move this comment with tidy-up at where making force_scan true.
Of course, we can find it by git log[246e87a9393] but as I looked the
git log, it explain this comment indirectly and it's not
understandable to newbies. I think this comment is more understandable
than changelog in git.



-- 
Kind regards,
Minchan Kim

Re: [patch 2/2] mm: vmscan: drop nr_force_scan[] from get_scan_count

[Minchan Kim]

On Fri, Aug 12, 2011 at 5:31 AM, Johannes Weiner <jweiner@redhat.com> wrote:
> The nr_force_scan[] tuple holds the effective scan numbers for anon
> and file pages in case the situation called for a forced scan and the
> regularly calculated scan numbers turned out zero.
>
> However, the effective scan number can always be assumed to be
> SWAP_CLUSTER_MAX right before the division into anon and file.  The
> numerators and denominator are properly set up for all cases, be it
> force scan for just file, just anon, or both, to do the right thing.
>
> Signed-off-by: Johannes Weiner <jweiner@redhat.com>

Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

There is a nitpick at below.

> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: Ying Han <yinghan@google.com>
> Cc: Balbir Singh <bsingharora@gmail.com>
> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Cc: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> Cc: Mel Gorman <mel@csn.ul.ie>
> ---
>  mm/vmscan.c |   24 ++----------------------
>  1 files changed, 2 insertions(+), 22 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 96061d7..45f0986 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1831,7 +1831,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
>        enum lru_list l;
>        int noswap = 0;
>        bool force_scan = false;
> -       unsigned long nr_force_scan[2];
>
>        /* kswapd does zone balancing and need to scan this zone */
>        if (scanning_global_lru(sc) && current_is_kswapd())
> @@ -1846,8 +1845,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
>                fraction[0] = 0;
>                fraction[1] = 1;
>                denominator = 1;
> -               nr_force_scan[0] = 0;
> -               nr_force_scan[1] = SWAP_CLUSTER_MAX;
>                goto out;
>        }
>
> @@ -1864,8 +1861,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
>                        fraction[0] = 1;
>                        fraction[1] = 0;
>                        denominator = 1;
> -                       nr_force_scan[0] = SWAP_CLUSTER_MAX;
> -                       nr_force_scan[1] = 0;
>                        goto out;
>                }
>        }
> @@ -1914,11 +1909,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
>        fraction[0] = ap;
>        fraction[1] = fp;
>        denominator = ap + fp + 1;
> -       if (force_scan) {
> -               unsigned long scan = SWAP_CLUSTER_MAX;
> -               nr_force_scan[0] = div64_u64(scan * ap, denominator);
> -               nr_force_scan[1] = div64_u64(scan * fp, denominator);
> -       }
>  out:
>        for_each_evictable_lru(l) {
>                int file = is_file_lru(l);
> @@ -1927,20 +1917,10 @@ out:
>                scan = zone_nr_lru_pages(zone, sc, l);
>                if (priority || noswap) {
>                        scan >>= priority;
> +                       if (!scan && force_scan)
> +                               scan = SWAP_CLUSTER_MAX;
>                        scan = div64_u64(scan * fraction[file], denominator);
>                }
> -
> -               /*
> -                * If zone is small or memcg is small, nr[l] can be 0.
> -                * This results no-scan on this priority and priority drop down.
> -                * For global direct reclaim, it can visit next zone and tend
> -                * not to have problems. For global kswapd, it's for zone
> -                * balancing and it need to scan a small amounts. When using
> -                * memcg, priority drop can cause big latency. So, it's better
> -                * to scan small amount. See may_noscan above.
> -                */

Please move this comment with tidy-up at where making force_scan true.
Of course, we can find it by git log[246e87a9393] but as I looked the
git log, it explain this comment indirectly and it's not
understandable to newbies. I think this comment is more understandable
than changelog in git.



-- 
Kind regards,
Minchan Kim

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] mm: vmscan: drop nr_force_scan[] from get_scan_count

[Johannes Weiner]

On Fri, Aug 12, 2011 at 08:44:34AM +0900, Minchan Kim wrote:
> On Fri, Aug 12, 2011 at 5:31 AM, Johannes Weiner <jweiner@redhat.com> wrote:
> > The nr_force_scan[] tuple holds the effective scan numbers for anon
> > and file pages in case the situation called for a forced scan and the
> > regularly calculated scan numbers turned out zero.
> >
> > However, the effective scan number can always be assumed to be
> > SWAP_CLUSTER_MAX right before the division into anon and file.  The
> > numerators and denominator are properly set up for all cases, be it
> > force scan for just file, just anon, or both, to do the right thing.
> >
> > Signed-off-by: Johannes Weiner <jweiner@redhat.com>
> 
> Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

Thanks.

> There is a nitpick at below.

> > @@ -1927,20 +1917,10 @@ out:
> >                scan = zone_nr_lru_pages(zone, sc, l);
> >                if (priority || noswap) {
> >                        scan >>= priority;
> > +                       if (!scan && force_scan)
> > +                               scan = SWAP_CLUSTER_MAX;
> >                        scan = div64_u64(scan * fraction[file], denominator);
> >                }
> > -
> > -               /*
> > -                * If zone is small or memcg is small, nr[l] can be 0.
> > -                * This results no-scan on this priority and priority drop down.
> > -                * For global direct reclaim, it can visit next zone and tend
> > -                * not to have problems. For global kswapd, it's for zone
> > -                * balancing and it need to scan a small amounts. When using
> > -                * memcg, priority drop can cause big latency. So, it's better
> > -                * to scan small amount. See may_noscan above.
> > -                */
> 
> Please move this comment with tidy-up at where making force_scan true.
> Of course, we can find it by git log[246e87a9393] but as I looked the
> git log, it explain this comment indirectly and it's not
> understandable to newbies. I think this comment is more understandable
> than changelog in git.

I guess you are right, I am a bit overeager when deleting comments.
How is this?

---
From: Johannes Weiner <jweiner@redhat.com>
Subject: [patch] mm: vmscan: drop nr_force_scan[] from get_scan_count

The nr_force_scan[] tuple holds the effective scan numbers for anon
and file pages in case the situation called for a forced scan and the
regularly calculated scan numbers turned out zero.

However, the effective scan number can always be assumed to be
SWAP_CLUSTER_MAX right before the division into anon and file.  The
numerators and denominator are properly set up for all cases, be it
force scan for just file, just anon, or both, to do the right thing.

Signed-off-by: Johannes Weiner <jweiner@redhat.com>
Cc: Minchan Kim <minchan.kim@gmail.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Ying Han <yinghan@google.com>
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
Cc: Mel Gorman <mel@csn.ul.ie>
---
 mm/vmscan.c |   36 ++++++++++++------------------------
 1 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 96061d7..a6ca076 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1831,12 +1831,19 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 	enum lru_list l;
 	int noswap = 0;
 	bool force_scan = false;
-	unsigned long nr_force_scan[2];
 
-	/* kswapd does zone balancing and need to scan this zone */
+	/*
+	 * If the zone or memcg is small, nr[l] can be 0.  This
+	 * results in no scanning on this priority and a potential
+	 * priority drop.  Global direct reclaim can go to the next
+	 * zone and tends to have no problems. Global kswapd is for
+	 * zone balancing and it needs to scan a minimum amount. When
+	 * reclaiming for a memcg, a priority drop can cause high
+	 * latencies, so it's better to scan a minimum amount there as
+	 * well.
+	 */
 	if (scanning_global_lru(sc) && current_is_kswapd())
 		force_scan = true;
-	/* memcg may have small limit and need to avoid priority drop */
 	if (!scanning_global_lru(sc))
 		force_scan = true;
 
@@ -1846,8 +1853,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 		fraction[0] = 0;
 		fraction[1] = 1;
 		denominator = 1;
-		nr_force_scan[0] = 0;
-		nr_force_scan[1] = SWAP_CLUSTER_MAX;
 		goto out;
 	}
 
@@ -1864,8 +1869,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 			fraction[0] = 1;
 			fraction[1] = 0;
 			denominator = 1;
-			nr_force_scan[0] = SWAP_CLUSTER_MAX;
-			nr_force_scan[1] = 0;
 			goto out;
 		}
 	}
@@ -1914,11 +1917,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 	fraction[0] = ap;
 	fraction[1] = fp;
 	denominator = ap + fp + 1;
-	if (force_scan) {
-		unsigned long scan = SWAP_CLUSTER_MAX;
-		nr_force_scan[0] = div64_u64(scan * ap, denominator);
-		nr_force_scan[1] = div64_u64(scan * fp, denominator);
-	}
 out:
 	for_each_evictable_lru(l) {
 		int file = is_file_lru(l);
@@ -1927,20 +1925,10 @@ out:
 		scan = zone_nr_lru_pages(zone, sc, l);
 		if (priority || noswap) {
 			scan >>= priority;
+			if (!scan && force_scan)
+				scan = SWAP_CLUSTER_MAX;
 			scan = div64_u64(scan * fraction[file], denominator);
 		}
-
-		/*
-		 * If zone is small or memcg is small, nr[l] can be 0.
-		 * This results no-scan on this priority and priority drop down.
-		 * For global direct reclaim, it can visit next zone and tend
-		 * not to have problems. For global kswapd, it's for zone
-		 * balancing and it need to scan a small amounts. When using
-		 * memcg, priority drop can cause big latency. So, it's better
-		 * to scan small amount. See may_noscan above.
-		 */
-		if (!scan && force_scan)
-			scan = nr_force_scan[file];
 		nr[l] = scan;
 	}
 }
-- 
1.7.6

Re: [patch 2/2] mm: vmscan: drop nr_force_scan[] from get_scan_count

[Johannes Weiner]

On Fri, Aug 12, 2011 at 08:44:34AM +0900, Minchan Kim wrote:
> On Fri, Aug 12, 2011 at 5:31 AM, Johannes Weiner <jweiner@redhat.com> wrote:
> > The nr_force_scan[] tuple holds the effective scan numbers for anon
> > and file pages in case the situation called for a forced scan and the
> > regularly calculated scan numbers turned out zero.
> >
> > However, the effective scan number can always be assumed to be
> > SWAP_CLUSTER_MAX right before the division into anon and file.  The
> > numerators and denominator are properly set up for all cases, be it
> > force scan for just file, just anon, or both, to do the right thing.
> >
> > Signed-off-by: Johannes Weiner <jweiner@redhat.com>
> 
> Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

Thanks.

> There is a nitpick at below.

> > @@ -1927,20 +1917,10 @@ out:
> >                scan = zone_nr_lru_pages(zone, sc, l);
> >                if (priority || noswap) {
> >                        scan >>= priority;
> > +                       if (!scan && force_scan)
> > +                               scan = SWAP_CLUSTER_MAX;
> >                        scan = div64_u64(scan * fraction[file], denominator);
> >                }
> > -
> > -               /*
> > -                * If zone is small or memcg is small, nr[l] can be 0.
> > -                * This results no-scan on this priority and priority drop down.
> > -                * For global direct reclaim, it can visit next zone and tend
> > -                * not to have problems. For global kswapd, it's for zone
> > -                * balancing and it need to scan a small amounts. When using
> > -                * memcg, priority drop can cause big latency. So, it's better
> > -                * to scan small amount. See may_noscan above.
> > -                */
> 
> Please move this comment with tidy-up at where making force_scan true.
> Of course, we can find it by git log[246e87a9393] but as I looked the
> git log, it explain this comment indirectly and it's not
> understandable to newbies. I think this comment is more understandable
> than changelog in git.

I guess you are right, I am a bit overeager when deleting comments.
How is this?

---
From: Johannes Weiner <jweiner@redhat.com>
Subject: [patch] mm: vmscan: drop nr_force_scan[] from get_scan_count

The nr_force_scan[] tuple holds the effective scan numbers for anon
and file pages in case the situation called for a forced scan and the
regularly calculated scan numbers turned out zero.

However, the effective scan number can always be assumed to be
SWAP_CLUSTER_MAX right before the division into anon and file.  The
numerators and denominator are properly set up for all cases, be it
force scan for just file, just anon, or both, to do the right thing.

Signed-off-by: Johannes Weiner <jweiner@redhat.com>
Cc: Minchan Kim <minchan.kim@gmail.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Ying Han <yinghan@google.com>
Cc: Balbir Singh <bsingharora@gmail.com>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
Cc: Mel Gorman <mel@csn.ul.ie>
---
 mm/vmscan.c |   36 ++++++++++++------------------------
 1 files changed, 12 insertions(+), 24 deletions(-)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 96061d7..a6ca076 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1831,12 +1831,19 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 	enum lru_list l;
 	int noswap = 0;
 	bool force_scan = false;
-	unsigned long nr_force_scan[2];
 
-	/* kswapd does zone balancing and need to scan this zone */
+	/*
+	 * If the zone or memcg is small, nr[l] can be 0.  This
+	 * results in no scanning on this priority and a potential
+	 * priority drop.  Global direct reclaim can go to the next
+	 * zone and tends to have no problems. Global kswapd is for
+	 * zone balancing and it needs to scan a minimum amount. When
+	 * reclaiming for a memcg, a priority drop can cause high
+	 * latencies, so it's better to scan a minimum amount there as
+	 * well.
+	 */
 	if (scanning_global_lru(sc) && current_is_kswapd())
 		force_scan = true;
-	/* memcg may have small limit and need to avoid priority drop */
 	if (!scanning_global_lru(sc))
 		force_scan = true;
 
@@ -1846,8 +1853,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 		fraction[0] = 0;
 		fraction[1] = 1;
 		denominator = 1;
-		nr_force_scan[0] = 0;
-		nr_force_scan[1] = SWAP_CLUSTER_MAX;
 		goto out;
 	}
 
@@ -1864,8 +1869,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 			fraction[0] = 1;
 			fraction[1] = 0;
 			denominator = 1;
-			nr_force_scan[0] = SWAP_CLUSTER_MAX;
-			nr_force_scan[1] = 0;
 			goto out;
 		}
 	}
@@ -1914,11 +1917,6 @@ static void get_scan_count(struct zone *zone, struct scan_control *sc,
 	fraction[0] = ap;
 	fraction[1] = fp;
 	denominator = ap + fp + 1;
-	if (force_scan) {
-		unsigned long scan = SWAP_CLUSTER_MAX;
-		nr_force_scan[0] = div64_u64(scan * ap, denominator);
-		nr_force_scan[1] = div64_u64(scan * fp, denominator);
-	}
 out:
 	for_each_evictable_lru(l) {
 		int file = is_file_lru(l);
@@ -1927,20 +1925,10 @@ out:
 		scan = zone_nr_lru_pages(zone, sc, l);
 		if (priority || noswap) {
 			scan >>= priority;
+			if (!scan && force_scan)
+				scan = SWAP_CLUSTER_MAX;
 			scan = div64_u64(scan * fraction[file], denominator);
 		}
-
-		/*
-		 * If zone is small or memcg is small, nr[l] can be 0.
-		 * This results no-scan on this priority and priority drop down.
-		 * For global direct reclaim, it can visit next zone and tend
-		 * not to have problems. For global kswapd, it's for zone
-		 * balancing and it need to scan a small amounts. When using
-		 * memcg, priority drop can cause big latency. So, it's better
-		 * to scan small amount. See may_noscan above.
-		 */
-		if (!scan && force_scan)
-			scan = nr_force_scan[file];
 		nr[l] = scan;
 	}
 }
-- 
1.7.6

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] mm: vmscan: drop nr_force_scan[] from get_scan_count

[Minchan Kim]

On Fri, Aug 12, 2011 at 3:58 PM, Johannes Weiner <jweiner@redhat.com> wrote:
> On Fri, Aug 12, 2011 at 08:44:34AM +0900, Minchan Kim wrote:
>> On Fri, Aug 12, 2011 at 5:31 AM, Johannes Weiner <jweiner@redhat.com> wrote:
>> > The nr_force_scan[] tuple holds the effective scan numbers for anon
>> > and file pages in case the situation called for a forced scan and the
>> > regularly calculated scan numbers turned out zero.
>> >
>> > However, the effective scan number can always be assumed to be
>> > SWAP_CLUSTER_MAX right before the division into anon and file.  The
>> > numerators and denominator are properly set up for all cases, be it
>> > force scan for just file, just anon, or both, to do the right thing.
>> >
>> > Signed-off-by: Johannes Weiner <jweiner@redhat.com>
>>
>> Reviewed-by: Minchan Kim <minchan.kim@gmail.com>
>
> Thanks.
>
>> There is a nitpick at below.
>
>> > @@ -1927,20 +1917,10 @@ out:
>> >                scan = zone_nr_lru_pages(zone, sc, l);
>> >                if (priority || noswap) {
>> >                        scan >>= priority;
>> > +                       if (!scan && force_scan)
>> > +                               scan = SWAP_CLUSTER_MAX;
>> >                        scan = div64_u64(scan * fraction[file], denominator);
>> >                }
>> > -
>> > -               /*
>> > -                * If zone is small or memcg is small, nr[l] can be 0.
>> > -                * This results no-scan on this priority and priority drop down.
>> > -                * For global direct reclaim, it can visit next zone and tend
>> > -                * not to have problems. For global kswapd, it's for zone
>> > -                * balancing and it need to scan a small amounts. When using
>> > -                * memcg, priority drop can cause big latency. So, it's better
>> > -                * to scan small amount. See may_noscan above.
>> > -                */
>>
>> Please move this comment with tidy-up at where making force_scan true.
>> Of course, we can find it by git log[246e87a9393] but as I looked the
>> git log, it explain this comment indirectly and it's not
>> understandable to newbies. I think this comment is more understandable
>> than changelog in git.
>
> I guess you are right, I am a bit overeager when deleting comments.
> How is this?
>
> ---
> From: Johannes Weiner <jweiner@redhat.com>
> Subject: [patch] mm: vmscan: drop nr_force_scan[] from get_scan_count
>
> The nr_force_scan[] tuple holds the effective scan numbers for anon
> and file pages in case the situation called for a forced scan and the
> regularly calculated scan numbers turned out zero.
>
> However, the effective scan number can always be assumed to be
> SWAP_CLUSTER_MAX right before the division into anon and file.  The
> numerators and denominator are properly set up for all cases, be it
> force scan for just file, just anon, or both, to do the right thing.
>
> Signed-off-by: Johannes Weiner <jweiner@redhat.com>
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

Thanks, Hannes.
-- 
Kind regards,
Minchan Kim

Re: [patch 2/2] mm: vmscan: drop nr_force_scan[] from get_scan_count

[Minchan Kim]

On Fri, Aug 12, 2011 at 3:58 PM, Johannes Weiner <jweiner@redhat.com> wrote:
> On Fri, Aug 12, 2011 at 08:44:34AM +0900, Minchan Kim wrote:
>> On Fri, Aug 12, 2011 at 5:31 AM, Johannes Weiner <jweiner@redhat.com> wrote:
>> > The nr_force_scan[] tuple holds the effective scan numbers for anon
>> > and file pages in case the situation called for a forced scan and the
>> > regularly calculated scan numbers turned out zero.
>> >
>> > However, the effective scan number can always be assumed to be
>> > SWAP_CLUSTER_MAX right before the division into anon and file.  The
>> > numerators and denominator are properly set up for all cases, be it
>> > force scan for just file, just anon, or both, to do the right thing.
>> >
>> > Signed-off-by: Johannes Weiner <jweiner@redhat.com>
>>
>> Reviewed-by: Minchan Kim <minchan.kim@gmail.com>
>
> Thanks.
>
>> There is a nitpick at below.
>
>> > @@ -1927,20 +1917,10 @@ out:
>> >                scan = zone_nr_lru_pages(zone, sc, l);
>> >                if (priority || noswap) {
>> >                        scan >>= priority;
>> > +                       if (!scan && force_scan)
>> > +                               scan = SWAP_CLUSTER_MAX;
>> >                        scan = div64_u64(scan * fraction[file], denominator);
>> >                }
>> > -
>> > -               /*
>> > -                * If zone is small or memcg is small, nr[l] can be 0.
>> > -                * This results no-scan on this priority and priority drop down.
>> > -                * For global direct reclaim, it can visit next zone and tend
>> > -                * not to have problems. For global kswapd, it's for zone
>> > -                * balancing and it need to scan a small amounts. When using
>> > -                * memcg, priority drop can cause big latency. So, it's better
>> > -                * to scan small amount. See may_noscan above.
>> > -                */
>>
>> Please move this comment with tidy-up at where making force_scan true.
>> Of course, we can find it by git log[246e87a9393] but as I looked the
>> git log, it explain this comment indirectly and it's not
>> understandable to newbies. I think this comment is more understandable
>> than changelog in git.
>
> I guess you are right, I am a bit overeager when deleting comments.
> How is this?
>
> ---
> From: Johannes Weiner <jweiner@redhat.com>
> Subject: [patch] mm: vmscan: drop nr_force_scan[] from get_scan_count
>
> The nr_force_scan[] tuple holds the effective scan numbers for anon
> and file pages in case the situation called for a forced scan and the
> regularly calculated scan numbers turned out zero.
>
> However, the effective scan number can always be assumed to be
> SWAP_CLUSTER_MAX right before the division into anon and file.  The
> numerators and denominator are properly set up for all cases, be it
> force scan for just file, just anon, or both, to do the right thing.
>
> Signed-off-by: Johannes Weiner <jweiner@redhat.com>
Reviewed-by: Minchan Kim <minchan.kim@gmail.com>

Thanks, Hannes.
-- 
Kind regards,
Minchan Kim

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/2] mm: vmscan: fix force-scanning small targets without swap

[Michal Hocko]

On Thu 11-08-11 22:31:54, Johannes Weiner wrote:
> Without swap, anonymous pages are not scanned.  As such, they should
> not count when considering force-scanning a small target if there is
> no swap.
> 
> Otherwise, targets are not force-scanned even when their effective
> scan number is zero and the other conditions--kswapd/memcg--apply.
> 
> Signed-off-by: Johannes Weiner <jweiner@redhat.com>

We are calculating nr_force_scan even if it is not needed but this
shouldn't hurt.

Reviewed-by: Michal Hocko <mhocko@suse.cz>

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

Re: [patch 1/2] mm: vmscan: fix force-scanning small targets without swap

[Michal Hocko]

On Thu 11-08-11 22:31:54, Johannes Weiner wrote:
> Without swap, anonymous pages are not scanned.  As such, they should
> not count when considering force-scanning a small target if there is
> no swap.
> 
> Otherwise, targets are not force-scanned even when their effective
> scan number is zero and the other conditions--kswapd/memcg--apply.
> 
> Signed-off-by: Johannes Weiner <jweiner@redhat.com>

We are calculating nr_force_scan even if it is not needed but this
shouldn't hurt.

Reviewed-by: Michal Hocko <mhocko@suse.cz>

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/2] mm: vmscan: fix force-scanning small targets without swap

[Michal Hocko]

On Fri 12-08-11 15:10:42, Michal Hocko wrote:
> On Thu 11-08-11 22:31:54, Johannes Weiner wrote:
> > Without swap, anonymous pages are not scanned.  As such, they should
> > not count when considering force-scanning a small target if there is
> > no swap.
> > 
> > Otherwise, targets are not force-scanned even when their effective
> > scan number is zero and the other conditions--kswapd/memcg--apply.
> > 
> > Signed-off-by: Johannes Weiner <jweiner@redhat.com>
> 
> We are calculating nr_force_scan even if it is not needed but this
> shouldn't hurt.

Ahh and the second patch removes that calculation. Sorry for noise.

> Reviewed-by: Michal Hocko <mhocko@suse.cz>

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

Re: [patch 1/2] mm: vmscan: fix force-scanning small targets without swap

[Michal Hocko]

On Fri 12-08-11 15:10:42, Michal Hocko wrote:
> On Thu 11-08-11 22:31:54, Johannes Weiner wrote:
> > Without swap, anonymous pages are not scanned.  As such, they should
> > not count when considering force-scanning a small target if there is
> > no swap.
> > 
> > Otherwise, targets are not force-scanned even when their effective
> > scan number is zero and the other conditions--kswapd/memcg--apply.
> > 
> > Signed-off-by: Johannes Weiner <jweiner@redhat.com>
> 
> We are calculating nr_force_scan even if it is not needed but this
> shouldn't hurt.

Ahh and the second patch removes that calculation. Sorry for noise.

> Reviewed-by: Michal Hocko <mhocko@suse.cz>

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] mm: vmscan: drop nr_force_scan[] from get_scan_count

[Michal Hocko]

On Fri 12-08-11 08:58:58, Johannes Weiner wrote:
[...]
> From: Johannes Weiner <jweiner@redhat.com>
> Subject: [patch] mm: vmscan: drop nr_force_scan[] from get_scan_count
> 
> The nr_force_scan[] tuple holds the effective scan numbers for anon
> and file pages in case the situation called for a forced scan and the
> regularly calculated scan numbers turned out zero.
> 
> However, the effective scan number can always be assumed to be
> SWAP_CLUSTER_MAX right before the division into anon and file.  The
> numerators and denominator are properly set up for all cases, be it
> force scan for just file, just anon, or both, to do the right thing.

> 
> Signed-off-by: Johannes Weiner <jweiner@redhat.com>

Nice. The code is much easier to read.
Reviewed-by: Michal Hocko <mhocko@suse.cz>

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

Re: [patch 2/2] mm: vmscan: drop nr_force_scan[] from get_scan_count

[Michal Hocko]

On Fri 12-08-11 08:58:58, Johannes Weiner wrote:
[...]
> From: Johannes Weiner <jweiner@redhat.com>
> Subject: [patch] mm: vmscan: drop nr_force_scan[] from get_scan_count
> 
> The nr_force_scan[] tuple holds the effective scan numbers for anon
> and file pages in case the situation called for a forced scan and the
> regularly calculated scan numbers turned out zero.
> 
> However, the effective scan number can always be assumed to be
> SWAP_CLUSTER_MAX right before the division into anon and file.  The
> numerators and denominator are properly set up for all cases, be it
> force scan for just file, just anon, or both, to do the right thing.

> 
> Signed-off-by: Johannes Weiner <jweiner@redhat.com>

Nice. The code is much easier to read.
Reviewed-by: Michal Hocko <mhocko@suse.cz>

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/2] mm: vmscan: fix force-scanning small targets without swap

[KAMEZAWA Hiroyuki]

On Thu, 11 Aug 2011 22:31:54 +0200
Johannes Weiner <jweiner@redhat.com> wrote:

> Without swap, anonymous pages are not scanned.  As such, they should
> not count when considering force-scanning a small target if there is
> no swap.
> 
> Otherwise, targets are not force-scanned even when their effective
> scan number is zero and the other conditions--kswapd/memcg--apply.
> 
> Signed-off-by: Johannes Weiner <jweiner@redhat.com>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: Ying Han <yinghan@google.com>
> Cc: Balbir Singh <bsingharora@gmail.com>
> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Cc: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> Cc: Mel Gorman <mel@csn.ul.ie>

Thanks,
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Re: [patch 1/2] mm: vmscan: fix force-scanning small targets without swap

[KAMEZAWA Hiroyuki]

On Thu, 11 Aug 2011 22:31:54 +0200
Johannes Weiner <jweiner@redhat.com> wrote:

> Without swap, anonymous pages are not scanned.  As such, they should
> not count when considering force-scanning a small target if there is
> no swap.
> 
> Otherwise, targets are not force-scanned even when their effective
> scan number is zero and the other conditions--kswapd/memcg--apply.
> 
> Signed-off-by: Johannes Weiner <jweiner@redhat.com>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: Ying Han <yinghan@google.com>
> Cc: Balbir Singh <bsingharora@gmail.com>
> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Cc: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> Cc: Mel Gorman <mel@csn.ul.ie>

Thanks,
Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] mm: vmscan: drop nr_force_scan[] from get_scan_count

[KAMEZAWA Hiroyuki]

On Thu, 11 Aug 2011 22:31:55 +0200
Johannes Weiner <jweiner@redhat.com> wrote:

> The nr_force_scan[] tuple holds the effective scan numbers for anon
> and file pages in case the situation called for a forced scan and the
> regularly calculated scan numbers turned out zero.
> 
> However, the effective scan number can always be assumed to be
> SWAP_CLUSTER_MAX right before the division into anon and file.  The
> numerators and denominator are properly set up for all cases, be it
> force scan for just file, just anon, or both, to do the right thing.
> 
> Signed-off-by: Johannes Weiner <jweiner@redhat.com>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: Ying Han <yinghan@google.com>
> Cc: Balbir Singh <bsingharora@gmail.com>
> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Cc: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> Cc: Mel Gorman <mel@csn.ul.ie>

Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Re: [patch 2/2] mm: vmscan: drop nr_force_scan[] from get_scan_count

[KAMEZAWA Hiroyuki]

On Thu, 11 Aug 2011 22:31:55 +0200
Johannes Weiner <jweiner@redhat.com> wrote:

> The nr_force_scan[] tuple holds the effective scan numbers for anon
> and file pages in case the situation called for a forced scan and the
> regularly calculated scan numbers turned out zero.
> 
> However, the effective scan number can always be assumed to be
> SWAP_CLUSTER_MAX right before the division into anon and file.  The
> numerators and denominator are properly set up for all cases, be it
> force scan for just file, just anon, or both, to do the right thing.
> 
> Signed-off-by: Johannes Weiner <jweiner@redhat.com>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: Ying Han <yinghan@google.com>
> Cc: Balbir Singh <bsingharora@gmail.com>
> Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
> Cc: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> Cc: Mel Gorman <mel@csn.ul.ie>

Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/2] mm: vmscan: fix force-scanning small targets without swap

[Mel Gorman]

On Thu, Aug 11, 2011 at 10:31:54PM +0200, Johannes Weiner wrote:
> Without swap, anonymous pages are not scanned.  As such, they should
> not count when considering force-scanning a small target if there is
> no swap.
> 
> Otherwise, targets are not force-scanned even when their effective
> scan number is zero and the other conditions--kswapd/memcg--apply.
> 
> Signed-off-by: Johannes Weiner <jweiner@redhat.com>

Acked-by: Mel Gorman <mel@csn.ul.ie>

-- 
Mel Gorman
SUSE Labs

Re: [patch 1/2] mm: vmscan: fix force-scanning small targets without swap

[Mel Gorman]

On Thu, Aug 11, 2011 at 10:31:54PM +0200, Johannes Weiner wrote:
> Without swap, anonymous pages are not scanned.  As such, they should
> not count when considering force-scanning a small target if there is
> no swap.
> 
> Otherwise, targets are not force-scanned even when their effective
> scan number is zero and the other conditions--kswapd/memcg--apply.
> 
> Signed-off-by: Johannes Weiner <jweiner@redhat.com>

Acked-by: Mel Gorman <mel@csn.ul.ie>

-- 
Mel Gorman
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 2/2] mm: vmscan: drop nr_force_scan[] from get_scan_count

[Mel Gorman]

On Fri, Aug 12, 2011 at 08:58:58AM +0200, Johannes Weiner wrote:
> From: Johannes Weiner <jweiner@redhat.com>
> Subject: [patch] mm: vmscan: drop nr_force_scan[] from get_scan_count
> 
> The nr_force_scan[] tuple holds the effective scan numbers for anon
> and file pages in case the situation called for a forced scan and the
> regularly calculated scan numbers turned out zero.
> 
> However, the effective scan number can always be assumed to be
> SWAP_CLUSTER_MAX right before the division into anon and file.  The
> numerators and denominator are properly set up for all cases, be it
> force scan for just file, just anon, or both, to do the right thing.
> 
> Signed-off-by: Johannes Weiner <jweiner@redhat.com>

Acked-by: Mel Gorman <mel@csn.ul.ie>

-- 
Mel Gorman
SUSE Labs

Re: [patch 2/2] mm: vmscan: drop nr_force_scan[] from get_scan_count

[Mel Gorman]

On Fri, Aug 12, 2011 at 08:58:58AM +0200, Johannes Weiner wrote:
> From: Johannes Weiner <jweiner@redhat.com>
> Subject: [patch] mm: vmscan: drop nr_force_scan[] from get_scan_count
> 
> The nr_force_scan[] tuple holds the effective scan numbers for anon
> and file pages in case the situation called for a forced scan and the
> regularly calculated scan numbers turned out zero.
> 
> However, the effective scan number can always be assumed to be
> SWAP_CLUSTER_MAX right before the division into anon and file.  The
> numerators and denominator are properly set up for all cases, be it
> force scan for just file, just anon, or both, to do the right thing.
> 
> Signed-off-by: Johannes Weiner <jweiner@redhat.com>

Acked-by: Mel Gorman <mel@csn.ul.ie>

-- 
Mel Gorman
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/2] mm: vmscan: fix force-scanning small targets without swap

[Johannes Weiner]

Andrew,

On Thu, Aug 11, 2011 at 10:31:54PM +0200, Johannes Weiner wrote:
> Without swap, anonymous pages are not scanned.  As such, they should
> not count when considering force-scanning a small target if there is
> no swap.
> 
> Otherwise, targets are not force-scanned even when their effective
> scan number is zero and the other conditions--kswapd/memcg--apply.

I forgot to mention, this patch is a fix for '246e87a memcg: fix
get_scan_count() for small targets', which went upstream this merge
window.

Probably makes sense to merge this one too before the release..?

Sorry.

Re: [patch 1/2] mm: vmscan: fix force-scanning small targets without swap

[Johannes Weiner]

Andrew,

On Thu, Aug 11, 2011 at 10:31:54PM +0200, Johannes Weiner wrote:
> Without swap, anonymous pages are not scanned.  As such, they should
> not count when considering force-scanning a small target if there is
> no swap.
> 
> Otherwise, targets are not force-scanned even when their effective
> scan number is zero and the other conditions--kswapd/memcg--apply.

I forgot to mention, this patch is a fix for '246e87a memcg: fix
get_scan_count() for small targets', which went upstream this merge
window.

Probably makes sense to merge this one too before the release..?

Sorry.

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch 1/2] mm: vmscan: fix force-scanning small targets without swap

[Andrew Morton]

On Mon, 29 Aug 2011 18:08:39 +0200
Johannes Weiner <jweiner@redhat.com> wrote:

> Andrew,
> 
> On Thu, Aug 11, 2011 at 10:31:54PM +0200, Johannes Weiner wrote:
> > Without swap, anonymous pages are not scanned.  As such, they should
> > not count when considering force-scanning a small target if there is
> > no swap.
> > 
> > Otherwise, targets are not force-scanned even when their effective
> > scan number is zero and the other conditions--kswapd/memcg--apply.
> 
> I forgot to mention, this patch is a fix for '246e87a memcg: fix
> get_scan_count() for small targets', which went upstream this merge
> window.
> 
> Probably makes sense to merge this one too before the release..?
> 

Ah, I didn't realise that.  Thanks.

Re: [patch 1/2] mm: vmscan: fix force-scanning small targets without swap

[Andrew Morton]

On Mon, 29 Aug 2011 18:08:39 +0200
Johannes Weiner <jweiner@redhat.com> wrote:

> Andrew,
> 
> On Thu, Aug 11, 2011 at 10:31:54PM +0200, Johannes Weiner wrote:
> > Without swap, anonymous pages are not scanned.  As such, they should
> > not count when considering force-scanning a small target if there is
> > no swap.
> > 
> > Otherwise, targets are not force-scanned even when their effective
> > scan number is zero and the other conditions--kswapd/memcg--apply.
> 
> I forgot to mention, this patch is a fix for '246e87a memcg: fix
> get_scan_count() for small targets', which went upstream this merge
> window.
> 
> Probably makes sense to merge this one too before the release..?
> 

Ah, I didn't realise that.  Thanks.

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>