On Thu, Nov 17, 2016 at 11:28:12AM +1100, Balbir Singh wrote:
> >> @@ -5773,6 +5771,59 @@ static int __init cgroup_memory(char *s)
> >> }
> >> __setup("cgroup.memory=", cgroup_memory);
> >>
> >> +static void memcg_node_offline(int node)
> >> +{
> >> + struct mem_cgroup *memcg;
> >> +
> >> + if (node < 0)
> >> + return;
> >
> > Is this possible?
>
> Yes, please see node_states_check_changes_online/offline
OK, I see.
>
> >
> >> +
> >> + for_each_mem_cgroup(memcg) {
> >> + free_mem_cgroup_per_node_info(memcg, node);
> >> + mem_cgroup_may_update_nodemask(memcg);
> >
> > If memcg->numainfo_events is 0, mem_cgroup_may_update_nodemask() won't
> > update memcg->scan_nodes. Is it OK?
> >
> >> + }
> >
> > What if a memory cgroup is created or destroyed while you're walking the
> > tree? Should we probably use get_online_mems() in mem_cgroup_alloc() to
> > avoid that?
> >
>
> The iterator internally takes rcu_read_lock() to avoid any side-effects
> of cgroups added/removed. I suspect you are also suggesting using get_online_mems()
> around each call to for_each_online_node
>
> My understanding so far is
>
> 1. invalidate_reclaim_iterators should be safe (no bad side-effects)
> 2. mem_cgroup_free - should be safe as well
> 3. mem_cgroup_alloc - needs protection
> 4. mem_cgroup_init - needs protection
> 5. mem_cgroup_remove_from_tress - should be safe
I'm not into the memory hotplug code, but my understanding is that if
memcg offline happens to race with node unplug, it's possible that
- mem_cgroup_free() doesn't free the node's data, because it sees the
node as already offline
- memcg hotplug code doesn't free the node's data either, because it
sees the cgroup as offline
May be, we should surround all the loops over online nodes with
get/put_online_mems() to be sure that nothing wrong can happen.
They are slow path, anyway.
Thanks,
Vladimir
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>