2013/2/7 Li Zefan <[email protected]>: > CC: Tejun > > 于 2013/2/6 23:16, Maxim Uvarov wrote: >> linux-3.0.y has put_css_set a litte bit down in the code, >> this should not be duplicated. > > Why do you want to make this change? Have you encountered some kernel bug > that relates > to cgroup? >
Yes, I thought that this code by mistake calls twice put_css_set() which affects to rcu code, which I see in rcu_implicit_dynticks_qs(). And when I am hot adding vcpu under xen I see this back trace: Pid: 3214, comm: udevd Not tainted #1 Xen HVM domU [<ffffffff81011c50>] xen_smp_send_reschedule+0x10/0x20 [<ffffffff810dbfc1>] rcu_implicit_offline_qs+0x41/0x80 [<ffffffff81507fa4>] ? _raw_spin_lock_irqsave+0x34/0x50 [<ffffffff810dd8b7>] rcu_implicit_dynticks_qs+0x47/0x50 [<ffffffff810dc4b9>] force_qs_rnp+0xa9/0x170 [<ffffffff810dd870>] ? rcu_check_callbacks+0xa0/0xa0 [<ffffffff810dc964>] force_quiescent_state+0x194/0x1b0 [<ffffffff810dd06d>] __rcu_process_callbacks+0x9d/0xd0 [<ffffffff810dd0c5>] rcu_process_callbacks+0x25/0x50 [<ffffffff81075c39>] __do_softirq+0xb9/0x1d0 After removing it back trace disappeared. So I thought that this code was duplicated by mistake, maybe git cherry-pick artifact. >> Signed-off-by: Maxim Uvarov <[email protected]> > > NAck > Agree, please do not commit this change. >> --- >> kernel/cgroup.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/kernel/cgroup.c b/kernel/cgroup.c >> index 1749dcd..2a08524 100644 >> --- a/kernel/cgroup.c >> +++ b/kernel/cgroup.c >> @@ -1777,7 +1777,6 @@ static int cgroup_task_migrate(struct cgroup *cgrp, >> struct cgroup *oldcgrp, >> return -ENOMEM; >> } >> } >> - put_css_set(oldcg); > > This put_css_set() is paired with the code a little above: > > task_lock(tsk); > oldcg = tsk->cgroups; > get_css_set(oldcg); > task_unlock(tsk); > > It was added by this commit: > > commit 77efecd9e0526327548152df715ab8644ecb5ba0 > Author: Lai Jiangshan <[email protected]> > Date: Wed Jan 7 18:07:39 2009 -0800 > > cgroups: call find_css_set() safely in cgroup_attach_task() > > And the put_css_set() at the end of this function is to really drop the ref > to tsk->cgroups, > as tsk->cgroups will point to @newcg, so @oldcg should be de-referenced. > > But yeah the comment above the second put_css_set() is confusing: > > /* > * We just gained a reference on oldcg by taking it from the task ... > * ... > */ > > I'll fix the comment for mainline. > >> >> /* if PF_EXITING is set, the tsk->cgroups pointer is no longer safe. */ >> task_lock(tsk); >> > Thank you for explanation this, fixing comment will be also useful here. -- Best regards, Maxim Uvarov -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
