login
Search
This series is update from v5. easy 4 patches are already posted as ready-to-go-series. This is need-more-discuss set. Includes following 6 patches. (reduced from v5). The whole series are reordered. [1/6] make page_cgroup->flags to be atomic. [2/6] allocate all page_cgroup at boot. [3/6] rewrite charge path by charge/commit/cancel [4/6] new force_empty and move_account [5/6] lazy lru free [6/6] lazy lru add. Patch [3/6] and [4/6] are totally rewritten. Races in Patch [6/6] is fixed....I think. Patch [1-4] seems to be big but there is no complicated ops. Patch [5-6] is more racy. Check-by-regression-test is necessary. (Of course, I does some.) If ready-to-go-series goes, next is patch 1 and 2. Thanks, -Kame --
This patch makes page_cgroup->flags to be atomic_ops and define
functions (and macros) to access it.
Before trying to modify memory resource controller, this atomic operation
on flags is necessary. Most of flags in this patch is for LRU and modfied
under mz->lru_lock but we'll add another flags which is not for LRU soon.
(lock_page_cgroup() will use LOCK bit on page_cgroup->flags)
So we use atomic version here.
Changelog: (v5) -> (v6)
- no changes.
Changelog: (v4) -> (v5)
- removed unsued operations.
- adjusted to new ctype MEM_CGROUP_CHARGE_TYPE_SHMEM
Changelog: (v3) -> (v4)
- no changes.
Changelog: (v2) -> (v3)
- renamed macros and flags to be longer name.
- added comments.
- added "default bit set" for File, Shmem, Anon.
Changelog: (preview) -> (v1):
- patch ordering is changed.
- Added macro for defining functions for Test/Set/Clear bit.
- made the names of flags shorter.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
mm/memcontrol.c | 122 +++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 82 insertions(+), 40 deletions(-)
Index: mmotm-2.6.27-rc7+/mm/memcontrol.c
===================================================================
--- mmotm-2.6.27-rc7+.orig/mm/memcontrol.c
+++ mmotm-2.6.27-rc7+/mm/memcontrol.c
@@ -157,12 +157,46 @@ struct page_cgroup {
struct list_head lru; /* per cgroup LRU list */
struct page *page;
struct mem_cgroup *mem_cgroup;
- int flags;
+ unsigned long flags;
};
-#define PAGE_CGROUP_FLAG_CACHE (0x1) /* charged as cache */
-#define PAGE_CGROUP_FLAG_ACTIVE (0x2) /* page is active in this cgroup */
-#define PAGE_CGROUP_FLAG_FILE (0x4) /* page is file system backed */
-#define PAGE_CGROUP_FLAG_UNEVICTABLE (0x8) /* page is unevictableable */
+
+enum {
+ /* flags for mem_cgroup */
+ PCG_CACHE, /* charged as cache */
+ /* flags for LRU placement */
+ PCG_ACTIVE, /* page is active in this cgroup */
+ PCG_FILE, /* page is file system backed ...Seems quite straightforward Acked-by: Balbir Singh <balbir@linux.vnet.ibm.com> -- Balbir --
Allocate all page_cgroup at boot and remove page_cgroup poitner
from struct page. This patch adds an interface as
struct page_cgroup *lookup_page_cgroup(struct page*)
All FLATMEM/DISCONTIGMEM/SPARSEMEM and MEMORY_HOTPLUG is supported.
Remove page_cgroup pointer reduces the amount of memory by
- 4 bytes per PAGE_SIZE.
- 8 bytes per PAGE_SIZE
if memory controller is disabled. (even if configured.)
On usual 8GB x86-32 server, this saves 8MB of NORMAL_ZONE memory.
On my x86-64 server with 48GB of memory, this saves 96MB of memory.
I think this reduction makes sense.
By pre-allocation, kmalloc/kfree in charge/uncharge are removed.
This means
- we're not necessary to be afraid of kmalloc faiulre.
(this can happen because of gfp_mask type.)
- we can avoid calling kmalloc/kfree.
- we can avoid allocating tons of small objects which can be fragmented.
- we can know what amount of memory will be used for this extra-lru handling.
I added printk message as
"allocated %ld bytes of page_cgroup"
"please try cgroup_disable=memory option if you don't want"
maybe enough informative for users.
Changelog: v5 -> v6.
* reflected comments.
* coding style fixes.
* removed "ctype" from uncharge.
* improved comment to show FLAT_NODE_MEM_MAP == !SPARSEMEM
* fixed errors in !SPARSEMEM codes
* removed unused function in !SPARSEMEM codes.
(start from v5 because of series..)
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
include/linux/memcontrol.h | 11 -
include/linux/mm_types.h | 4
include/linux/mmzone.h | 14 ++
include/linux/page_cgroup.h | 90 ++++++++++++++++
mm/Makefile | 2
mm/memcontrol.c | 246 ++++++++++++++------------------------------
mm/page_alloc.c | 12 --
mm/page_cgroup.c | 237 ++++++++++++++++++++++++++++++++++++++++++
8 files changed, 424 insertions(+), 192 deletions(-)
Index: ...[3-6/6] cannot be applied because of HUNK.
This is fixed one for 2/6.
only for test. I'll have to post this series again, anyway.
=
Allocate all page_cgroup at boot and remove page_cgroup poitner
from struct page. This patch adds an interface as
struct page_cgroup *lookup_page_cgroup(struct page*)
All FLATMEM/DISCONTIGMEM/SPARSEMEM and MEMORY_HOTPLUG is supported.
Remove page_cgroup pointer reduces the amount of memory by
- 4 bytes per PAGE_SIZE.
- 8 bytes per PAGE_SIZE
if memory controller is disabled. (even if configured.)
On usual 8GB x86-32 server, this saves 8MB of NORMAL_ZONE memory.
On my x86-64 server with 48GB of memory, this saves 96MB of memory.
I think this reduction makes sense.
By pre-allocation, kmalloc/kfree in charge/uncharge are removed.
This means
- we're not necessary to be afraid of kmalloc faiulre.
(this can happen because of gfp_mask type.)
- we can avoid calling kmalloc/kfree.
- we can avoid allocating tons of small objects which can be fragmented.
- we can know what amount of memory will be used for this extra-lru handling.
I added printk message as
"allocated %ld bytes of page_cgroup"
"please try cgroup_disable=memory option if you don't want"
maybe enough informative for users.
Changelog: v5 -> v6.
* reflected comments.
* coding style fixes.
* removed "ctype" from uncharge.
* improved comment to show FLAT_NODE_MEM_MAP == !SPARSEMEM
* fixed errors in !SPARSEMEM codes
* removed unused function in !SPARSEMEM codes.
(start from v5 because of series..)
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
include/linux/memcontrol.h | 11 -
include/linux/mm_types.h | 4
include/linux/mmzone.h | 14 ++
include/linux/page_cgroup.h | 90 ++++++++++++++++
mm/Makefile | 2
mm/memcontrol.c | 246 ++++++++++++++------------------------------
mm/page_alloc.c | 12 --
mm/page_cgroup.c | 237 ...Just FYI, this hunk fails for mmotm Oct 2nd, applied on top of 2.6.27-rc8, the space after the #endif is gone -- Balbir --
I like this patch very much Reviewed-by: Balbir Singh <balbir@linux.vnet.ibm.com> -- Balbir --
There is a small race in do_swap_page(). When the page swapped-in is charged,
the mapcount can be greater than 0. But, at the same time some process (shares
it ) call unmap and make mapcount 1->0 and the page is uncharged.
CPUA CPUB
mapcount == 1.
(1) charge if mapcount==0 zap_pte_range()
(2) mapcount 1 => 0.
(3) uncharge(). (success)
(4) set page'rmap()
mapcoint 0=>1
Then, this swap page's account is leaked.
For fixing this, I added a new interface.
- precharge
account to res_counter by PAGE_SIZE and try to free pages if necessary.
- commit
register page_cgroup and add to LRU if necessary.
- cancel
uncharge PAGE_SIZE because of do_swap_page failure.
CPUA
(1) charge (always)
(2) set page's rmap (mapcount > 0)
(3) commit charge was necessary or not after set_pte().
This protocol uses PCG_USED bit on page_cgroup for avoiding over accounting.
Usual mem_cgroup_charge_common() does precharge -> commit at a time.
And this patch also adds following function to clarify all charges.
- mem_cgroup_newpage_charge() ....replacement for mem_cgroup_charge()
called against newly allocated anon pages.
- mem_cgroup_charge_migrate_fixup()
called only from remove_migration_ptes().
we'll have to rewrite this later.(this patch just keeps old behavior)
Good for clarify "what we does"
Then, we have 4 following charge points.
- newpage
- swapin
- add-to-cache.
- migration.
precharge/commit/cancel can be used for other places,
- shmem, (and other places need precharge.)
- move_account(force_empty) etc...
we'll revisit later.
Changelog v5 -> v6:
- added newpage_charge() and migrate_fixup().
- renamed functions for swap-in from "swap" to "swapin"
- add more precise description.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
include/linux/memcontrol.h | 35 +++++++++-
mm/memcontrol.c | ...Hmm, this patch cannot be applied because of this part.
After [2/6], mem_cgroup_charge_common looks like:
---
if (!nr_retries--) {
mem_cgroup_out_of_memory(mem, gfp_mask);
goto out;
}
}
lock_page_cgroup(pc);
if (unlikely(PageCgroupUsed(pc))) {
unlock_page_cgroup(pc);
res_counter_uncharge(&mem->res, PAGE_SIZE);
css_put(&mem->css);
goto done;
}
pc->mem_cgroup = mem;
/*
---
There is an empty line after lock_page_cgroup.
After removing this line, I can appliy this patch(and [4-6/6]).
Thanks,
Daisuke Nishimura.
--
Ah, sorry. maybe refresh miss.. I'll check it again tomorrow. Thanks, --
I don't have any objection to this direction now, but I have one quiestion. Does mem_cgroup_charge_migrate_fixup need to charge a newpage, while mem_cgroup_prepare_migration has charged it already? I agree adding I/F would be good for future, but I think mem_cgroup_charge_migration_fixup can be no-op function for now. Thanks, --
Hmm, handling failure case in explicit way may be better. Ok, I'll try some. Thanks, -Kame --
On Sat, 4 Oct 2008 00:15:17 +0900 (JST) Ah... you are right. Sorry for noise. --
On Wed, 1 Oct 2008 16:57:34 +0900
Not tested yet but an idea for handling page migraion in better way.
How do you think ?
Now, management of "charge" under page migration is done under following
manner. (Assume migrate page contents from oldapge to newpage)
Before
- "newpage" is charged before migration.
at success.
- "oldpage" is uncharged at somewhere(unmap, radix-tree-replace)
at failure
- "newpage" is uncharged.
- "oldpage" is charged if necessary (*1)
But (*1) is not trustable....because of GFP_ATOMIC and we need
special handler.
This patch tries to change behavior as following by
pre-charge/commit/cancel ops.
Before
- charge PAGE_SIZE (no target page)
success
- commit charge against "newpage".
failure
- commit charge against "oldpage".
(PCG_USED bit works effectively to avoid double-counting)
- if "oldpage" is obsolete, cancel charge of PAGE_SIZE.
I think this is much simpler and trustable.
This is based on "allocate all page_cgroup at boot" behavior.
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
include/linux/memcontrol.h | 19 ++++-------
mm/memcontrol.c | 72 +++++++++++++++++++++++++++++----------------
mm/migrate.c | 42 ++++++++------------------
3 files changed, 68 insertions(+), 65 deletions(-)
Index: mmotm-2.6.27-rc8+/mm/migrate.c
===================================================================
--- mmotm-2.6.27-rc8+.orig/mm/migrate.c
+++ mmotm-2.6.27-rc8+/mm/migrate.c
@@ -121,20 +121,6 @@ static void remove_migration_pte(struct
if (!is_migration_entry(entry) || migration_entry_to_page(entry) != old)
goto out;
- /*
- * Yes, ignore the return value from a GFP_ATOMIC mem_cgroup_charge.
- * Failure is not an option here: we're now expected to remove every
- * migration pte, and will cause crashes otherwise. Normally this
- * is not an issue: mem_cgroup_prepare_migration bumped up the old
- * page_cgroup count for safety, that's now ...This patch provides a function to move account information of a page between
mem_cgroups and rewrite force_empty to make use of this.
This moving of page_cgroup is done under
- lru_lock of source/destination mem_cgroup is held.
- lock_page_cgroup() is held.
Then, a routine which touches pc->mem_cgroup without lock_page_cgroup() should
confirm pc->mem_cgroup is still valid or not. Typlical code can be following.
(while page is not under lock_page())
mem = pc->mem_cgroup;
mz = page_cgroup_zoneinfo(pc)
spin_lock_irqsave(&mz->lru_lock);
if (pc->mem_cgroup == mem)
...../* some list handling */
spin_unlock_irq(&mz->lru_lock);
Of course, better way is
lock_page_cgroup(pc);
....
unlock_page_cgroup(pc);
But you should confirm the nest of lock and avoid deadlock.
If you treats page_cgroup from mem_cgroup's LRU under mz->lru_lock,
you don't have to worry about what pc->mem_cgroup points to.
moved pages are added to head of lru, not to tail.
Expected users of this routine is:
- force_empty (rmdir)
- moving tasks between cgroup (for moving account information.)
- hierarchy (maybe useful.)
force_empty(rmdir) uses this move_account and move pages to its parent.
This "move" will not cause OOM (I added "oom" parameter to try_charge().)
If the parent is busy (not enough memory), force_empty calls try_to_free_page()
and reduce usage.
Purpose of this behavior is
- Fix "forget all" behavior of force_empty and avoid leak of accounting.
- By "moving first, free if necessary", keep pages on memory as much as
possible.
Adding a switch to change behavior of force_empty to
- free first, move if necessary
- free all, if there is mlocked/busy pages, return -EBUSY.
is under consideration.
Changelog: (v5) -> (v6)
- removed unnecessary check.
- do all under lock_page_cgroup().
- removed res_counter_charge() from move function itself.
(and modifies try_charge() function.)
- add argument to add_list() to specify to add page_cgroup head ...Bad kernel-doc format. Use (e.g.) * @pc: page_cgroup of the page maybe: --- ~Randy --
Thank yoy for review! On Wed, 1 Oct 2008 09:38:04 -0700 It's unstable state. If someone frees some page, it's not full. But "seems" is not good, I'll remove "seems". Thanks, -Kame --
Free page_cgroup from its LRU in batched manner.
When uncharge() is called, page is pushed onto per-cpu vector and
removed from LRU, later.. This routine resembles to global LRU's pagevec.
This patch is half of the whole patch and a set with following lazy LRU add
patch.
After this, a pc, which is PageCgroupLRU(pc)==true, is on LRU.
This LRU bit is guarded by lru_lock().
PageCgroupUsed(pc) && PageCgroupLRU(pc) means "pc" is used and on LRU.
This check makes sense only when both 2 locks, lock_page_cgroup()/lru_lock(),
are aquired.
PageCgroupUsed(pc) && !PageCgroupLRU(pc) means "pc" is used but not on LRU.
!PageCgroupUsed(pc) && PageCgroupLRU(pc) means "pc" is unused but still on
LRU. lru walk routine should avoid touching this.
Changelog (v5) => (v6):
- Fixing race and added PCG_LRU bit
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
include/linux/page_cgroup.h | 5 +
mm/memcontrol.c | 201 ++++++++++++++++++++++++++++++++++++++++----
2 files changed, 189 insertions(+), 17 deletions(-)
Index: mmotm-2.6.27-rc7+/mm/memcontrol.c
===================================================================
--- mmotm-2.6.27-rc7+.orig/mm/memcontrol.c
+++ mmotm-2.6.27-rc7+/mm/memcontrol.c
@@ -34,6 +34,7 @@
#include <linux/vmalloc.h>
#include <linux/mm_inline.h>
#include <linux/page_cgroup.h>
+#include <linux/cpu.h>
#include <asm/uaccess.h>
@@ -344,7 +345,7 @@ void mem_cgroup_move_lists(struct page *
pc = lookup_page_cgroup(page);
if (!trylock_page_cgroup(pc))
return;
- if (pc && PageCgroupUsed(pc)) {
+ if (pc && PageCgroupUsed(pc) && PageCgroupLRU(pc)) {
mz = page_cgroup_zoneinfo(pc);
spin_lock_irqsave(&mz->lru_lock, flags);
__mem_cgroup_move_lists(pc, lru);
@@ -470,6 +471,122 @@ unsigned long mem_cgroup_isolate_pages(u
return nr_taken;
}
+
+#define MEMCG_PCPVEC_SIZE (14) /* size of pagevec */
+struct memcg_percpu_vec {
+ int nr;
+ int limit;
+ struct page_cgroup ...I'm wondering if page_cgroup_zoneinfo is safe without lock_page_cgroup because it dereferences pc->mem_cgroup. I'm worring if the pc has been moved to another lru by re-charge(and re-uncharge), and __mem_cgroup_remove_list toches a wrong(old) group. Hmm, there are many things to be done for re-charge and re-uncharge, so "if (!PageCgroupUsed(pc) && PageCgroupLRU(pc))" would be enough. (it can avoid race between re-charge.) Another user of page_cgroup_zoneinfo without lock_page_cgroup is __mem_cgroup_move_lists called by mem_cgroup_isolate_pages, but mem_cgroup_isolate_pages handles pc which is actually on the mz->lru so it would be ok. (I think adding VM_BUG_ON(mz != page_cgroup_zoneifno(pc)) would make sense, or add new arg *mz to __mem_cgroup_move_lists?) Thanks, Daisuke Nishimura. --
On Thu, 9 Oct 2008 14:39:49 +0900
It's safe just because I added following check.
+ /*
+ * This page_cgroup is not used but may be on LRU.
+ */
+ if (unlikely(PageCgroupLRU(pc))) {
+ /*
+ * pc->mem_cgroup has old information. force_empty() guarantee
+ * that we never see stale mem_cgroup here.
+ */
+ mz = page_cgroup_zoneinfo(pc);
+ spin_lock_irqsave(&mz->lru_lock, flags);
+ if (PageCgroupLRU(pc)) {
+ ClearPageCgroupLRU(pc);
+ __mem_cgroup_remove_list(mz, pc);
+ css_put(&pc->mem_cgroup->css);
+ }
+ spin_unlock_irqrestore(&mz->lru_lock, flags);
+ }
+ /* Here, PCG_LRU bit is cleared */
ok, I'll add VM_BUG_ON().
Thanks,
-Kame
--
Delaying add_to_lru() and do it in batched manner like page_vec. For doing that 2 flags PCG_USED and PCG_LRU. Because __set_page_cgroup_lru() itself doesn't take lock_page_cgroup(), we need a sanity check inside lru_lock(). And this delaying make css_put()/get() complicated. To make it clear, * css_get() is called from mem_cgroup_add_list(). * css_put() is called from mem_cgroup_remove_list(). * css_get()->css_put() is called while try_charge()->commit/cancel sequence. is newly added. Changelog: v5 -> v6. - css_get()/put comes back again...it's called via add_list(), remove_list(). - patch for PCG_LRU bit part is moved to release_page_cgroup_lru() patch. - Avoid TestSet and just use lock_page_cgroup() etc. - fixed race condition we saw in v5. (smp_wmb() and USED bit magic help us) Changelog: v3 -> v5. - removed css_get/put per page_cgroup struct. Now, *new* force_empty checks there is page_cgroup on the memcg. We don't need to be afraid of leak. Changelog: v2 -> v3 - added TRANSIT flag and removed lock from core logic. Changelog: v1 -> v2: - renamed function name from use_page_cgroup to set_page_cgroup_lru(). Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> mm/memcontrol.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 73 insertions(+), 10 deletions(-) Index: mmotm-2.6.27-rc7+/mm/memcontrol.c =================================================================== --- mmotm-2.6.27-rc7+.orig/mm/memcontrol.c +++ mmotm-2.6.27-rc7+/mm/memcontrol.c @@ -255,6 +255,7 @@ static void __mem_cgroup_remove_list(str mem_cgroup_charge_statistics(pc->mem_cgroup, pc, false); list_del(&pc->lru); + css_put(&pc->mem_cgroup->css); } static void __mem_cgroup_add_list(struct mem_cgroup_per_zone *mz, @@ -278,6 +279,7 @@ static void __mem_cgroup_add_list(struct list_add_tail(&pc->lru, &mz->lists[lru]); mem_cgroup_charge_statistics(pc->mem_cgroup, pc, true); + css_get(&pc->mem_cgroup->css); } ...
===
if (PageCgroupLRU(pc)) {
ClearPageCgroupLRU(pc);
__mem_cgroup_remove_list(mz, pc);
css_put(&pc->mem_cgroup->css);
}
spin_unlock_irqrestore(&mz->lru_lock, flags);
}
===
Thanks,
Daisuke Nishimura.
--
On Thu, 9 Oct 2008 15:21:32 +0900 Oh, nice catch. it's unnecessary. I'll fix this in the next. Thank you for review. I'll post still-under-discuss set (v7), tomorrow. includes - charge/commit/cancel - move account & force_empty - lazy lru free - lazy lru add Currently works well under my test.. In the next week, I'd like to restart Mem+Swap series. Thanks, -Kame --
On Wed, 1 Oct 2008 16:52:33 +0900 No terrible bugs until now on my test. My current idea for next week is following. (I may have to wait until the end of next merge window. If so, I'll wait and maintain this set.) - post ready-to-go set again. - post 1/6 and 2/6 as may-ready-to-go set. I don't chagnge order of these. - reflects comments for 3/6. patch 3/6 adds new functions. So, please tell me if you have better idea about new functions. - check logic for 4/6. - 5/6 and 6/6 may need some more comments in codes. - no new additional ones. Thanks, -Kame --
Kamezawa-San, Andrew, I think patches 1 and 2 are ready to go. Andrew they remove the cgroup member from struct page and will help reduce the overhead for distros that care about 32 bit systems and also help with performance (in my runs so far). I would recommend pushing 1 and 2 right away to -mm followed by the other performance improvements. Comments? -- Balbir --
On Mon, 06 Oct 2008 22:56:20 +0530 I'll rebase ready-to-go 4 patches and this 1 and 2 onto the latest mmotm and send again. Thank you for review. Regards, -- KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> --
