Re: [PATCH 07/12] cgroup: unify cgroup_write_X64() and cgroup_write_string()

[Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

Hello, Michal.

On Thu, Nov 28, 2013 at 12:18:18PM +0100, Michal Hocko wrote:
> > Unify the two into cgroup_file_write() which always allocates dynamic
> > buffer for simplicity
> 
> It's true that this is simpler but the allocation might cause some
> issues with memcg. Although it is not common that userspace oom handler
> is a part of the target memcg there are users (e.g. Google) who do that.
> 
> Why is that a problem? Consider that a memcg is under OOM, handler gets
> notified and tries to solve the situation by enlarging the hard limit.
> This worked before this patch because cgroup_write_X64 used an on stack
> buffer but now it would use kmalloc which might be accounted and trip
> over the same OOM situation.
> 
> This is not limited to the OOM handling. The group might be close to OOM
> and the last thing user expects is to trigger OOM when he tries to
> enlarge the limit.
> 
> Is the additional simplicity worth breaking those usecases?

Whoa, so we support oom handler inside the memcg that it handles?
Does that work reliably?  Changing the above detail in this patch
isn't difficult (and we'll later need to update kernfs too) but
supporting such setup properly would be a *lot* of commitment and I'm
very doubtful we'd be able to achieve that by just carefully avoiding
memory allocation in the operations that usreland oom handler uses -
that set is destined to expand over time, extremely fragile and will
be hellish to maintain.

So, I'm not at all excited about commiting to this guarantee.  This
one is an easy one but it looks like the first step onto dizzying
slippery slope.

Am I misunderstanding something here?  Are you and Johannes firm on
supporting this?

Thanks.

-- 
tejun