Messages in this thread | | | Date | Fri, 5 Mar 2010 12:31:33 +0530 | From | Balbir Singh <> | Subject | Re: [PATCH -mmotm 3/4] memcg: dirty pages accounting and limiting infrastructure |
| |
* KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2010-03-05 10:58:55]:
> On Fri, 5 Mar 2010 10:12:34 +0900 > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote: > > > On Thu, 4 Mar 2010 11:40:14 +0100, Andrea Righi <arighi@develer.com> wrote: > > > Infrastructure to account dirty pages per cgroup and add dirty limit > > > static int mem_cgroup_count_children_cb(struct mem_cgroup *mem, void *data) > > > { > > > int *val = data; > > > @@ -1275,34 +1423,70 @@ static void record_last_oom(struct mem_cgroup *mem) > > > } > > > > > > /* > > > - * Currently used to update mapped file statistics, but the routine can be > > > - * generalized to update other statistics as well. > > > + * Generalized routine to update file cache's status for memcg. > > > + * > > > + * Before calling this, mapping->tree_lock should be held and preemption is > > > + * disabled. Then, it's guarnteed that the page is not uncharged while we > > > + * access page_cgroup. We can make use of that. > > > */ > > IIUC, mapping->tree_lock is held with irq disabled, so I think "mapping->tree_lock > > should be held with irq disabled" would be enouth. > > And, as far as I can see, callers of this function have not ensured this yet in [4/4]. > > > > how about: > > > > void mem_cgroup_update_stat_locked(...) > > { > > ... > > } > > > > void mem_cgroup_update_stat_unlocked(mapping, ...) > > { > > spin_lock_irqsave(mapping->tree_lock, ...); > > mem_cgroup_update_stat_locked(); > > spin_unlock_irqrestore(...); > > } > > > Rather than tree_lock, lock_page_cgroup() can be used if tree_lock is not held. > > lock_page_cgroup(); > mem_cgroup_update_stat_locked(); > unlock_page_cgroup(); > > Andrea-san, FILE_MAPPED is updated without treelock, at least. You can't depend > on migration_lock about FILE_MAPPED. >
FILE_MAPPED is updated under pte lock in the rmap context and page_cgroup lock within update_file_mapped.
-- Three Cheers, Balbir
| |