On Tue, 8 Feb 2011, KAMEZAWA Hiroyuki wrote:
> > +static int mem_cgroup_oom_delay_millisecs_write(struct cgroup *cgrp,
> > + struct cftype *cft, u64 val)
> > +{
> > + struct mem_cgroup *memcg = mem_cgroup_from_cont(cgrp);
> > + struct mem_cgroup *iter;
> > +
> > + if (val > MAX_SCHEDULE_TIMEOUT)
> > + return -EINVAL;
> > +
> > + for_each_mem_cgroup_tree(iter, memcg) {
> > + iter->oom_delay = msecs_to_jiffies(val);
> > + memcg_oom_recover(iter);
> > + }
> > + return 0;
>
> Seems nicer and it seems you tries to update all children cgroups.
>
> BTW, with above code, with following heirarchy,
>
> A
> /
> B
> /
> C
>
> When a user set oom_delay in order as A->B->C, A,B,C can have 'different' numbers.
> When a user set oom_delay in order as C->B->A, A,B,C will have the same numbers.
>
> This intreface seems magical, or broken.
>
It's not really magical, it just means that if you change the delay for a
memcg that you do so for all of its children implicitly as well.
An alternative would be to ensure that a child memcg may never have a
delay greater than the delay of its parent. Would you prefer that
instead?
> So, my recomendation is 'just allow to set value a cgroup which has no children/parent'.
> Or 'just allo to se value a cgroup which is a root of a hierarchy'.
> Could you add a check ? Inheritance at mkdir() is okay to me.
>
I'm trying to get away from this only because it doesn't seem very logical
that creating a child memcg within a parent means that the parent is now
locked out of setting memory.oom_delay_millisecs.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>