| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 16 Apr 2009 17:15:35 +0900
From: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
To: KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
Cc: balbir@...ux.vnet.ibm.com,
"linux-mm@...ck.org" <linux-mm@...ck.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] Add file based RSS accounting for memory resource
controller (v2)
> Sorry, some troubles found. Ignore above Ack. 3points now.
>
> 1. get_cpu should be after (*)
> ==mem_cgroup_update_mapped_file_stat()
> + int cpu = get_cpu();
> +
> + if (!page_is_file_cache(page))
> + return;
> +
> + if (unlikely(!mm))
> + mm = &init_mm;
> +
> + mem = try_get_mem_cgroup_from_mm(mm);
> + if (!mem)
> + return;
> + ----------------------------------------(*)
> + stat = &mem->stat;
> + cpustat = &stat->cpustat[cpu];
> +
> + __mem_cgroup_stat_add_safe(cpustat, MEM_CGROUP_STAT_MAPPED_FILE, val);
> + put_cpu();
> +}
> ==
>
> 2. In above, "mem" shouldn't be got from "mm"....please get "mem" from page_cgroup.
> (Because it's file cache, pc->mem_cgroup is not NULL always.)
>
> I saw this very easily.
> ==
> Cache: 4096
> mapped_file: 20480
> ==
>
> 3. at force_empty().
> ==
> +
> + cpu = get_cpu();
> + /* Update mapped_file data for mem_cgroup "from" */
> + stat = &from->stat;
> + cpustat = &stat->cpustat[cpu];
> + __mem_cgroup_stat_add_safe(cpustat, MEM_CGROUP_STAT_MAPPED_FILE, -1);
> +
> + /* Update mapped_file data for mem_cgroup "to" */
> + stat = &to->stat;
> + cpustat = &stat->cpustat[cpu];
> + __mem_cgroup_stat_add_safe(cpustat, MEM_CGROUP_STAT_MAPPED_FILE, 1);
> + put_cpu();
>
> This just breaks counter when page is not mapped. please check page_mapped().
>
> like this:
> ==
> if (page_is_file_cache(page) && page_mapped(page)) {
> modify counter.
> }
> ==
>
> and call lock_page_cgroup() in mem_cgroup_update_mapped_file_stat().
>
> This will be slow, but optimization will be very tricky and need some amount of time.
>
This is my fix for above 3 problems. but plz do as you like.
I'm not very intersted in details.
==
---
Index: mmotm-2.6.30-Apr14/mm/memcontrol.c
===================================================================
--- mmotm-2.6.30-Apr14.orig/mm/memcontrol.c
+++ mmotm-2.6.30-Apr14/mm/memcontrol.c
@@ -322,33 +322,39 @@ static bool mem_cgroup_is_obsolete(struc
return css_is_removed(&mem->css);
}
-/*
- * Currently used to update mapped file statistics, but the routine can be
- * generalized to update other statistics as well.
- */
-void mem_cgroup_update_mapped_file_stat(struct page *page, struct mm_struct *mm,
- int val)
+void mem_cgroup_update_mapped_file_stat(struct page *page, bool map)
{
struct mem_cgroup *mem;
struct mem_cgroup_stat *stat;
struct mem_cgroup_stat_cpu *cpustat;
- int cpu = get_cpu();
+ struct page_cgroup *pc;
+ int cpu;
if (!page_is_file_cache(page))
return;
- if (unlikely(!mm))
- mm = &init_mm;
-
- mem = try_get_mem_cgroup_from_mm(mm);
- if (!mem)
+ pc = lookup_page_cgroup(page);
+ if (unlikely(!pc))
return;
-
- stat = &mem->stat;
- cpustat = &stat->cpustat[cpu];
-
- __mem_cgroup_stat_add_safe(cpustat, MEM_CGROUP_STAT_MAPPED_FILE, val);
- put_cpu();
+ lock_page_cgroup(pc);
+ mem = pc->mem_cgroup;
+ if (mem) {
+ cpu = get_cpu();
+ stat = &mem->stat;
+ cpustat = &stat->cpustat[cpu];
+ if (map)
+ __mem_cgroup_stat_add_safe(cpustat,
+ MEM_CGROUP_STAT_MAPPED_FILE, 1);
+ else
+ __mem_cgroup_stat_add_safe(cpustat,
+ MEM_CGROUP_STAT_MAPPED_FILE, -1);
+ put_cpu();
+ }
+ if (map)
+ SetPageCgroupMapped(pc);
+ else
+ ClearPageCgroupMapped(pc);
+ unlock_page_cgroup(pc);
}
/*
@@ -1149,17 +1155,19 @@ static int mem_cgroup_move_account(struc
res_counter_uncharge(&from->res, PAGE_SIZE);
mem_cgroup_charge_statistics(from, pc, false);
- cpu = get_cpu();
- /* Update mapped_file data for mem_cgroup "from" */
- stat = &from->stat;
- cpustat = &stat->cpustat[cpu];
- __mem_cgroup_stat_add_safe(cpustat, MEM_CGROUP_STAT_MAPPED_FILE, -1);
-
- /* Update mapped_file data for mem_cgroup "to" */
- stat = &to->stat;
- cpustat = &stat->cpustat[cpu];
- __mem_cgroup_stat_add_safe(cpustat, MEM_CGROUP_STAT_MAPPED_FILE, 1);
- put_cpu();
+ if (PageCgroupMapped(pc)) {
+ cpu = get_cpu();
+ /* Update mapped_file data for mem_cgroup "from" and "to" */
+ stat = &from->stat;
+ cpustat = &stat->cpustat[cpu];
+ __mem_cgroup_stat_add_safe(cpustat,
+ MEM_CGROUP_STAT_MAPPED_FILE, -1);
+ stat = &to->stat;
+ cpustat = &stat->cpustat[cpu];
+ __mem_cgroup_stat_add_safe(cpustat,
+ MEM_CGROUP_STAT_MAPPED_FILE, 1);
+ put_cpu();
+ }
if (do_swap_account)
res_counter_uncharge(&from->memsw, PAGE_SIZE);
Index: mmotm-2.6.30-Apr14/include/linux/page_cgroup.h
===================================================================
--- mmotm-2.6.30-Apr14.orig/include/linux/page_cgroup.h
+++ mmotm-2.6.30-Apr14/include/linux/page_cgroup.h
@@ -26,6 +26,7 @@ enum {
PCG_LOCK, /* page cgroup is locked */
PCG_CACHE, /* charged as cache */
PCG_USED, /* this object is in use. */
+ PCG_MAPPED, /* mapped file cache */
};
#define TESTPCGFLAG(uname, lname) \
@@ -46,6 +47,10 @@ TESTPCGFLAG(Cache, CACHE)
TESTPCGFLAG(Used, USED)
CLEARPCGFLAG(Used, USED)
+TESTPCGFLAG(Mapped, USED)
+CLEARPCGFLAG(Mapped, USED)
+SETPCGFLAG(Mapped, USED)
+
static inline int page_cgroup_nid(struct page_cgroup *pc)
{
return page_to_nid(pc->page);
Index: mmotm-2.6.30-Apr14/include/linux/memcontrol.h
===================================================================
--- mmotm-2.6.30-Apr14.orig/include/linux/memcontrol.h
+++ mmotm-2.6.30-Apr14/include/linux/memcontrol.h
@@ -116,8 +116,7 @@ static inline bool mem_cgroup_disabled(v
}
extern bool mem_cgroup_oom_called(struct task_struct *task);
-void mem_cgroup_update_mapped_file_stat(struct page *page, struct mm_struct *mm,
- int val);
+void mem_cgroup_update_mapped_file_stat(struct page *page, bool map);
#else /* CONFIG_CGROUP_MEM_RES_CTLR */
struct mem_cgroup;
@@ -266,8 +265,7 @@ mem_cgroup_print_oom_info(struct mem_cgr
}
static inline void mem_cgroup_update_mapped_file_stat(struct page *page,
- struct mm_struct *mm,
- int val)
+ bool map)
{
}
Index: mmotm-2.6.30-Apr14/mm/rmap.c
===================================================================
--- mmotm-2.6.30-Apr14.orig/mm/rmap.c
+++ mmotm-2.6.30-Apr14/mm/rmap.c
@@ -690,7 +690,7 @@ void page_add_file_rmap(struct page *pag
{
if (atomic_inc_and_test(&page->_mapcount)) {
__inc_zone_page_state(page, NR_FILE_MAPPED);
- mem_cgroup_update_mapped_file_stat(page, vma->vm_mm, 1);
+ mem_cgroup_update_mapped_file_stat(page, true);
}
}
@@ -740,7 +740,7 @@ void page_remove_rmap(struct page *page,
mem_cgroup_uncharge_page(page);
__dec_zone_page_state(page,
PageAnon(page) ? NR_ANON_PAGES : NR_FILE_MAPPED);
- mem_cgroup_update_mapped_file_stat(page, vma->vm_mm, -1);
+ mem_cgroup_update_mapped_file_stat(page, false);
/*
* It would be tidy to reset the PageAnon mapping here,
* but that might overwrite a racing page_add_anon_rmap
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists