From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tejun Heo Subject: Re: [PATCH 07/12] cgroup: unify cgroup_write_X64() and cgroup_write_string() Date: Fri, 29 Nov 2013 15:05:25 -0500 Message-ID: <20131129200525.GC21755@mtj.dyndns.org> References: <1385595759-17656-1-git-send-email-tj@kernel.org> <1385595759-17656-8-git-send-email-tj@kernel.org> <20131128111818.GG2761@dhcp22.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=G8zRhZPn2x9LQUsF69AUtFNsEaBljsmrwN3P6Ht5e/4=; b=tH3xl5r8zjXU62zX1+e3i8B63AzPSea790oMS6E2V5iu6n8Soav4yPAqxe5NQFnsFt TD77yvIGDE2qD8hIoyfeo9bnTsPRN8ev51k5k2e5cEebmyp62ijutg8jZs58Wc+uuc6p miqpNeKZCAXf/mqSBt51c5M1fsHBHzHgYBcSlaxyga5pyAPeteLPfj493HnQuqwBrMyH Fn/uCbKYIugcbtM0xD19uNCCwVKDQ0qRtJpnlUc9h9nzepVQ/E9lg2HPOwK60ZijtYAC 0PXTa/LRvvKyn/v38gsRdNAYSsaii3IQ547hXhg+ShwKOiVGo62+1isSclR2d1m96LQr yfoA== Content-Disposition: inline In-Reply-To: <20131128111818.GG2761-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: containers-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Michal Hocko Cc: nhorman-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org, containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, daniel.wagner-98C5kh4wR6ohFhg+JK9F0w@public.gmane.org, hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org, cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, vgoyal-H+wXaHxf7aLQT0dZR+AlfA@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