All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Frederic Weisbecker <fweisbec-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	LKML <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Subject: Re: Is not locking task_lock in cgroup_fork() safe?
Date: Thu, 18 Oct 2012 13:07:05 -0700	[thread overview]
Message-ID: <20121018200705.GG13370@google.com> (raw)
In-Reply-To: <CAFTL4hzo_w7HTgC9ApTk113X8WdZSpV+D+VSEe=604YEJFmKsg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

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

WARNING: multiple messages have this Message-ID (diff)
From: Tejun Heo <tj@kernel.org>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Li Zefan <lizefan@huawei.com>,
	containers@lists.linux-foundation.org, cgroups@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: Is not locking task_lock in cgroup_fork() safe?
Date: Thu, 18 Oct 2012 13:07:05 -0700	[thread overview]
Message-ID: <20121018200705.GG13370@google.com> (raw)
In-Reply-To: <CAFTL4hzo_w7HTgC9ApTk113X8WdZSpV+D+VSEe=604YEJFmKsg@mail.gmail.com>

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

  parent reply	other threads:[~2012-10-18 20:07 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-08  2:00 Is not locking task_lock in cgroup_fork() safe? Tejun Heo
2012-10-08  2:01 ` Tejun Heo
2012-10-08  5:46   ` Li Zefan
     [not found]     ` <507268AA.8050509-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2012-10-08  6:57       ` Tejun Heo
2012-10-16 19:34         ` Tejun Heo
     [not found]           ` <20121016193428.GE16166-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-10-17  7:26             ` Li Zefan
2012-10-17  7:26             ` Li Zefan
2012-10-08  6:57       ` Tejun Heo
2012-10-08  5:46   ` Li Zefan
2012-10-08 12:58   ` Frederic Weisbecker
2012-10-08 12:58   ` Frederic Weisbecker
2012-10-08 12:48 ` Frederic Weisbecker
     [not found]   ` <CAFTL4hzXWtzp7megsCAEuak5=_2SWmp9age-+wrpyQAU4BRZ0w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-10-16 19:33     ` Tejun Heo
     [not found]       ` <20121016193341.GD16166-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-10-18 14:50         ` Frederic Weisbecker
2012-10-18 14:50         ` Frederic Weisbecker
2012-10-18 14:50           ` Frederic Weisbecker
     [not found]           ` <CAFTL4hzo_w7HTgC9ApTk113X8WdZSpV+D+VSEe=604YEJFmKsg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-10-18 20:07             ` Tejun Heo [this message]
2012-10-18 20:07               ` Tejun Heo
     [not found]               ` <20121018200705.GG13370-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-10-18 20:53                 ` Frederic Weisbecker
2012-10-18 20:53                   ` Frederic Weisbecker
     [not found]                   ` <CAFTL4hy7g4e11OUOyoihrEU8hiVgZoV1=141UtUpj9a72SNs_Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-10-19  0:38                     ` Tejun Heo
2012-10-19  0:38                       ` Tejun Heo
     [not found]                       ` <20121019003835.GE13370-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-10-19  0:58                         ` Tejun Heo
2012-10-19  0:58                           ` Tejun Heo
     [not found]                           ` <20121019005801.GF13370-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-10-19  8:50                             ` Li Zefan
2012-10-19  8:50                               ` Li Zefan
2012-10-19  8:50                             ` Li Zefan
2012-10-19  0:38                     ` Tejun Heo
2012-10-08 12:48 ` Frederic Weisbecker
2012-10-19  0:59 ` [PATCH cgroup/for-3.7-fixes 1/2] Revert "cgroup: Remove task_lock() from cgroup_post_fork()" Tejun Heo
     [not found]   ` <20121019005922.GG13370-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-10-19  8:51     ` Li Zefan
2012-10-19 13:35     ` Frederic Weisbecker
     [not found]       ` <CAFTL4hz82==b3ioSMhbKzh0CN1ivR7RQMKKMFFWu5PHPjg=Bfg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-10-19 19:38         ` Tejun Heo
     [not found]           ` <20121019193808.GL13370-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-10-19 19:44             ` Frederic Weisbecker
2012-10-19 19:44             ` Frederic Weisbecker
2012-10-19 19:44               ` Frederic Weisbecker
     [not found]               ` <CAFTL4hwQ6Ntn5GJwj=jiO2p3GdwhEMp0MyR8dgUj_Lx0U4kNqg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-10-19 21:07                 ` Tejun Heo
2012-10-19 21:07                 ` Tejun Heo
2012-10-19 21:07                   ` Tejun Heo
     [not found]                   ` <20121019210738.GA1180-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-10-20 18:21                     ` Frederic Weisbecker
2012-10-20 18:21                       ` Frederic Weisbecker
     [not found]                       ` <CAFTL4hy+vrvJKrc1Y2FW44k=LBi72H=34337xALpbtG_3u5O7w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-10-20 18:23                         ` Frederic Weisbecker
2012-10-20 18:23                           ` Frederic Weisbecker
2012-10-20 22:37                         ` Tejun Heo
2012-10-20 22:37                           ` Tejun Heo
     [not found]                           ` <20121020223709.GA5626-Gd/HAXX7CRxy/B6EtB590w@public.gmane.org>
2012-10-22  9:30                             ` Frederic Weisbecker
2012-10-22  9:30                             ` Frederic Weisbecker
2012-10-22  9:30                               ` Frederic Weisbecker
2012-10-19 19:38         ` Tejun Heo
2012-10-19 13:35     ` Frederic Weisbecker
2012-10-19  0:59 ` Tejun Heo
2012-10-19  0:59 ` [PATCH cgroup/for-3.7-fixes 2/2] Revert "cgroup: Drop task_lock(parent) on cgroup_fork()" Tejun Heo
     [not found]   ` <20121019005951.GH13370-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2012-10-19  8:51     ` Li Zefan
2012-10-19  8:51     ` Li Zefan
2012-10-19 13:45     ` Frederic Weisbecker
2012-10-19 13:45     ` Frederic Weisbecker
2012-10-19  0:59 ` Tejun Heo
  -- strict thread matches above, loose matches on Subject: below --
2012-10-08  2:00 Is not locking task_lock in cgroup_fork() safe? Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121018200705.GG13370@google.com \
    --to=tj-dgejt+ai2ygdnm+yrofe0a@public.gmane.org \
    --cc=cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=fweisbec-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.