[PATCH] cgroup: missing rcu read lock around task_css_set
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 --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cgroup: missing rcu read lock around task_css_set
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cgroup: missing rcu read lock around task_css_set
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] 0001 85e01d58 8446a933 0001 [0.377753] 85e2e4a0 85e01d88 811a6ddb [0.380550] 88052a2398a8 88052a238000 85e01de8 [0.381370] Call Trace: [0.381641] [] dump_stack+0x52/0x7f [0.382261] [] lockdep_rcu_suspicious+0x10b/0x120 [0.382893] [] cgroup_exit+0x20d/0x250 [0.383566] [] ? ktime_get_ts+0x145/0x1d0 [0.384221] [] copy_process+0x5d6/0x670 [0.384850] [] do_fork+0x8b/0x2e0 [0.385347] [] ? trace_hardirqs_on+0xd/0x10 [0.386009] [] ? mutex_unlock+0xe/0x10 [0.386559] [] ? early_idt_handlers+0x117/0x120 [0.387327] [] kernel_thread+0x26/0x30 [0.387947] [] rest_init+0x26/0x150 [0.388491] [] start_kernel+0x3c0/0x3c7 [0.389126] [] ? repair_env_string+0x5b/0x5b [0.389813] [] ? memblock_reserve+0x49/0x4e [0.390019] [] x86_64_start_reservations+0x2a/0x2c [0.390754] [] x86_64_start_kernel+0x186/0x195 Thanks, Sasha -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cgroup: missing rcu read lock around task_css_set
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cgroup: missing rcu read lock around task_css_set
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cgroup: missing rcu read lock around task_css_set
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cgroup: missing rcu read lock around task_css_set
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cgroup: missing rcu read lock around task_css_set
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cgroup: missing rcu read lock around task_css_set
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(). -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cgroup: missing rcu read lock around task_css_set
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] cgroup: missing rcu read lock around task_css_set
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! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
