diff options
author | Greg Thelen <gthelen@google.com> | 2015-10-01 18:37:02 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2015-10-01 21:42:35 -0400 |
commit | 0610c25daa3e76e38ad5a8fae683a89ff9f71798 (patch) | |
tree | 4e37a050e59f20773ec59af2191b9bc6b9da66fe /mm | |
parent | 8346c416d17bf5b4ea1508662959bb62e73fd6a5 (diff) |
memcg: fix dirty page migration
The problem starts with a file backed dirty page which is charged to a
memcg. Then page migration is used to move oldpage to newpage.
Migration:
- copies the oldpage's data to newpage
- clears oldpage.PG_dirty
- sets newpage.PG_dirty
- uncharges oldpage from memcg
- charges newpage to memcg
Clearing oldpage.PG_dirty decrements the charged memcg's dirty page
count.
However, because newpage is not yet charged, setting newpage.PG_dirty
does not increment the memcg's dirty page count. After migration
completes newpage.PG_dirty is eventually cleared, often in
account_page_cleaned(). At this time newpage is charged to a memcg so
the memcg's dirty page count is decremented which causes underflow
because the count was not previously incremented by migration. This
underflow causes balance_dirty_pages() to see a very large unsigned
number of dirty memcg pages which leads to aggressive throttling of
buffered writes by processes in non root memcg.
This issue:
- can harm performance of non root memcg buffered writes.
- can report too small (even negative) values in
memory.stat[(total_)dirty] counters of all memcg, including the root.
To avoid polluting migrate.c with #ifdef CONFIG_MEMCG checks, introduce
page_memcg() and set_page_memcg() helpers.
Test:
0) setup and enter limited memcg
mkdir /sys/fs/cgroup/test
echo 1G > /sys/fs/cgroup/test/memory.limit_in_bytes
echo $$ > /sys/fs/cgroup/test/cgroup.procs
1) buffered writes baseline
dd if=/dev/zero of=/data/tmp/foo bs=1M count=1k
sync
grep ^dirty /sys/fs/cgroup/test/memory.stat
2) buffered writes with compaction antagonist to induce migration
yes 1 > /proc/sys/vm/compact_memory &
rm -rf /data/tmp/foo
dd if=/dev/zero of=/data/tmp/foo bs=1M count=1k
kill %
sync
grep ^dirty /sys/fs/cgroup/test/memory.stat
3) buffered writes without antagonist, should match baseline
rm -rf /data/tmp/foo
dd if=/dev/zero of=/data/tmp/foo bs=1M count=1k
sync
grep ^dirty /sys/fs/cgroup/test/memory.stat
(speed, dirty residue)
unpatched patched
1) 841 MB/s 0 dirty pages 886 MB/s 0 dirty pages
2) 611 MB/s -33427456 dirty pages 793 MB/s 0 dirty pages
3) 114 MB/s -33427456 dirty pages 891 MB/s 0 dirty pages
Notice that unpatched baseline performance (1) fell after
migration (3): 841 -> 114 MB/s. In the patched kernel, post
migration performance matches baseline.
Fixes: c4843a7593a9 ("memcg: add per cgroup dirty page accounting")
Signed-off-by: Greg Thelen <gthelen@google.com>
Reported-by: Dave Hansen <dave.hansen@intel.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Cc: <stable@vger.kernel.org> [4.2+]
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Diffstat (limited to 'mm')
-rw-r--r-- | mm/migrate.c | 12 |
1 files changed, 11 insertions, 1 deletions
diff --git a/mm/migrate.c b/mm/migrate.c index 7452a00bbb50..842ecd7aaf7f 100644 --- a/mm/migrate.c +++ b/mm/migrate.c | |||
@@ -740,6 +740,15 @@ static int move_to_new_page(struct page *newpage, struct page *page, | |||
740 | if (PageSwapBacked(page)) | 740 | if (PageSwapBacked(page)) |
741 | SetPageSwapBacked(newpage); | 741 | SetPageSwapBacked(newpage); |
742 | 742 | ||
743 | /* | ||
744 | * Indirectly called below, migrate_page_copy() copies PG_dirty and thus | ||
745 | * needs newpage's memcg set to transfer memcg dirty page accounting. | ||
746 | * So perform memcg migration in two steps: | ||
747 | * 1. set newpage->mem_cgroup (here) | ||
748 | * 2. clear page->mem_cgroup (below) | ||
749 | */ | ||
750 | set_page_memcg(newpage, page_memcg(page)); | ||
751 | |||
743 | mapping = page_mapping(page); | 752 | mapping = page_mapping(page); |
744 | if (!mapping) | 753 | if (!mapping) |
745 | rc = migrate_page(mapping, newpage, page, mode); | 754 | rc = migrate_page(mapping, newpage, page, mode); |
@@ -756,9 +765,10 @@ static int move_to_new_page(struct page *newpage, struct page *page, | |||
756 | rc = fallback_migrate_page(mapping, newpage, page, mode); | 765 | rc = fallback_migrate_page(mapping, newpage, page, mode); |
757 | 766 | ||
758 | if (rc != MIGRATEPAGE_SUCCESS) { | 767 | if (rc != MIGRATEPAGE_SUCCESS) { |
768 | set_page_memcg(newpage, NULL); | ||
759 | newpage->mapping = NULL; | 769 | newpage->mapping = NULL; |
760 | } else { | 770 | } else { |
761 | mem_cgroup_migrate(page, newpage, false); | 771 | set_page_memcg(page, NULL); |
762 | if (page_was_mapped) | 772 | if (page_was_mapped) |
763 | remove_migration_ptes(page, newpage); | 773 | remove_migration_ptes(page, newpage); |
764 | page->mapping = NULL; | 774 | page->mapping = NULL; |