From: Mandeep Singh Baines <msb-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> To: Frederic Weisbecker <fweisbec-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Cc: Containers <containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, LKML <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, Paul Menage <paul-inf54ven1CmVyaH7bEyXVA@public.gmane.org>, Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Cgroups <cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>, "Paul E. McKenney" <paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>, Mandeep Singh Baines <msb-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> Subject: Re: Q: cgroup: Questions about possible issues in cgroup locking Date: Wed, 21 Dec 2011 11:24:13 -0800 [thread overview] Message-ID: <20111221192413.GF13529@google.com> (raw) In-Reply-To: <20111221190817.GI17668@somewhere> Frederic Weisbecker (fweisbec-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote: > On Wed, Dec 21, 2011 at 11:01:02AM -0800, Mandeep Singh Baines wrote: > > Frederic Weisbecker (fweisbec-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org) wrote: > > > On Wed, Dec 21, 2011 at 02:08:48PM +0100, Oleg Nesterov wrote: > > > > On 12/21, Frederic Weisbecker wrote: > > > > > Hi, > > > > > > > > > > Starring at some parts of cgroups, I have a few questions: > > > > > > > > > > - Is cgroup_enable_task_cg_list()'s while_each_thread() safe > > > > > against concurrent exec()? The leader may change in de_thread() > > > > > and invalidate the test done in while_each_thread(). > > > > > > > > Yes. Oh, we need to do something with while_each_thread. > > > > > > Would something like this work? > > > > > > diff --git a/include/linux/sched.h b/include/linux/sched.h > > > index c0c5876..e002a00 100644 > > > --- a/include/linux/sched.h > > > +++ b/include/linux/sched.h > > > @@ -2293,8 +2293,12 @@ extern bool current_is_single_threaded(void); > > > #define do_each_thread(g, t) \ > > > for (g = t = &init_task ; (g = t = next_task(g)) != &init_task ; ) do > > > > > > -#define while_each_thread(g, t) \ > > > - while ((t = next_thread(t)) != g) > > > +#define while_each_thread(g, t) \ > > > + while (({ \ > > > + struct task_struct *__prev = t; \ > > > + t = next_thread(t); \ > > > + t != __prev && t != g; \ > > > > Hi, > > > > Don't you still have an (highly unlikely) race if you exec > > and then pthread_create()? > > I'm not sure what you mean. Here is what I'm thinking: If you call exec from a thread other than g, g is now unlinked. So "t != g" will always be true. If you then pthread_create, you now have two threads so "t != __prev" will also always be true. So you now have an infinite loop. > > > > > Instead of: > > > > t != __prev && t != g; > > > > How about: > > > > t != t->group_leader; > > That might work too but we need a pair of memory barriers. next_thread() calls list_entry_rcu. Shouldn't that protect against a dereference? You don't need to synchronize group_leader since you are only using it as a value. You don't dereference it. Regards, Mandeep
WARNING: multiple messages have this Message-ID (diff)
From: Mandeep Singh Baines <msb@chromium.org> To: Frederic Weisbecker <fweisbec@gmail.com> Cc: Mandeep Singh Baines <msb@chromium.org>, Oleg Nesterov <oleg@redhat.com>, Li Zefan <lizf@cn.fujitsu.com>, Tejun Heo <tj@kernel.org>, LKML <linux-kernel@vger.kernel.org>, Containers <containers@lists.linux-foundation.org>, Cgroups <cgroups@vger.kernel.org>, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>, Paul Menage <paul@paulmenage.org>, Andrew Morton <akpm@linux-foundation.org>, "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> Subject: Re: Q: cgroup: Questions about possible issues in cgroup locking Date: Wed, 21 Dec 2011 11:24:13 -0800 [thread overview] Message-ID: <20111221192413.GF13529@google.com> (raw) In-Reply-To: <20111221190817.GI17668@somewhere> Frederic Weisbecker (fweisbec@gmail.com) wrote: > On Wed, Dec 21, 2011 at 11:01:02AM -0800, Mandeep Singh Baines wrote: > > Frederic Weisbecker (fweisbec@gmail.com) wrote: > > > On Wed, Dec 21, 2011 at 02:08:48PM +0100, Oleg Nesterov wrote: > > > > On 12/21, Frederic Weisbecker wrote: > > > > > Hi, > > > > > > > > > > Starring at some parts of cgroups, I have a few questions: > > > > > > > > > > - Is cgroup_enable_task_cg_list()'s while_each_thread() safe > > > > > against concurrent exec()? The leader may change in de_thread() > > > > > and invalidate the test done in while_each_thread(). > > > > > > > > Yes. Oh, we need to do something with while_each_thread. > > > > > > Would something like this work? > > > > > > diff --git a/include/linux/sched.h b/include/linux/sched.h > > > index c0c5876..e002a00 100644 > > > --- a/include/linux/sched.h > > > +++ b/include/linux/sched.h > > > @@ -2293,8 +2293,12 @@ extern bool current_is_single_threaded(void); > > > #define do_each_thread(g, t) \ > > > for (g = t = &init_task ; (g = t = next_task(g)) != &init_task ; ) do > > > > > > -#define while_each_thread(g, t) \ > > > - while ((t = next_thread(t)) != g) > > > +#define while_each_thread(g, t) \ > > > + while (({ \ > > > + struct task_struct *__prev = t; \ > > > + t = next_thread(t); \ > > > + t != __prev && t != g; \ > > > > Hi, > > > > Don't you still have an (highly unlikely) race if you exec > > and then pthread_create()? > > I'm not sure what you mean. Here is what I'm thinking: If you call exec from a thread other than g, g is now unlinked. So "t != g" will always be true. If you then pthread_create, you now have two threads so "t != __prev" will also always be true. So you now have an infinite loop. > > > > > Instead of: > > > > t != __prev && t != g; > > > > How about: > > > > t != t->group_leader; > > That might work too but we need a pair of memory barriers. next_thread() calls list_entry_rcu. Shouldn't that protect against a dereference? You don't need to synchronize group_leader since you are only using it as a value. You don't dereference it. Regards, Mandeep
next prev parent reply other threads:[~2011-12-21 19:24 UTC|newest] Thread overview: 73+ messages / expand[flat|nested] mbox.gz Atom feed top 2011-12-21 3:43 Q: cgroup: Questions about possible issues in cgroup locking Frederic Weisbecker 2011-12-21 3:43 ` Frederic Weisbecker 2011-12-21 13:08 ` Oleg Nesterov 2011-12-21 13:08 ` Oleg Nesterov 2011-12-21 17:56 ` Frederic Weisbecker 2011-12-21 17:56 ` Frederic Weisbecker 2011-12-21 19:01 ` Mandeep Singh Baines 2011-12-21 19:01 ` Mandeep Singh Baines [not found] ` <20111221190102.GE13529-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 2011-12-21 19:08 ` Frederic Weisbecker 2011-12-21 19:08 ` Frederic Weisbecker 2011-12-21 19:24 ` Mandeep Singh Baines [this message] 2011-12-21 19:24 ` Mandeep Singh Baines [not found] ` <20111221192413.GF13529-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 2011-12-21 20:04 ` Frederic Weisbecker 2011-12-21 20:04 ` Frederic Weisbecker 2011-12-21 20:04 ` Frederic Weisbecker 2011-12-22 15:30 ` Oleg Nesterov 2011-12-22 15:30 ` Oleg Nesterov [not found] ` <20111222153004.GA30522-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2012-01-04 19:36 ` Mandeep Singh Baines 2012-01-04 19:36 ` Mandeep Singh Baines 2012-01-04 19:36 ` Mandeep Singh Baines [not found] ` <20120104193614.GF9511-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 2012-01-06 15:23 ` Oleg Nesterov 2012-01-06 15:23 ` Oleg Nesterov [not found] ` <20120106152356.GA23995-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2012-01-06 18:25 ` Mandeep Singh Baines 2012-01-06 18:25 ` Mandeep Singh Baines 2012-01-06 18:25 ` Mandeep Singh Baines [not found] ` <20120106182535.GJ9511-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 2012-01-11 16:07 ` Oleg Nesterov 2012-01-11 16:07 ` Oleg Nesterov [not found] ` <20120111160730.GA24556-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2012-01-12 0:31 ` Mandeep Singh Baines 2012-01-12 0:31 ` Mandeep Singh Baines [not found] ` <20120112003102.GB9511-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 2012-01-12 17:07 ` Oleg Nesterov 2012-01-12 17:07 ` Oleg Nesterov 2012-01-12 17:57 ` Mandeep Singh Baines 2012-01-12 17:57 ` Mandeep Singh Baines [not found] ` <20120112175725.GD9511-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 2012-01-13 15:20 ` Oleg Nesterov 2012-01-13 15:20 ` Oleg Nesterov [not found] ` <20120113152010.GA19215-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2012-01-13 18:27 ` Mandeep Singh Baines 2012-01-13 18:27 ` Mandeep Singh Baines [not found] ` <20120113182750.GD18166-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 2012-01-14 17:36 ` Oleg Nesterov 2012-01-14 17:36 ` Oleg Nesterov 2012-01-18 23:17 ` Mandeep Singh Baines 2012-01-18 23:17 ` Mandeep Singh Baines [not found] ` <20120118231742.GS18166-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 2012-01-19 15:45 ` Oleg Nesterov 2012-01-19 15:45 ` Oleg Nesterov 2012-01-19 18:18 ` Mandeep Singh Baines 2012-01-19 18:18 ` Mandeep Singh Baines [not found] ` <20120119181803.GU18166-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 2012-01-20 15:06 ` Oleg Nesterov 2012-01-20 15:06 ` Oleg Nesterov 2012-03-20 19:34 ` Oleg Nesterov 2012-03-20 19:34 ` Oleg Nesterov [not found] ` <20120320193414.GA21277-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2012-03-21 18:59 ` Mandeep Singh Baines 2012-03-21 18:59 ` Mandeep Singh Baines 2012-03-23 17:51 ` Oleg Nesterov 2012-03-23 17:51 ` Oleg Nesterov [not found] ` <20120321185955.GK27051-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> 2012-03-23 17:51 ` Oleg Nesterov [not found] ` <20120119154522.GA14058-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2012-01-19 18:18 ` Mandeep Singh Baines 2012-03-20 19:34 ` Oleg Nesterov [not found] ` <20120114173648.GA32543-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2012-01-18 23:17 ` Mandeep Singh Baines [not found] ` <20120112170728.GA25717-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2012-01-12 17:57 ` Mandeep Singh Baines 2011-12-22 15:30 ` Oleg Nesterov 2011-12-21 19:01 ` Mandeep Singh Baines 2012-02-01 16:28 ` Frederic Weisbecker 2012-02-01 16:28 ` Frederic Weisbecker [not found] ` <20111221130848.GA19679-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2011-12-21 17:56 ` Frederic Weisbecker 2011-12-21 17:59 ` Frederic Weisbecker 2011-12-21 17:59 ` Frederic Weisbecker 2011-12-21 18:11 ` Oleg Nesterov 2011-12-21 18:11 ` Oleg Nesterov [not found] ` <20111221181101.GA3092-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> 2011-12-21 18:23 ` Frederic Weisbecker 2011-12-21 18:23 ` Frederic Weisbecker 2011-12-21 18:23 ` Frederic Weisbecker 2012-02-01 16:28 ` Frederic Weisbecker 2011-12-21 13:08 ` Oleg Nesterov -- strict thread matches above, loose matches on Subject: below -- 2011-12-21 3:43 Frederic Weisbecker
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=20111221192413.GF13529@google.com \ --to=msb-f7+t8e8rja9g9huczpvpmw@public.gmane.org \ --cc=akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@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 \ --cc=oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \ --cc=paul-inf54ven1CmVyaH7bEyXVA@public.gmane.org \ --cc=paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org \ --cc=tj-DgEjT+Ai2ygdnm+yROfE0A@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: linkBe 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.