Messages in this thread | | | Date | Mon, 23 Aug 2010 12:35:33 +0900 | From | Daisuke Nishimura <> | Subject | Re: [PATCH 2/5] memcg: use array and ID for quick look up |
| |
Hi,
> +/* 0 is unused */ > +static atomic_t mem_cgroup_num; > +#define NR_MEMCG_GROUPS (CONFIG_MEM_CGROUP_MAX_GROUPS + 1) > +static struct mem_cgroup *mem_cgroups[NR_MEMCG_GROUPS] __read_mostly; > + > +/* Must be called under rcu_read_lock */ > +static struct mem_cgroup *id_to_memcg(unsigned short id) > +{ > + struct mem_cgroup *ret; > + /* see mem_cgroup_free() */ > + ret = rcu_dereference_check(mem_cgroups[id], rch_read_lock_held()); > + if (likely(ret && ret->valid)) > + return ret; > + return NULL; > +} > + I prefer "mem" to "ret".
> @@ -2231,7 +2244,7 @@ __mem_cgroup_commit_charge_swapin(struct > > id = swap_cgroup_record(ent, 0); > rcu_read_lock(); > - memcg = mem_cgroup_lookup(id); > + memcg = id_to_memcg(id); > if (memcg) { > /* > * This recorded memcg can be obsolete one. So, avoid > @@ -2240,9 +2253,10 @@ __mem_cgroup_commit_charge_swapin(struct > if (!mem_cgroup_is_root(memcg)) > res_counter_uncharge(&memcg->memsw, PAGE_SIZE); > mem_cgroup_swap_statistics(memcg, false); > + rcu_read_unlock(); > mem_cgroup_put(memcg); > - } > - rcu_read_unlock(); > + } else > + rcu_read_unlock(); > } > /* > * At swapin, we may charge account against cgroup which has no tasks. > @@ -2495,7 +2509,7 @@ void mem_cgroup_uncharge_swap(swp_entry_ > > id = swap_cgroup_record(ent, 0); > rcu_read_lock(); > - memcg = mem_cgroup_lookup(id); > + memcg = id_to_memcg(id); > if (memcg) { > /* > * We uncharge this because swap is freed. > @@ -2504,9 +2518,10 @@ void mem_cgroup_uncharge_swap(swp_entry_ > if (!mem_cgroup_is_root(memcg)) > res_counter_uncharge(&memcg->memsw, PAGE_SIZE); > mem_cgroup_swap_statistics(memcg, false); > + rcu_read_unlock(); > mem_cgroup_put(memcg); > - } > - rcu_read_unlock(); > + } else > + rcu_read_unlock(); > } > > /** Could you explain why we need rcu_read_unlock() before mem_cgroup_put() ? I suspect that it's because mem_cgroup_put() can free the memcg, but do we need mem->valid then ?
Thanks, Daisuke Nishimura.
| |