mm: do batched scans for mem_cgroup

shrink_inactive_list() won't be called to scan too much pages (unless in hibernation code which is fine) or too few pages (ie. batching is taken care of by the callers).  So we can just remove the big loop and isolate the exact number of pages requested.

Just a RFC, and a scratch patch to show the basic idea. Please kindly NAK quick if you don't like it ;)

Signed-off-by Wu Fengguang <[email protected]>

---
 mm/vmscan.c |   32 ++++++++++++++++----------------

 1 file changed, 16 insertions(+), 16 deletions(-)

mm/vmscan.c   2009-08-20 10:24:34.000000000 +0800 renamed from mm/vmscan.c      2009-08-20 10:16:18.000000000 +0800 +15 -14

@@ -1032,16 +1032,22 @@ int isolate_lru_page(struct page *page)
  * shrink_inactive_list() is a helper for shrink_zone().  It returns the number
  * of reclaimed pages
  */
-static unsigned long shrink_inactive_list(unsigned long max_scan,
+static unsigned long shrink_inactive_list(unsigned long nr_to_scan,
                        struct zone *zone, struct scan_control *sc,
                        int priority, int file)
  {
        LIST_HEAD(page_list);
        struct pagevec pvec;
-       unsigned long nr_scanned = 0;
-       unsigned long nr_reclaimed = 0;
+       unsigned long nr_reclaimed;
        struct zone_reclaim_stat *reclaim_stat = get_reclaim_stat(zone, sc);
-       int lumpy_reclaim = 0;
+       int lumpy_reclaim;
+       struct page *page;
+       unsigned long nr_taken;
+       unsigned long nr_scan;
+       unsigned long nr_freed;
+       unsigned long nr_active;
+       unsigned int count[NR_LRU_LISTS] = { 0, };
+       int mode;
 
        /*
         * If we need a large contiguous chunk of memory, or have

@@ -1054,21 +1060,17 @@ static unsigned long shrink_inactive_lis
                lumpy_reclaim = 1;
        else if (sc->order && priority < DEF_PRIORITY - 2)
                lumpy_reclaim = 1;
+       else
+               lumpy_reclaim = 0;
+
+       mode = lumpy_reclaim ? ISOLATE_BOTH : ISOLATE_INACTIVE;
 
        pagevec_init(&pvec, 1);
 
        lru_add_drain();
        spin_lock_irq(&zone->lru_lock);
-       do {
-               struct page *page;
-               unsigned long nr_taken;
-               unsigned long nr_scan;
-               unsigned long nr_freed;
-               unsigned long nr_active;
-               unsigned int count[NR_LRU_LISTS] = { 0, };
-               int mode = lumpy_reclaim ? ISOLATE_BOTH : ISOLATE_INACTIVE;
 
-               nr_taken = sc->isolate_pages(sc->swap_cluster_max,
+               nr_taken = sc->isolate_pages(nr_to_scan,
                             &page_list, &nr_scan, sc->order, mode,
                                zone, sc->mem_cgroup, 0, file);
                nr_active = clear_active_flags(&page_list, count);

@@ -1093,7 +1095,6 @@ static unsigned long shrink_inactive_lis
 
                spin_unlock_irq(&zone->lru_lock);
 
-               nr_scanned += nr_scan;
                nr_freed = shrink_page_list(&page_list, sc, PAGEOUT_IO_ASYNC);
 
                /*

mm/vmscan.c   2009-08-20 10:24:34.000000000 +0800 renamed from mm/vmscan.c      2009-08-20 10:16:18.000000000 +0800 +0 -1

                local_irq_disable();
                if (current_is_kswapd()) {
                        __count_zone_vm_events(PGSCAN_KSWAPD, zone, nr_scan);

@@ -1158,7 +1159,6 @@ static unsigned long shrink_inactive_lis
                                spin_lock_irq(&zone->lru_lock);
                        }
                }
-       } while (nr_scanned < max_scan);
        spin_unlock(&zone->lru_lock);

 done:
       local_irq_enable();

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

2009/8/20 Wu Fengguang <[email protected]>:

Quoted 7 lines

shrink_inactive_list() won't be called to scan too much pages (unless in hibernation code which is fine) or too few pages (ie. batching is taken care of by the callers).  So we can just remove the big loop and isolate the exact number of pages requested.

Just a RFC, and a scratch patch to show the basic idea. Please kindly NAK quick if you don't like it ;)

Hm, I think this patch taks only cleanups. right?
if so, I don't find any objection reason.

Quoted 4 lines

-               nr_taken = sc->isolate_pages(sc->swap_cluster_max,
+               nr_taken = sc->isolate_pages(nr_to_scan, &page_list, &nr_scan, sc->order, mode, zone, sc->mem_cgroup, 0, file);

Quoted 13 lines

               nr_active = clear_active_flags(&page_list, count);

mm/vmscan.c +0 -1

@@ -1093,7 +1095,6 @@ static unsigned long shrink_inactive_lis
 
                spin_unlock_irq(&zone->lru_lock);
 
-               nr_scanned += nr_scan;
                nr_freed = shrink_page_list(&page_list, sc, PAGEOUT_IO_ASYNC);
 
                /*

@@ -1117,7 +1118,7 @@ static unsigned long shrink_inactive_lis
                                                        PAGEOUT_IO_SYNC);
                }
 

mm/vmscan.c static unsigned long shrink_inactive_lis · line 1121

Quoted 2 lines

mm/vmscan.c +1 -1

-               nr_reclaimed += nr_freed;
+               nr_reclaimed = nr_freed;

maybe, nr_freed can be removed perfectly. it have the same meaning as nr_reclaimed.

Quoted 11 lines

2009/8/20 Wu Fengguang <[email protected]>:

Quoted 7 lines

shrink_inactive_list() won't be called to scan too much pages (unless in hibernation code which is fine) or too few pages (ie. batching is taken care of by the callers). A So we can just remove the big loop and isolate the exact number of pages requested.

Just a RFC, and a scratch patch to show the basic idea. Please kindly NAK quick if you don't like it ;)

Hm, I think this patch taks only cleanups. right?
if so, I don't find any objection reason.

Mostly cleanups, but one behavior change here:

Quoted 4 lines

Quoted 4 lines

- A  A  A  A  A  A  A  nr_taken = sc->isolate_pages(sc->swap_cluster_max,
+ A  A  A  A  A  A  A  nr_taken = sc->isolate_pages(nr_to_scan,
A  A  A  A  A  A  A  A  A  A  A  A  A  A  &page_list, &nr_scan, sc->order, mode,
A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A zone, sc->mem_cgroup, 0, file);

The new behavior is to scan exactly the number of pages that shrink_zone() or other callers tell it. It won't try to "round it up" to 32 pages. This new behavior is in line with shrink_active_list()'s current status as well as shrink_zone()'s expectation.

shrink_zone() may still submit scan requests for <32 pages, which is suboptimal. I'll try to eliminate that totally with more patches.

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX. It effectively scales up the inactive list scan rate by up to 32 times.

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

CC Rik van Riel <[email protected]>

CC Minchan Kim <[email protected]>

CC Balbir Singh <[email protected]>

CC KOSAKI Motohiro <[email protected]>

CC KAMEZAWA Hiroyuki <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---

Quoted 21 lines · On Thu, 20 Aug 2009 10:49:29 +0800 Wu Fengguang <[email protected]> wrote:

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX. It effectively scales up the inactive list scan rate by up to 32 times.

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

Quoted 7 lines

CC Rik van Riel <[email protected]>

CC Minchan Kim <[email protected]>

CC Balbir Singh <[email protected]>

CC KOSAKI Motohiro <[email protected]>

CC KAMEZAWA Hiroyuki <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---

Hmm, how about this ?
==
Now, nr_saved_scan is tied to zone's LRU.
But, considering how vmscan works, it should be tied to reclaim_stat.

By this, memcg can make use of nr_saved_scan information seamlessly.

Quoted 32 lines · KAMEZAWA Hiroyuki

Quoted 28 lines · On Thu, 20 Aug 2009 10:49:29 +0800 Wu Fengguang <[email protected]> wrote:

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX. It effectively scales up the inactive list scan rate by up to 32 times.

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

CC Rik van Riel <[email protected]>

CC Minchan Kim <[email protected]>

CC Balbir Singh <[email protected]>

CC KOSAKI Motohiro <[email protected]>

CC KAMEZAWA Hiroyuki <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---

Quoted 6 lines

Hmm, how about this ?
==
Now, nr_saved_scan is tied to zone's LRU.
But, considering how vmscan works, it should be tied to reclaim_stat.

By this, memcg can make use of nr_saved_scan information seamlessly.

Good idea, full patch updated with your signed-off-by :)

Thanks,
Fengguang
---
mm: do batched scans for mem_cgroup

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX. It effectively scales up the inactive list scan rate by up to 32 times.

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

CC Rik van Riel <[email protected]>

CC Minchan Kim <[email protected]>

CC Balbir Singh <[email protected]>

CC KOSAKI Motohiro <[email protected]>

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---
 include/linux/mmzone.h |    6 +++++-
 mm/page_alloc.c        |    2 +-
 mm/vmscan.c            |   20 +++++++++++---------
 3 files changed, 17 insertions(+), 11 deletions(-)

include/linux/mmzone.h +5 -1

@@ -269,6 +269,11 @@ struct zone_reclaim_stat {
 	 */
 	unsigned long		recent_rotated[2];
 	unsigned long		recent_scanned[2];
+
+	/*
+	 * accumulated for batching
+	 */
+	unsigned long		nr_saved_scan[NR_LRU_LISTS];
 };
 
 struct zone {

@@ -323,7 +328,6 @@ struct zone {
 	spinlock_t		lru_lock;	
 	struct zone_lru {
 		struct list_head list;
-		unsigned long nr_saved_scan;	/* accumulated for batching */
 	} lru[NR_LRU_LISTS];
 
 	struct zone_reclaim_stat reclaim_stat;

mm/vmscan.c +11 -9

@@ -1521,6 +1521,7 @@ static void shrink_zone(int priority, st
 	enum lru_list l;
 	unsigned long nr_reclaimed = sc->nr_reclaimed;
 	unsigned long swap_cluster_max = sc->swap_cluster_max;
+	struct zone_reclaim_stat *reclaim_stat = get_reclaim_stat(zone, sc);
 	int noswap = 0;
 
 	/* If we have no swap space, do not bother scanning anon pages. */

@@ -1540,12 +1541,9 @@ static void shrink_zone(int priority, st
 			scan >>= priority;
 			scan = (scan * percent[file]) / 100;
 		}
-		if (scanning_global_lru(sc))
-			nr[l] = nr_scan_try_batch(scan,
-						  &zone->lru[l].nr_saved_scan,
-						  swap_cluster_max);
-		else
-			nr[l] = scan;
+		nr[l] = nr_scan_try_batch(scan,
+					  &reclaim_stat->nr_saved_scan[l],
+					  swap_cluster_max);
 	}
 
 	while (nr[LRU_INACTIVE_ANON] || nr[LRU_ACTIVE_FILE] ||

@@ -2128,6 +2126,7 @@ static void shrink_all_zones(unsigned lo
 {
 	struct zone *zone;
 	unsigned long nr_reclaimed = 0;
+	struct zone_reclaim_stat *reclaim_stat;
 
 	for_each_populated_zone(zone) {
 		enum lru_list l;

@@ -2144,11 +2143,14 @@ static void shrink_all_zones(unsigned lo
 						l == LRU_ACTIVE_FILE))
 				continue;
 
-			zone->lru[l].nr_saved_scan += (lru_pages >> prio) + 1;
-			if (zone->lru[l].nr_saved_scan >= nr_pages || pass > 3) {
+			reclaim_stat = get_reclaim_stat(zone, sc);
+			reclaim_stat->nr_saved_scan[l] +=
+						(lru_pages >> prio) + 1;
+			if (reclaim_stat->nr_saved_scan[l]
+						>= nr_pages || pass > 3) {
 				unsigned long nr_to_scan;
 
-				zone->lru[l].nr_saved_scan = 0;
+				reclaim_stat->nr_saved_scan[l] = 0;
 				nr_to_scan = min(nr_pages, lru_pages);
 				nr_reclaimed += shrink_list(l, nr_to_scan, zone,
 								sc, prio);

Quoted 32 lines · KAMEZAWA Hiroyuki

Quoted 21 lines · On Thu, 20 Aug 2009 10:49:29 +0800 Wu Fengguang <[email protected]> wrote:

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX. It effectively scales up the inactive list scan rate by up to 32 times.

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

Quoted 7 lines

CC Rik van Riel <[email protected]>

CC Minchan Kim <[email protected]>

CC Balbir Singh <[email protected]>

CC KOSAKI Motohiro <[email protected]>

CC KAMEZAWA Hiroyuki <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---

Quoted 6 lines

Hmm, how about this ?
==
Now, nr_saved_scan is tied to zone's LRU.
But, considering how vmscan works, it should be tied to reclaim_stat.

By this, memcg can make use of nr_saved_scan information seamlessly.

Good idea, full patch updated with your signed-off-by :)

Thanks,
Fengguang
---
mm: do batched scans for mem_cgroup

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX. It effectively scales up the inactive list scan rate by up to 32 times.

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

Quoted 65 lines · Wu Fengguang

Quoted 32 lines · KAMEZAWA Hiroyuki

Quoted 21 lines · On Thu, 20 Aug 2009 10:49:29 +0800 Wu Fengguang <[email protected]> wrote:

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX. It effectively scales up the inactive list scan rate by up to 32 times.

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

Quoted 7 lines

CC Rik van Riel <[email protected]>

CC Minchan Kim <[email protected]>

CC Balbir Singh <[email protected]>

CC KOSAKI Motohiro <[email protected]>

CC KAMEZAWA Hiroyuki <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---

Quoted 6 lines

Hmm, how about this ?
==
Now, nr_saved_scan is tied to zone's LRU.
But, considering how vmscan works, it should be tied to reclaim_stat.

By this, memcg can make use of nr_saved_scan information seamlessly.

Good idea, full patch updated with your signed-off-by :)

Thanks,
Fengguang
---
mm: do batched scans for mem_cgroup

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX. It effectively scales up the inactive list scan rate by up to 32 times.

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

Quoted 3 lines

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

Looks good to me, how did you test it?

Acked-by Balbir Singh <[email protected]>

Quoted 67 lines · Balbir Singh

Quoted 65 lines · Wu Fengguang

Quoted 32 lines · KAMEZAWA Hiroyuki

Quoted 21 lines · On Thu, 20 Aug 2009 10:49:29 +0800 Wu Fengguang <[email protected]> wrote:

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX. It effectively scales up the inactive list scan rate by up to 32 times.

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

Quoted 7 lines

CC Rik van Riel <[email protected]>

CC Minchan Kim <[email protected]>

CC Balbir Singh <[email protected]>

CC KOSAKI Motohiro <[email protected]>

CC KAMEZAWA Hiroyuki <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---

Quoted 6 lines

Hmm, how about this ?
==
Now, nr_saved_scan is tied to zone's LRU.
But, considering how vmscan works, it should be tied to reclaim_stat.

By this, memcg can make use of nr_saved_scan information seamlessly.

Good idea, full patch updated with your signed-off-by :)

Thanks,
Fengguang
---
mm: do batched scans for mem_cgroup

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX. It effectively scales up the inactive list scan rate by up to 32 times.

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

Quoted 5 lines

Quoted 3 lines

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

Looks good to me, how did you test it?

I observed the shrink_inactive_list() calls with this patch:

@@ -1043,6 +1043,13 @@ static unsigned long shrink_inactive_lis
struct zone_reclaim_stat *reclaim_stat = get_reclaim_stat(zone, sc); int lumpy_reclaim = 0;

+ if (!scanning_global_lru(sc))
+ printk("shrink inactive %s count=%lu scan=%lu\n",
+ file ? "file" : "anon",
+ mem_cgroup_zone_nr_pages(sc->mem_cgroup, zone,
+ LRU_INACTIVE_ANON + 2 * !!file),
+ max_scan);

and these commands:

        mkdir /cgroup/0
        echo 100M > /cgroup/0/memory.limit_in_bytes
        echo $$ > /cgroup/0/tasks
        cp /tmp/10G /dev/null

before patch:

        [ 3682.646008] shrink inactive file count=25535 scan=6
        [ 3682.661548] shrink inactive file count=25535 scan=6
        [ 3682.666933] shrink inactive file count=25535 scan=6
        [ 3682.682865] shrink inactive file count=25535 scan=6
        [ 3682.688572] shrink inactive file count=25535 scan=6
        [ 3682.703908] shrink inactive file count=25535 scan=6
        [ 3682.709431] shrink inactive file count=25535 scan=6

after patch:

        [  223.146544] shrink inactive file count=25531 scan=32
        [  223.152060] shrink inactive file count=25507 scan=10
        [  223.167503] shrink inactive file count=25531 scan=32
        [  223.173426] shrink inactive file count=25507 scan=10
        [  223.188764] shrink inactive file count=25531 scan=32
        [  223.194270] shrink inactive file count=25507 scan=10
        [  223.209885] shrink inactive file count=25531 scan=32
        [  223.215388] shrink inactive file count=25507 scan=10

Before patch, the inactive list is over scanned by 30/6=5 times; After patch, it is over scanned by 64/42=1.5 times. It's much better, and can be further improved if necessary.

Quoted 1 line

Acked-by Balbir Singh <[email protected]>

Thanks!

Quoted 32 lines · KAMEZAWA Hiroyuki

Quoted 21 lines · On Thu, 20 Aug 2009 10:49:29 +0800 Wu Fengguang <[email protected]> wrote:

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX.  It effectively scales up the inactive list scan rate by up to 32 times.

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

Quoted 7 lines

CC Rik van Riel <[email protected]>

CC Minchan Kim <[email protected]>

CC Balbir Singh <[email protected]>

CC KOSAKI Motohiro <[email protected]>

CC KAMEZAWA Hiroyuki <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---

Quoted 6 lines

Hmm, how about this ?
==
Now, nr_saved_scan is tied to zone's LRU.
But, considering how vmscan works, it should be tied to reclaim_stat.

By this, memcg can make use of nr_saved_scan information seamlessly.

Good idea, full patch updated with your signed-off-by :)

Thanks,
Fengguang
---
mm: do batched scans for mem_cgroup

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX.  It effectively scales up the inactive list scan rate by up to 32 times.

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

CC Rik van Riel <[email protected]>

CC Minchan Kim <[email protected]>

CC Balbir Singh <[email protected]>

CC KOSAKI Motohiro <[email protected]>

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---
 include/linux/mmzone.h |    6 +++++- mm/page_alloc.c        |    2 +- mm/vmscan.c            |   20 +++++++++++---------

 3 files changed, 17 insertions(+), 11 deletions(-)

include/linux/mmzone.h        2009-08-20 11:51:08.000000000 +0800 renamed from include/linux/mmzone.h   2009-07-30 10:45:15.000000000 +0800 +5 -1

@@ -269,6 +269,11 @@ struct zone_reclaim_stat {
         */
        unsigned long           recent_rotated[2];
        unsigned long           recent_scanned[2];
+
+       /*
+        * accumulated for batching
+        */
+       unsigned long           nr_saved_scan[NR_LRU_LISTS];
  };
 
  struct zone {

@@ -323,7 +328,6 @@ struct zone {
        spinlock_t              lru_lock;
        struct zone_lru {
                struct list_head list;
-               unsigned long nr_saved_scan;    /* accumulated for batching */
        } lru[NR_LRU_LISTS];
 
        struct zone_reclaim_stat reclaim_stat;

mm/vmscan.c   2009-08-20 12:00:55.000000000 +0800 renamed from mm/vmscan.c      2009-08-20 11:48:46.000000000 +0800 +4 -6

@@ -1521,6 +1521,7 @@ static void shrink_zone(int priority, st
        enum lru_list l;
        unsigned long nr_reclaimed = sc->nr_reclaimed;
        unsigned long swap_cluster_max = sc->swap_cluster_max;
+       struct zone_reclaim_stat *reclaim_stat = get_reclaim_stat(zone, sc);
        int noswap = 0;
 
        /* If we have no swap space, do not bother scanning anon pages. */

@@ -1540,12 +1541,9 @@ static void shrink_zone(int priority, st
                        scan >>= priority;
                        scan = (scan * percent[file]) / 100;
                }
-               if (scanning_global_lru(sc))
-                       nr[l] = nr_scan_try_batch(scan,
-                                                 &zone->lru[l].nr_saved_scan,
-                                                 swap_cluster_max);
-               else
-                       nr[l] = scan;
+               nr[l] = nr_scan_try_batch(scan,
+                                         &reclaim_stat->nr_saved_scan[l],
+                                         swap_cluster_max);
        }
 

       while (nr[LRU_INACTIVE_ANON] || nr[LRU_ACTIVE_FILE] ||

mm/vmscan.c   2009-08-20 12:00:55.000000000 +0800 renamed from mm/vmscan.c      2009-08-20 11:48:46.000000000 +0800 +7 -3

@@ -2128,6 +2126,7 @@ static void shrink_all_zones(unsigned lo
  {
        struct zone *zone;
        unsigned long nr_reclaimed = 0;
+       struct zone_reclaim_stat *reclaim_stat;
 
        for_each_populated_zone(zone) {
                enum lru_list l;

@@ -2144,11 +2143,14 @@ static void shrink_all_zones(unsigned lo
                                                l == LRU_ACTIVE_FILE))
                                continue;
 
-                       zone->lru[l].nr_saved_scan += (lru_pages >> prio) + 1;
-                       if (zone->lru[l].nr_saved_scan >= nr_pages || pass > 3) {
+                       reclaim_stat = get_reclaim_stat(zone, sc);
+                       reclaim_stat->nr_saved_scan[l] +=
+                                               (lru_pages >> prio) + 1;
+                       if (reclaim_stat->nr_saved_scan[l]
+                                               >= nr_pages || pass > 3) {
                                unsigned long nr_to_scan;
 
-                               zone->lru[l].nr_saved_scan = 0;
+                               reclaim_stat->nr_saved_scan[l] = 0;
                                nr_to_scan = min(nr_pages, lru_pages);
                                nr_reclaimed += shrink_list(l, nr_to_scan, zone,
                                                                sc, prio);

mm/page_alloc.c       2009-08-20 11:58:39.000000000 +0800 renamed from mm/page_alloc.c  2009-08-20 11:57:54.000000000 +0800 +1 -2

@@ -3716,7 +3716,7 @@ static void __paginginit free_area_init_
                zone_pcp_init(zone);
                for_each_lru(l) {
                        INIT_LIST_HEAD(&zone->lru[l].list);
-                       zone->lru[l].nr_saved_scan = 0;
+                       zone->reclaim_stat.nr_saved_scan[l] = 0;
                }
                zone->reclaim_stat.recent_rotated[0] = 0;
                zone->reclaim_stat.recent_rotated[1] = 0;
 
--

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

Hi, Wu.

Quoted 47 lines · Wu Fengguang

Quoted 32 lines · KAMEZAWA Hiroyuki

Quoted 21 lines · On Thu, 20 Aug 2009 10:49:29 +0800 Wu Fengguang <[email protected]> wrote:

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX. A It effectively scales up the inactive list scan rate by up to 32 times.

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

Quoted 7 lines

CC Rik van Riel <[email protected]>

CC Minchan Kim <[email protected]>

CC Balbir Singh <[email protected]>

CC KOSAKI Motohiro <[email protected]>

CC KAMEZAWA Hiroyuki <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---

Quoted 6 lines

Hmm, how about this ?
==
Now, nr_saved_scan is tied to zone's LRU.
But, considering how vmscan works, it should be tied to reclaim_stat.

By this, memcg can make use of nr_saved_scan information seamlessly.

Good idea, full patch updated with your signed-off-by :)

Thanks,
Fengguang
---
mm: do batched scans for mem_cgroup

Quoted 4 lines

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX. A It effectively scales up the inactive list scan rate by up to 32 times.

Yes. It can scan 32 times pages in only inactive list, not active list.

Quoted 3 lines

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

Active list scan would be scanned in 4, inactive list is 32.

Quoted 4 lines

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

Yes.

Quoted 4 lines

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

You mean nr_scan_try_batch logic ?
But that logic works for just global reclaim?
Now am I missing something?

Could you elaborate more? :)

Quoted 101 lines

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

CC Rik van Riel <[email protected]>

CC Minchan Kim <[email protected]>

CC Balbir Singh <[email protected]>

CC KOSAKI Motohiro <[email protected]>

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---
A include/linux/mmzone.h | A A 6 +++++-
A mm/page_alloc.c A A A A | A A 2 +-
A mm/vmscan.c A A A A A A | A 20 +++++++++++--------- A 3 files changed, 17 insertions(+), 11 deletions(-)

include/linux/mmzone.h A A A A 2009-08-20 11:51:08.000000000 +0800 renamed from include/linux/mmzone.h A 2009-07-30 10:45:15.000000000 +0800 +0 -0

@@ -269,6 +269,11 @@ struct zone_reclaim_stat {

A  A  A  A  */
A  A  A  A unsigned long A  A  A  A  A  recent_rotated[2];
A  A  A  A unsigned long A  A  A  A  A  recent_scanned[2];
+
+ A  A  A  /*
+ A  A  A  A * accumulated for batching
+ A  A  A  A */
+ A  A  A  unsigned long A  A  A  A  A  nr_saved_scan[NR_LRU_LISTS];

A };

A struct zone {

include/linux/mmzone.h A A A A 2009-08-20 11:51:08.000000000 +0800 renamed from include/linux/mmzone.h A 2009-07-30 10:45:15.000000000 +0800 +0 -1

@@ -323,7 +328,6 @@ struct zone {
 A  A  A  A spinlock_t A  A  A  A  A  A  A lru_lock;
 A  A  A  A struct zone_lru {
 A  A  A  A  A  A  A  A struct list_head list;
- A  A  A  A  A  A  A  unsigned long nr_saved_scan; A  A /* accumulated for batching */
 A  A  A  A } lru[NR_LRU_LISTS];
 
 A  A  A  A struct zone_reclaim_stat reclaim_stat;

mm/vmscan.c A 2009-08-20 12:00:55.000000000 +0800 renamed from mm/vmscan.c A A A 2009-08-20 11:48:46.000000000 +0800 +1 -0

@@ -1521,6 +1521,7 @@ static void shrink_zone(int priority, st
 A  A  A  A enum lru_list l;
 A  A  A  A unsigned long nr_reclaimed = sc->nr_reclaimed;
 A  A  A  A unsigned long swap_cluster_max = sc->swap_cluster_max;
+ A  A  A  struct zone_reclaim_stat *reclaim_stat = get_reclaim_stat(zone, sc);
 A  A  A  A int noswap = 0;
 

A A A A /* If we have no swap space, do not bother scanning anon pages. */

mm/vmscan.c A 2009-08-20 12:00:55.000000000 +0800 renamed from mm/vmscan.c A A A 2009-08-20 11:48:46.000000000 +0800 +3 -6

@@ -1540,12 +1541,9 @@ static void shrink_zone(int priority, st
 A  A  A  A  A  A  A  A  A  A  A  A scan >>= priority;
 A  A  A  A  A  A  A  A  A  A  A  A scan = (scan * percent[file]) / 100;
 A  A  A  A  A  A  A  A }
- A  A  A  A  A  A  A  if (scanning_global_lru(sc))
- A  A  A  A  A  A  A  A  A  A  A  nr[l] = nr_scan_try_batch(scan,
- A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  &zone->lru[l].nr_saved_scan,
- A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  swap_cluster_max);
- A  A  A  A  A  A  A  else
- A  A  A  A  A  A  A  A  A  A  A  nr[l] = scan;
+ A  A  A  A  A  A  A  nr[l] = nr_scan_try_batch(scan,
+ A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  &reclaim_stat->nr_saved_scan[l],
+ A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  swap_cluster_max);
 A  A  A  A }
 

A A A A while (nr[LRU_INACTIVE_ANON] || nr[LRU_ACTIVE_FILE] ||

mm/vmscan.c A 2009-08-20 12:00:55.000000000 +0800 renamed from mm/vmscan.c A A A 2009-08-20 11:48:46.000000000 +0800 +7 -3

@@ -2128,6 +2126,7 @@ static void shrink_all_zones(unsigned lo
 A {
 A  A  A  A struct zone *zone;
 A  A  A  A unsigned long nr_reclaimed = 0;
+ A  A  A  struct zone_reclaim_stat *reclaim_stat;
 
 A  A  A  A for_each_populated_zone(zone) {
 A  A  A  A  A  A  A  A enum lru_list l;

@@ -2144,11 +2143,14 @@ static void shrink_all_zones(unsigned lo
 A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A l == LRU_ACTIVE_FILE))
 A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A continue;
 
- A  A  A  A  A  A  A  A  A  A  A  zone->lru[l].nr_saved_scan += (lru_pages >> prio) + 1;
- A  A  A  A  A  A  A  A  A  A  A  if (zone->lru[l].nr_saved_scan >= nr_pages || pass > 3) {
+ A  A  A  A  A  A  A  A  A  A  A  reclaim_stat = get_reclaim_stat(zone, sc);
+ A  A  A  A  A  A  A  A  A  A  A  reclaim_stat->nr_saved_scan[l] +=
+ A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  (lru_pages >> prio) + 1;
+ A  A  A  A  A  A  A  A  A  A  A  if (reclaim_stat->nr_saved_scan[l]
+ A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  >= nr_pages || pass > 3) {
 A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A unsigned long nr_to_scan;
 
- A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  zone->lru[l].nr_saved_scan = 0;
+ A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  reclaim_stat->nr_saved_scan[l] = 0;
 A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A nr_to_scan = min(nr_pages, lru_pages);
 A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A nr_reclaimed += shrink_list(l, nr_to_scan, zone,
 A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A sc, prio);

mm/page_alloc.c A A A 2009-08-20 11:58:39.000000000 +0800 renamed from mm/page_alloc.c A 2009-08-20 11:57:54.000000000 +0800 +1 -1

@@ -3716,7 +3716,7 @@ static void __paginginit free_area_init_
 A  A  A  A  A  A  A  A zone_pcp_init(zone);
 A  A  A  A  A  A  A  A for_each_lru(l) {
 A  A  A  A  A  A  A  A  A  A  A  A INIT_LIST_HEAD(&zone->lru[l].list);
- A  A  A  A  A  A  A  A  A  A  A  zone->lru[l].nr_saved_scan = 0;
+ A  A  A  A  A  A  A  A  A  A  A  zone->reclaim_stat.nr_saved_scan[l] = 0;
 A  A  A  A  A  A  A  A }
 A  A  A  A  A  A  A  A zone->reclaim_stat.recent_rotated[0] = 0;
 A  A  A  A  A  A  A  A zone->reclaim_stat.recent_rotated[1] = 0;

Quoted 6 lines

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

Signature · 2 lines

Kind regards,
Minchan Kim

Quoted 55 lines · Minchan Kim

Hi, Wu.

Quoted 51 lines · Wu Fengguang

Quoted 32 lines · KAMEZAWA Hiroyuki

Quoted 21 lines · On Thu, 20 Aug 2009 10:49:29 +0800 Wu Fengguang <[email protected]> wrote:

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX.  It effectively scales up the inactive list scan rate by up to 32 times.

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

Quoted 7 lines

CC Rik van Riel <[email protected]>

CC Minchan Kim <[email protected]>

CC Balbir Singh <[email protected]>

CC KOSAKI Motohiro <[email protected]>

CC KAMEZAWA Hiroyuki <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---

Quoted 6 lines

Hmm, how about this ?
==
Now, nr_saved_scan is tied to zone's LRU.
But, considering how vmscan works, it should be tied to reclaim_stat.

By this, memcg can make use of nr_saved_scan information seamlessly.

Good idea, full patch updated with your signed-off-by :)

Thanks,
Fengguang
---
mm: do batched scans for mem_cgroup

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX.  It effectively scales up the inactive list scan rate by up to 32 times.

Quoted 1 line

Yes. It can scan 32 times pages in only inactive list, not active list.

Yes and no ;)

inactive anon list over scanned => inactive_anon_is_low() == TRUE => shrink_active_list() => active anon list over scanned

So the end result may be

- anon inactive  => over scanned
- anon active    => over scanned (maybe not as much)
- file inactive  => over scanned
- file active    => under scanned (relatively)

Quoted 5 lines

Quoted 3 lines

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

Active list scan would be scanned in 4,  inactive list  is 32.

Exactly.

Quoted 16 lines

Quoted 4 lines

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

Yes.

Quoted 4 lines

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

You mean nr_scan_try_batch logic ?
But that logic works for just global reclaim?
Now am I missing something?

Quoted 1 line

Could you elaborate more? :)

Sorry for the confusion. The above paragraph originates from Balbir's concern:

       This might be a concern (although not a big ATM), since we can't afford to miss limits by much. If a cgroup is near its limit and we drop scanning it. We'll have to work out what this means for the end

       user. May be more fundamental look through is required at the priority based logic of exposing how much to scan, I don't know.

Thanks,
Fengguang

Quoted 113 lines

Quoted 102 lines

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

CC Rik van Riel <[email protected]>

CC Minchan Kim <[email protected]>

CC Balbir Singh <[email protected]>

CC KOSAKI Motohiro <[email protected]>

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---
 include/linux/mmzone.h |    6 +++++- mm/page_alloc.c        |    2 +- mm/vmscan.c            |   20 +++++++++++---------

 3 files changed, 17 insertions(+), 11 deletions(-)

include/linux/mmzone.h        2009-08-20 11:51:08.000000000 +0800 renamed from include/linux/mmzone.h   2009-07-30 10:45:15.000000000 +0800 +5 -1

@@ -269,6 +269,11 @@ struct zone_reclaim_stat {
         */
        unsigned long           recent_rotated[2];
        unsigned long           recent_scanned[2];
+
+       /*
+        * accumulated for batching
+        */
+       unsigned long           nr_saved_scan[NR_LRU_LISTS];
  };
 
  struct zone {

@@ -323,7 +328,6 @@ struct zone {
        spinlock_t              lru_lock;
        struct zone_lru {
                struct list_head list;
-               unsigned long nr_saved_scan;    /* accumulated for batching */
        } lru[NR_LRU_LISTS];
 
        struct zone_reclaim_stat reclaim_stat;

mm/vmscan.c   2009-08-20 12:00:55.000000000 +0800 renamed from mm/vmscan.c      2009-08-20 11:48:46.000000000 +0800 +4 -6

@@ -1521,6 +1521,7 @@ static void shrink_zone(int priority, st
        enum lru_list l;
        unsigned long nr_reclaimed = sc->nr_reclaimed;
        unsigned long swap_cluster_max = sc->swap_cluster_max;
+       struct zone_reclaim_stat *reclaim_stat = get_reclaim_stat(zone, sc);
        int noswap = 0;
 
        /* If we have no swap space, do not bother scanning anon pages. */

@@ -1540,12 +1541,9 @@ static void shrink_zone(int priority, st
                        scan >>= priority;
                        scan = (scan * percent[file]) / 100;
                }
-               if (scanning_global_lru(sc))
-                       nr[l] = nr_scan_try_batch(scan,
-                                                 &zone->lru[l].nr_saved_scan,
-                                                 swap_cluster_max);
-               else
-                       nr[l] = scan;
+               nr[l] = nr_scan_try_batch(scan,
+                                         &reclaim_stat->nr_saved_scan[l],
+                                         swap_cluster_max);
        }
 

       while (nr[LRU_INACTIVE_ANON] || nr[LRU_ACTIVE_FILE] ||

mm/vmscan.c   2009-08-20 12:00:55.000000000 +0800 renamed from mm/vmscan.c      2009-08-20 11:48:46.000000000 +0800 +7 -3

@@ -2128,6 +2126,7 @@ static void shrink_all_zones(unsigned lo
  {
        struct zone *zone;
        unsigned long nr_reclaimed = 0;
+       struct zone_reclaim_stat *reclaim_stat;
 
        for_each_populated_zone(zone) {
                enum lru_list l;

@@ -2144,11 +2143,14 @@ static void shrink_all_zones(unsigned lo
                                                l == LRU_ACTIVE_FILE))
                                continue;
 
-                       zone->lru[l].nr_saved_scan += (lru_pages >> prio) + 1;
-                       if (zone->lru[l].nr_saved_scan >= nr_pages || pass > 3) {
+                       reclaim_stat = get_reclaim_stat(zone, sc);
+                       reclaim_stat->nr_saved_scan[l] +=
+                                               (lru_pages >> prio) + 1;
+                       if (reclaim_stat->nr_saved_scan[l]
+                                               >= nr_pages || pass > 3) {
                                unsigned long nr_to_scan;
 
-                               zone->lru[l].nr_saved_scan = 0;
+                               reclaim_stat->nr_saved_scan[l] = 0;
                                nr_to_scan = min(nr_pages, lru_pages);
                                nr_reclaimed += shrink_list(l, nr_to_scan, zone,
                                                                sc, prio);

mm/page_alloc.c       2009-08-20 11:58:39.000000000 +0800 renamed from mm/page_alloc.c  2009-08-20 11:57:54.000000000 +0800 +1 -2

@@ -3716,7 +3716,7 @@ static void __paginginit free_area_init_
                zone_pcp_init(zone);
                for_each_lru(l) {
                        INIT_LIST_HEAD(&zone->lru[l].list);
-                       zone->lru[l].nr_saved_scan = 0;
+                       zone->reclaim_stat.nr_saved_scan[l] = 0;
                }
                zone->reclaim_stat.recent_rotated[0] = 0;
                zone->reclaim_stat.recent_rotated[1] = 0;
 
--

Quoted 5 lines

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

--
Kind regards,
Minchan Kim

Quoted 56 lines · Wu Fengguang

Quoted 55 lines · Minchan Kim

Hi, Wu.

Quoted 51 lines · Wu Fengguang

Quoted 32 lines · KAMEZAWA Hiroyuki

Quoted 21 lines · On Thu, 20 Aug 2009 10:49:29 +0800 Wu Fengguang <[email protected]> wrote:

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX. A It effectively scales up the inactive list scan rate by up to 32 times.

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

Quoted 7 lines

CC Rik van Riel <[email protected]>

CC Minchan Kim <[email protected]>

CC Balbir Singh <[email protected]>

CC KOSAKI Motohiro <[email protected]>

CC KAMEZAWA Hiroyuki <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---

Quoted 6 lines

Hmm, how about this ?
==
Now, nr_saved_scan is tied to zone's LRU.
But, considering how vmscan works, it should be tied to reclaim_stat.

By this, memcg can make use of nr_saved_scan information seamlessly.

Good idea, full patch updated with your signed-off-by :)

Thanks,
Fengguang
---
mm: do batched scans for mem_cgroup

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX. A It effectively scales up the inactive list scan rate by up to 32 times.

Quoted 7 lines

Quoted 1 line

Yes. It can scan 32 times pages in only inactive list, not active list.

Yes and no ;)

inactive anon list over scanned => inactive_anon_is_low() == TRUE

A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A => shrink_active_list()
A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A => active anon list over scanned

Why inactive anon list is overscanned in case mem_cgroup ?

in shrink_zone,
1) The vm doesn't accumulate nr[l].
2) Below routine store min value to nr_to_scan. nr_to_scan = min(nr[l], swap_cluster_max);
ex) if nr[l] = 4, vm calls shrink_active_list with 4 as nr_to_scan.

So I think overscan doesn't occur in active list.

Quoted 33 lines

So the end result may be

- anon inactive A => over scanned
- anon active A A => over scanned (maybe not as much)
- file inactive A => over scanned
- file active A A => under scanned (relatively)

Quoted 5 lines

Quoted 3 lines

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

Active list scan would be scanned in 4, A inactive list A is 32.

Exactly.

Quoted 17 lines

Quoted 4 lines

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

Yes.

Quoted 4 lines

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

You mean nr_scan_try_batch logic ?
But that logic works for just global reclaim?
Now am I missing something?

Could you elaborate more? :)

Quoted 6 lines

Sorry for the confusion. The above paragraph originates from Balbir's concern:

A  A  A  A This might be a concern (although not a big ATM), since we can't
A  A  A  A afford to miss limits by much. If a cgroup is near its limit and we
A  A  A  A drop scanning it. We'll have to work out what this means for the end

Why does mem_cgroup drops scanning ?

It's because nr_scan_try_batch? or something ?

Sorry. Still, I can't understand your point. :(

Quoted 107 lines

A  A  A  A user. May be more fundamental look through is required at the priority
A  A  A  A based logic of exposing how much to scan, I don't know.

Thanks,
Fengguang

Quoted 101 lines

Quoted 101 lines

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

CC Rik van Riel <[email protected]>

CC Minchan Kim <[email protected]>

CC Balbir Singh <[email protected]>

CC KOSAKI Motohiro <[email protected]>

Signed-off-by KAMEZAWA Hiroyuki <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

---
A include/linux/mmzone.h | A A 6 +++++-
A mm/page_alloc.c A A A A | A A 2 +-
A mm/vmscan.c A A A A A A | A 20 +++++++++++--------- A 3 files changed, 17 insertions(+), 11 deletions(-)

include/linux/mmzone.h A A A A 2009-08-20 11:51:08.000000000 +0800 renamed from include/linux/mmzone.h A 2009-07-30 10:45:15.000000000 +0800 +0 -0

@@ -269,6 +269,11 @@ struct zone_reclaim_stat {

A  A  A  A  */
A  A  A  A unsigned long A  A  A  A  A  recent_rotated[2];
A  A  A  A unsigned long A  A  A  A  A  recent_scanned[2];
+
+ A  A  A  /*
+ A  A  A  A * accumulated for batching
+ A  A  A  A */
+ A  A  A  unsigned long A  A  A  A  A  nr_saved_scan[NR_LRU_LISTS];

A };

A struct zone {

include/linux/mmzone.h A A A A 2009-08-20 11:51:08.000000000 +0800 renamed from include/linux/mmzone.h A 2009-07-30 10:45:15.000000000 +0800 +0 -1

@@ -323,7 +328,6 @@ struct zone {
 A  A  A  A spinlock_t A  A  A  A  A  A  A lru_lock;
 A  A  A  A struct zone_lru {
 A  A  A  A  A  A  A  A struct list_head list;
- A  A  A  A  A  A  A  unsigned long nr_saved_scan; A  A /* accumulated for batching */
 A  A  A  A } lru[NR_LRU_LISTS];
 
 A  A  A  A struct zone_reclaim_stat reclaim_stat;

mm/vmscan.c A 2009-08-20 12:00:55.000000000 +0800 renamed from mm/vmscan.c A A A 2009-08-20 11:48:46.000000000 +0800 +1 -0

@@ -1521,6 +1521,7 @@ static void shrink_zone(int priority, st
 A  A  A  A enum lru_list l;
 A  A  A  A unsigned long nr_reclaimed = sc->nr_reclaimed;
 A  A  A  A unsigned long swap_cluster_max = sc->swap_cluster_max;
+ A  A  A  struct zone_reclaim_stat *reclaim_stat = get_reclaim_stat(zone, sc);
 A  A  A  A int noswap = 0;
 

A A A A /* If we have no swap space, do not bother scanning anon pages. */

mm/vmscan.c A 2009-08-20 12:00:55.000000000 +0800 renamed from mm/vmscan.c A A A 2009-08-20 11:48:46.000000000 +0800 +3 -6

@@ -1540,12 +1541,9 @@ static void shrink_zone(int priority, st
 A  A  A  A  A  A  A  A  A  A  A  A scan >>= priority;
 A  A  A  A  A  A  A  A  A  A  A  A scan = (scan * percent[file]) / 100;
 A  A  A  A  A  A  A  A }
- A  A  A  A  A  A  A  if (scanning_global_lru(sc))
- A  A  A  A  A  A  A  A  A  A  A  nr[l] = nr_scan_try_batch(scan,
- A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  &zone->lru[l].nr_saved_scan,
- A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  swap_cluster_max);
- A  A  A  A  A  A  A  else
- A  A  A  A  A  A  A  A  A  A  A  nr[l] = scan;
+ A  A  A  A  A  A  A  nr[l] = nr_scan_try_batch(scan,
+ A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  &reclaim_stat->nr_saved_scan[l],
+ A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  swap_cluster_max);
 A  A  A  A }
 

A A A A while (nr[LRU_INACTIVE_ANON] || nr[LRU_ACTIVE_FILE] ||

mm/vmscan.c A 2009-08-20 12:00:55.000000000 +0800 renamed from mm/vmscan.c A A A 2009-08-20 11:48:46.000000000 +0800 +7 -3

@@ -2128,6 +2126,7 @@ static void shrink_all_zones(unsigned lo
 A {
 A  A  A  A struct zone *zone;
 A  A  A  A unsigned long nr_reclaimed = 0;
+ A  A  A  struct zone_reclaim_stat *reclaim_stat;
 
 A  A  A  A for_each_populated_zone(zone) {
 A  A  A  A  A  A  A  A enum lru_list l;

@@ -2144,11 +2143,14 @@ static void shrink_all_zones(unsigned lo
 A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A l == LRU_ACTIVE_FILE))
 A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A continue;
 
- A  A  A  A  A  A  A  A  A  A  A  zone->lru[l].nr_saved_scan += (lru_pages >> prio) + 1;
- A  A  A  A  A  A  A  A  A  A  A  if (zone->lru[l].nr_saved_scan >= nr_pages || pass > 3) {
+ A  A  A  A  A  A  A  A  A  A  A  reclaim_stat = get_reclaim_stat(zone, sc);
+ A  A  A  A  A  A  A  A  A  A  A  reclaim_stat->nr_saved_scan[l] +=
+ A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  (lru_pages >> prio) + 1;
+ A  A  A  A  A  A  A  A  A  A  A  if (reclaim_stat->nr_saved_scan[l]
+ A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  >= nr_pages || pass > 3) {
 A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A unsigned long nr_to_scan;
 
- A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  zone->lru[l].nr_saved_scan = 0;
+ A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  reclaim_stat->nr_saved_scan[l] = 0;
 A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A nr_to_scan = min(nr_pages, lru_pages);
 A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A nr_reclaimed += shrink_list(l, nr_to_scan, zone,
 A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A  A sc, prio);

mm/page_alloc.c A A A 2009-08-20 11:58:39.000000000 +0800 renamed from mm/page_alloc.c A 2009-08-20 11:57:54.000000000 +0800 +1 -1

@@ -3716,7 +3716,7 @@ static void __paginginit free_area_init_
 A  A  A  A  A  A  A  A zone_pcp_init(zone);
 A  A  A  A  A  A  A  A for_each_lru(l) {
 A  A  A  A  A  A  A  A  A  A  A  A INIT_LIST_HEAD(&zone->lru[l].list);
- A  A  A  A  A  A  A  A  A  A  A  zone->lru[l].nr_saved_scan = 0;
+ A  A  A  A  A  A  A  A  A  A  A  zone->reclaim_stat.nr_saved_scan[l] = 0;
 A  A  A  A  A  A  A  A }
 A  A  A  A  A  A  A  A zone->reclaim_stat.recent_rotated[0] = 0;
 A  A  A  A  A  A  A  A zone->reclaim_stat.recent_rotated[1] = 0;

Quoted 13 lines

Quoted 12 lines

Quoted 6 lines

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

--
Kind regards,
Minchan Kim

Signature · 2 lines

Kind regards,
Minchan Kim

Quoted 24 lines · KAMEZAWA Hiroyuki

Quoted 21 lines · On Thu, 20 Aug 2009 10:49:29 +0800 Wu Fengguang <[email protected]> wrote:

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX.  It effectively scales up the inactive list scan rate by up to 32 times.

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, it will be scanned SWAP_CLUSTER_MAX=32 pages in effect.

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.

This batching won't blur up the cgroup limits, since it is driven by "pages reclaimed" rather than "pages scanned". When shrink_zone() decides to cancel (and save) one smallish scan, it may well be called again to accumulate up nr_saved_scan.

It could possibly be a problem for some tiny mem_cgroup (which may be _full_ scanned too much times in order to accumulate up nr_saved_scan).

Quoted 6 lines

Quoted 6 lines

CC Rik van Riel <[email protected]>

CC Minchan Kim <[email protected]>

CC Balbir Singh <[email protected]>

CC KOSAKI Motohiro <[email protected]>

CC KAMEZAWA Hiroyuki <[email protected]>

Signed-off-by Wu Fengguang <[email protected]>

For mem_cgroup, shrink_zone() may call shrink_list() with nr_to_scan=1, in which case shrink_list() _still_ calls isolate_pages() with the much larger SWAP_CLUSTER_MAX.  It effectively scales up the inactive list scan rate by up to 32 times.

For example, with 16k inactive pages and DEF_PRIORITY=12, (16k >> 12)=4. So when shrink_zone() expects to scan 4 pages in the active/inactive list, the active list will be scanned 4 pages, while the inactive list will be (over) scanned SWAP_CLUSTER_MAX=32 pages in effect. And that could break the balance between the two lists.

It can further impact the scan of anon active list, due to the anon active/inactive ratio rebalance logic in balance_pgdat()/shrink_zone():

inactive anon list over scanned => inactive_anon_is_low() == TRUE => shrink_active_list() => active anon list over scanned

So the end result may be

- anon inactive  => over scanned
- anon active    => over scanned (maybe not as much)
- file inactive  => over scanned
- file active    => under scanned (relatively)

The accesses to nr_saved_scan are not lock protected and so not 100% accurate, however we can tolerate small errors and the resulted small imbalanced scan rates between zones.