From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tejun Heo Subject: Re: Is not locking task_lock in cgroup_fork() safe? Date: Thu, 18 Oct 2012 13:07:05 -0700 Message-ID: <20121018200705.GG13370@google.com> References: <20121008020000.GB2575@localhost> <20121016193341.GD16166@google.com> 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=3lmEO8vPi7bSYNScGJH9/fHJUpueFb9aB39qTaZsscg=; b=qEwQMkGE4n8xCr2SBifyV/K361ycXXNmVtK3NoSpMqavmSM3/bC3d2C6P7ltgGssg/ P3BNM9fw8q0z0deNW3hXx6YULPakkOEQ2+wMYZoh93OurJJyzXCK4fF6Sfz6pJyOM6j0 7YKQf9aj0ibTw35k4VQHpuTv28JEHMqQTTCOJEmJcDLN/1JhRSLSMU4ncegwDWX2sk2t wdXd5J/0jiGDMUlxUWgoBY5kkIJtDYQBdXqKBF5LbHvrlX4efIqEk8Rtaxaa3MktZ1f3 oBLEjwNZ/J44opsPZw9Mc5gWk/EoizHIkufGYc+nSuf1Eijt3eV4nlHNUwPXocafiOS+ iU1g== Content-Disposition: inline In-Reply-To: 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: Frederic Weisbecker Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, LKML Hello, Frederic. On Thu, Oct 18, 2012 at 04:50:59PM +0200, Frederic Weisbecker wrote: > Ah right I was confused. Hmm, indeed we have a race here on > cgroup_fork(). How about using css_try_get() in cgroup_fork() and > refetch the parent's css until we succeed? This requires rcu_read_lock > though, and freeing the css_set under RCU. > > Don't know which is better. For now, I'll revert the patches and cc stable. Let's think about improving it later. > Different problem but I really would like we sanitize the cgroup hooks > in fork. There is cgroup_fork(), cgroup_post_fork() which takes that > big css_set_lock, plus the big threadgroup lock... I hope we can > simplify the mess there. Oh yeah, I've been looking at that one too. There are a few problems in that area. I think all we need is clearing ->cgroups to NULL on copy_process() and all the rest can be moved to cgroup_post_fork(). I'd also like to make it very explicit that migration can't happen before post_fork is complete. > > I really don't know. Why isn't it locking the threadgroup to begin > > with? > > No idea, sounds like something to fix. Alrighty. Thanks. -- tejun