On Fri 29-11-13 15:05:25, Tejun Heo wrote: > 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? Well, I am not happy about the use case as well and I will always discourage people from doing this. I was merely pointing out that the patch will break those even though it seems trivial to not do so. That being said I would rather see no allocation in that path but if that doesn't seem viable to you then I will not loose any sleep over it. -- Michal Hocko SUSE Labs _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/containers
