* [PATCH] cgroup: missing rcu read lock around task_css_set
@ 2014-03-02 3:00 Sasha Levin
2014-03-03 22:33 ` Tejun Heo
0 siblings, 1 reply; 11+ messages in thread
From: Sasha Levin @ 2014-03-02 3:00 UTC (permalink / raw)
To: tj, lizefan; +Cc: containers, cgroups, linux-kernel, Sasha Levin
rcu read lock should be held when calling and working with task_css_set.
This patch also fixes a related lockdep warning.
Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
kernel/cgroup.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 8f4ddbe..f638ad6 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4361,6 +4361,7 @@ void cgroup_exit(struct task_struct *tsk, int run_callbacks)
}
/* Reassign the task to the init_css_set. */
+ rcu_read_lock();
cset = task_css_set(tsk);
RCU_INIT_POINTER(tsk->cgroups, &init_css_set);
@@ -4378,6 +4379,7 @@ void cgroup_exit(struct task_struct *tsk, int run_callbacks)
if (put_cset)
put_css_set(cset, true);
+ rcu_read_unlock();
}
static void check_for_release(struct cgroup *cgrp)
--
1.7.2.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] cgroup: missing rcu read lock around task_css_set
2014-03-02 3:00 [PATCH] cgroup: missing rcu read lock around task_css_set Sasha Levin
@ 2014-03-03 22:33 ` Tejun Heo
2014-03-03 22:43 ` Sasha Levin
0 siblings, 1 reply; 11+ messages in thread
From: Tejun Heo @ 2014-03-03 22:33 UTC (permalink / raw)
To: Sasha Levin; +Cc: lizefan, containers, cgroups, linux-kernel
On Sat, Mar 01, 2014 at 10:00:11PM -0500, Sasha Levin wrote:
> rcu read lock should be held when calling and working with task_css_set.
>
> This patch also fixes a related lockdep warning.
Hmmm... PF_EXITING should be visible at that point and cset can't
change anymore. We prolly need to update lockdep annotation rather
than adding spurious rcu locking around it. Against which branch is
it? Can you please post the lockdep warning?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] cgroup: missing rcu read lock around task_css_set
2014-03-03 22:33 ` Tejun Heo
@ 2014-03-03 22:43 ` Sasha Levin
2014-03-03 22:45 ` Tejun Heo
0 siblings, 1 reply; 11+ messages in thread
From: Sasha Levin @ 2014-03-03 22:43 UTC (permalink / raw)
To: Tejun Heo; +Cc: lizefan, containers, cgroups, linux-kernel
On 03/03/2014 05:33 PM, Tejun Heo wrote:
> On Sat, Mar 01, 2014 at 10:00:11PM -0500, Sasha Levin wrote:
>> rcu read lock should be held when calling and working with task_css_set.
>>
>> This patch also fixes a related lockdep warning.
>
> Hmmm... PF_EXITING should be visible at that point and cset can't
> change anymore. We prolly need to update lockdep annotation rather
> than adding spurious rcu locking around it. Against which branch is
> it? Can you please post the lockdep warning?
I see it on -next.
[ 0.370543] ===============================
[ 0.371030] [ INFO: suspicious RCU usage. ]
[ 0.371453] 3.14.0-rc4-next-20140303-sasha-00012-g35a2897-dirty #43 Not tainted
[ 0.372223] -------------------------------
[ 0.372627] include/linux/cgroup.h:692 suspicious rcu_dereference_check() usage!
[ 0.373417]
[ 0.373417] other info that might help us debug this:
[ 0.373417]
[ 0.374223]
[ 0.374223] rcu_scheduler_active = 1, debug_locks = 1
[ 0.374993] no locks held by swapper/0/0.
[ 0.375422]
[ 0.375422] stack backtrace:
[ 0.375865] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.14.0-rc4-next-20140303-sasha-00012-g35a2897-dirty #43
[ 0.376936] 0000000000000001 ffffffff85e01d58 ffffffff8446a933 0000000000000001
[ 0.377753] ffffffff85e2e4a0 ffffffff85e01d88 ffffffff811a6ddb 0000000000000000
[ 0.380550] ffff88052a2398a8 ffff88052a238000 0000000000000000 ffffffff85e01de8
[ 0.381370] Call Trace:
[ 0.381641] [<ffffffff8446a933>] dump_stack+0x52/0x7f
[ 0.382261] [<ffffffff811a6ddb>] lockdep_rcu_suspicious+0x10b/0x120
[ 0.382893] [<ffffffff8120024d>] cgroup_exit+0x20d/0x250
[ 0.383566] [<ffffffff811dc075>] ? ktime_get_ts+0x145/0x1d0
[ 0.384221] [<ffffffff8113fd66>] copy_process+0x5d6/0x670
[ 0.384850] [<ffffffff8114049b>] do_fork+0x8b/0x2e0
[ 0.385347] [<ffffffff811a9c0d>] ? trace_hardirqs_on+0xd/0x10
[ 0.386009] [<ffffffff8446d47e>] ? mutex_unlock+0xe/0x10
[ 0.386559] [<ffffffff872a4117>] ? early_idt_handlers+0x117/0x120
[ 0.387327] [<ffffffff81140776>] kernel_thread+0x26/0x30
[ 0.387947] [<ffffffff844606a6>] rest_init+0x26/0x150
[ 0.388491] [<ffffffff872a538d>] start_kernel+0x3c0/0x3c7
[ 0.389126] [<ffffffff872a4dfb>] ? repair_env_string+0x5b/0x5b
[ 0.389813] [<ffffffff84468dcb>] ? memblock_reserve+0x49/0x4e
[ 0.390019] [<ffffffff872a43d9>] x86_64_start_reservations+0x2a/0x2c
[ 0.390754] [<ffffffff872a45ae>] x86_64_start_kernel+0x186/0x195
Thanks,
Sasha
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] cgroup: missing rcu read lock around task_css_set
2014-03-03 22:43 ` Sasha Levin
@ 2014-03-03 22:45 ` Tejun Heo
2014-03-03 22:48 ` Sasha Levin
2014-03-03 23:00 ` Sasha Levin
0 siblings, 2 replies; 11+ messages in thread
From: Tejun Heo @ 2014-03-03 22:45 UTC (permalink / raw)
To: Sasha Levin; +Cc: lizefan, containers, cgroups, linux-kernel
On Mon, Mar 03, 2014 at 05:43:11PM -0500, Sasha Levin wrote:
> On 03/03/2014 05:33 PM, Tejun Heo wrote:
> >On Sat, Mar 01, 2014 at 10:00:11PM -0500, Sasha Levin wrote:
> >>rcu read lock should be held when calling and working with task_css_set.
> >>
> >>This patch also fixes a related lockdep warning.
> >
> >Hmmm... PF_EXITING should be visible at that point and cset can't
> >change anymore. We prolly need to update lockdep annotation rather
> >than adding spurious rcu locking around it. Against which branch is
> >it? Can you please post the lockdep warning?
>
> I see it on -next.
I think the right thing to do is using task_css_set_check() with
PF_EXITING check and add comment explaining that cset can no longer
change. Can you please write up a patch?
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] cgroup: missing rcu read lock around task_css_set
2014-03-03 22:45 ` Tejun Heo
@ 2014-03-03 22:48 ` Sasha Levin
2014-03-03 23:00 ` Sasha Levin
1 sibling, 0 replies; 11+ messages in thread
From: Sasha Levin @ 2014-03-03 22:48 UTC (permalink / raw)
To: Tejun Heo; +Cc: lizefan, containers, cgroups, linux-kernel
On 03/03/2014 05:45 PM, Tejun Heo wrote:
> On Mon, Mar 03, 2014 at 05:43:11PM -0500, Sasha Levin wrote:
>> On 03/03/2014 05:33 PM, Tejun Heo wrote:
>>> On Sat, Mar 01, 2014 at 10:00:11PM -0500, Sasha Levin wrote:
>>>> rcu read lock should be held when calling and working with task_css_set.
>>>>
>>>> This patch also fixes a related lockdep warning.
>>>
>>> Hmmm... PF_EXITING should be visible at that point and cset can't
>>> change anymore. We prolly need to update lockdep annotation rather
>>> than adding spurious rcu locking around it. Against which branch is
>>> it? Can you please post the lockdep warning?
>>
>> I see it on -next.
>
> I think the right thing to do is using task_css_set_check() with
> PF_EXITING check and add comment explaining that cset can no longer
> change. Can you please write up a patch?
Sure!
Thanks,
Sasha
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] cgroup: missing rcu read lock around task_css_set
2014-03-03 22:45 ` Tejun Heo
2014-03-03 22:48 ` Sasha Levin
@ 2014-03-03 23:00 ` Sasha Levin
2014-03-04 17:20 ` Sasha Levin
1 sibling, 1 reply; 11+ messages in thread
From: Sasha Levin @ 2014-03-03 23:00 UTC (permalink / raw)
To: Tejun Heo; +Cc: lizefan, containers, cgroups, linux-kernel
On 03/03/2014 05:45 PM, Tejun Heo wrote:
> On Mon, Mar 03, 2014 at 05:43:11PM -0500, Sasha Levin wrote:
>> On 03/03/2014 05:33 PM, Tejun Heo wrote:
>>> On Sat, Mar 01, 2014 at 10:00:11PM -0500, Sasha Levin wrote:
>>>> rcu read lock should be held when calling and working with task_css_set.
>>>>
>>>> This patch also fixes a related lockdep warning.
>>>
>>> Hmmm... PF_EXITING should be visible at that point and cset can't
>>> change anymore. We prolly need to update lockdep annotation rather
>>> than adding spurious rcu locking around it. Against which branch is
>>> it? Can you please post the lockdep warning?
>>
>> I see it on -next.
>
> I think the right thing to do is using task_css_set_check() with
> PF_EXITING check and add comment explaining that cset can no longer
> change. Can you please write up a patch?
Hrm... there is a PF_EXITING check there already:
#define task_css_set_check(task, __c) \
rcu_dereference_check((task)->cgroups, \
lockdep_is_held(&cgroup_mutex) || \
lockdep_is_held(&css_set_rwsem) || \
((task)->flags & PF_EXITING) || (__c))
I see it's not happening on Linus's master so I'll run a bisection to figure out what broke it.
Thanks,
Sasha
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] cgroup: missing rcu read lock around task_css_set
2014-03-03 23:00 ` Sasha Levin
@ 2014-03-04 17:20 ` Sasha Levin
2014-03-04 19:47 ` Tejun Heo
0 siblings, 1 reply; 11+ messages in thread
From: Sasha Levin @ 2014-03-04 17:20 UTC (permalink / raw)
To: Tejun Heo; +Cc: lizefan, containers, cgroups, linux-kernel
On 03/03/2014 06:00 PM, Sasha Levin wrote:
> On 03/03/2014 05:45 PM, Tejun Heo wrote:
>> On Mon, Mar 03, 2014 at 05:43:11PM -0500, Sasha Levin wrote:
>>> On 03/03/2014 05:33 PM, Tejun Heo wrote:
>>>> On Sat, Mar 01, 2014 at 10:00:11PM -0500, Sasha Levin wrote:
>>>>> rcu read lock should be held when calling and working with task_css_set.
>>>>>
>>>>> This patch also fixes a related lockdep warning.
>>>>
>>>> Hmmm... PF_EXITING should be visible at that point and cset can't
>>>> change anymore. We prolly need to update lockdep annotation rather
>>>> than adding spurious rcu locking around it. Against which branch is
>>>> it? Can you please post the lockdep warning?
>>>
>>> I see it on -next.
>>
>> I think the right thing to do is using task_css_set_check() with
>> PF_EXITING check and add comment explaining that cset can no longer
>> change. Can you please write up a patch?
>
> Hrm... there is a PF_EXITING check there already:
>
> #define task_css_set_check(task, __c) \
> rcu_dereference_check((task)->cgroups, \
> lockdep_is_held(&cgroup_mutex) || \
> lockdep_is_held(&css_set_rwsem) || \
> ((task)->flags & PF_EXITING) || (__c))
>
> I see it's not happening on Linus's master so I'll run a bisection to figure out what broke it.
Hi Tejun,
It bisects down to your patch: "cgroup: drop task_lock() protection around task->cgroups". I'll
look into it later unless it's obvious to you.
Thanks,
Sasha
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] cgroup: missing rcu read lock around task_css_set
2014-03-04 17:20 ` Sasha Levin
@ 2014-03-04 19:47 ` Tejun Heo
2014-03-05 0:57 ` Li Zefan
0 siblings, 1 reply; 11+ messages in thread
From: Tejun Heo @ 2014-03-04 19:47 UTC (permalink / raw)
To: Sasha Levin; +Cc: lizefan, containers, cgroups, linux-kernel
On Tue, Mar 04, 2014 at 12:20:45PM -0500, Sasha Levin wrote:
> >Hrm... there is a PF_EXITING check there already:
> >
> > #define task_css_set_check(task, __c) \
> > rcu_dereference_check((task)->cgroups, \
> > lockdep_is_held(&cgroup_mutex) || \
> > lockdep_is_held(&css_set_rwsem) || \
> > ((task)->flags & PF_EXITING) || (__c))
> >
> >I see it's not happening on Linus's master so I'll run a bisection to figure out what broke it.
>
> Hi Tejun,
>
> It bisects down to your patch: "cgroup: drop task_lock() protection
> around task->cgroups". I'll look into it later unless it's obvious
> to you.
Hmmm... maybe I'm confused and PF_EXITING is not set there and
task_lock was what held off the lockdep warning. Confused....
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] cgroup: missing rcu read lock around task_css_set
2014-03-04 19:47 ` Tejun Heo
@ 2014-03-05 0:57 ` Li Zefan
2014-03-27 15:35 ` Sasha Levin
0 siblings, 1 reply; 11+ messages in thread
From: Li Zefan @ 2014-03-05 0:57 UTC (permalink / raw)
To: Tejun Heo; +Cc: Sasha Levin, containers, cgroups, linux-kernel
On 2014/3/5 3:47, Tejun Heo wrote:
> On Tue, Mar 04, 2014 at 12:20:45PM -0500, Sasha Levin wrote:
>>> Hrm... there is a PF_EXITING check there already:
>>>
>>> #define task_css_set_check(task, __c) \
>>> rcu_dereference_check((task)->cgroups, \
>>> lockdep_is_held(&cgroup_mutex) || \
>>> lockdep_is_held(&css_set_rwsem) || \
>>> ((task)->flags & PF_EXITING) || (__c))
>>>
>>> I see it's not happening on Linus's master so I'll run a bisection to figure out what broke it.
>>
>> Hi Tejun,
>>
>> It bisects down to your patch: "cgroup: drop task_lock() protection
>> around task->cgroups". I'll look into it later unless it's obvious
>> to you.
>
> Hmmm... maybe I'm confused and PF_EXITING is not set there and
> task_lock was what held off the lockdep warning. Confused....
>
Because this cgroup_exit() is called in a failure path in copy_process().
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] cgroup: missing rcu read lock around task_css_set
2014-03-05 0:57 ` Li Zefan
@ 2014-03-27 15:35 ` Sasha Levin
2014-03-28 2:48 ` Li Zefan
0 siblings, 1 reply; 11+ messages in thread
From: Sasha Levin @ 2014-03-27 15:35 UTC (permalink / raw)
To: Li Zefan, Tejun Heo; +Cc: containers, cgroups, linux-kernel
On 03/04/2014 07:57 PM, Li Zefan wrote:
> On 2014/3/5 3:47, Tejun Heo wrote:
>> On Tue, Mar 04, 2014 at 12:20:45PM -0500, Sasha Levin wrote:
>>>> Hrm... there is a PF_EXITING check there already:
>>>>
>>>> #define task_css_set_check(task, __c) \
>>>> rcu_dereference_check((task)->cgroups, \
>>>> lockdep_is_held(&cgroup_mutex) || \
>>>> lockdep_is_held(&css_set_rwsem) || \
>>>> ((task)->flags & PF_EXITING) || (__c))
>>>>
>>>> I see it's not happening on Linus's master so I'll run a bisection to figure out what broke it.
>>>
>>> Hi Tejun,
>>>
>>> It bisects down to your patch: "cgroup: drop task_lock() protection
>>> around task->cgroups". I'll look into it later unless it's obvious
>>> to you.
>>
>> Hmmm... maybe I'm confused and PF_EXITING is not set there and
>> task_lock was what held off the lockdep warning. Confused....
>>
>
> Because this cgroup_exit() is called in a failure path in copy_process().
It seems there was no conclusion here and it still happens in -next, anything
we can do about it?
Thanks,
Sasha
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] cgroup: missing rcu read lock around task_css_set
2014-03-27 15:35 ` Sasha Levin
@ 2014-03-28 2:48 ` Li Zefan
0 siblings, 0 replies; 11+ messages in thread
From: Li Zefan @ 2014-03-28 2:48 UTC (permalink / raw)
To: Sasha Levin; +Cc: Tejun Heo, containers, cgroups, linux-kernel
On 2014/3/27 23:35, Sasha Levin wrote:
> On 03/04/2014 07:57 PM, Li Zefan wrote:
>> On 2014/3/5 3:47, Tejun Heo wrote:
>>> On Tue, Mar 04, 2014 at 12:20:45PM -0500, Sasha Levin wrote:
>>>>> Hrm... there is a PF_EXITING check there already:
>>>>>
>>>>> #define task_css_set_check(task, __c) \
>>>>> rcu_dereference_check((task)->cgroups, \
>>>>> lockdep_is_held(&cgroup_mutex) || \
>>>>> lockdep_is_held(&css_set_rwsem) || \
>>>>> ((task)->flags & PF_EXITING) || (__c))
>>>>>
>>>>> I see it's not happening on Linus's master so I'll run a bisection to figure out what broke it.
>>>>
>>>> Hi Tejun,
>>>>
>>>> It bisects down to your patch: "cgroup: drop task_lock() protection
>>>> around task->cgroups". I'll look into it later unless it's obvious
>>>> to you.
>>>
>>> Hmmm... maybe I'm confused and PF_EXITING is not set there and
>>> task_lock was what held off the lockdep warning. Confused....
>>>
>>
>> Because this cgroup_exit() is called in a failure path in copy_process().
>
> It seems there was no conclusion here and it still happens in -next, anything
> we can do about it?
>
I'll send a patch to fix it. Thanks!
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-03-28 2:51 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-02 3:00 [PATCH] cgroup: missing rcu read lock around task_css_set Sasha Levin
2014-03-03 22:33 ` Tejun Heo
2014-03-03 22:43 ` Sasha Levin
2014-03-03 22:45 ` Tejun Heo
2014-03-03 22:48 ` Sasha Levin
2014-03-03 23:00 ` Sasha Levin
2014-03-04 17:20 ` Sasha Levin
2014-03-04 19:47 ` Tejun Heo
2014-03-05 0:57 ` Li Zefan
2014-03-27 15:35 ` Sasha Levin
2014-03-28 2:48 ` Li Zefan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).