All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: lizefan@huawei.com
Cc: cgroups@vger.kernel.org, linux-kernel@vger.kernel.org,
	Tejun Heo <tj@kernel.org>
Subject: [PATCH 3/7] cgroup: css_release() shouldn't clear cgroup->subsys[]
Date: Fri,  9 May 2014 15:32:51 -0400	[thread overview]
Message-ID: <1399663975-315-4-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1399663975-315-1-git-send-email-tj@kernel.org>

c1a71504e971 ("cgroup: don't recycle cgroup id until all csses' have
been destroyed") made cgroup ID persist until a cgroup is released and
add cgroup->subsys[] clearing to css_release() so that css_from_id()
doesn't return a css which has already been released which happens
before cgroup release; however, the right change here was updating
offline_css() to clear cgroup->subsys[] which was done by e32978031016
("cgroup: cgroup->subsys[] should be cleared after the css is
offlined") instead of clearing it from css_release().

We're now clearing cgroup->subsys[] twice.  This is okay for
traditional hierarchies as a css's lifetime is the same as its
cgroup's; however, this confuses unified hierarchy and turning on and
off a controller repeatedly using "cgroup.subtree_control" can lead to
an oops like the following which happens because cgroup->subsys[] is
incorrectly cleared asynchronously by css_release().

 BUG: unable to handle kernel NULL pointer dereference at 00000000000000 08
 IP: [<ffffffff81130c11>] kill_css+0x21/0x1c0
 PGD 1170d067 PUD f0ab067 PMD 0
 Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
 Modules linked in:
 CPU: 2 PID: 459 Comm: bash Not tainted 3.15.0-rc2-work+ #5
 Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
 task: ffff880009296710 ti: ffff88000e198000 task.ti: ffff88000e198000
 RIP: 0010:[<ffffffff81130c11>]  [<ffffffff81130c11>] kill_css+0x21/0x1c0
 RSP: 0018:ffff88000e199dc8  EFLAGS: 00010202
 RAX: 0000000000000001 RBX: 0000000000000000 RCX: 0000000000000001
 RDX: 0000000000000001 RSI: ffffffff8238a968 RDI: ffff880009296f98
 RBP: ffff88000e199de0 R08: 0000000000000001 R09: 02b0000000000000
 R10: 0000000000000000 R11: ffff880009296fc0 R12: 0000000000000001
 R13: ffff88000db6fc58 R14: 0000000000000001 R15: ffff8800139dcc00
 FS:  00007ff9160c5740(0000) GS:ffff88001fb00000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 0000000000000008 CR3: 0000000013947000 CR4: 00000000000006e0
 Stack:
  ffff88000e199de0 ffffffff82389160 0000000000000001 ffff88000e199e80
  ffffffff8113537f 0000000000000007 ffff88000e74af00 ffff88000e199e48
  ffff880009296710 ffff88000db6fc00 ffffffff8239c100 0000000000000002
 Call Trace:
  [<ffffffff8113537f>] cgroup_subtree_control_write+0x85f/0xa00
  [<ffffffff8112fd18>] cgroup_file_write+0x38/0x1d0
  [<ffffffff8126fc97>] kernfs_fop_write+0xe7/0x170
  [<ffffffff811f2ae6>] vfs_write+0xb6/0x1c0
  [<ffffffff811f35ad>] SyS_write+0x4d/0xc0
  [<ffffffff81d0acd2>] system_call_fastpath+0x16/0x1b
 Code: 5c 41 5d 41 5e 41 5f 5d c3 90 0f 1f 44 00 00 55 48 89 e5 41 54 53 48 89 fb 48 83 ec 08 8b 05 37 ad 29 01 85 c0 0f 85 df 00 00 00 <48> 8b 43 08 48 8b 3b be 01 00 00 00 8b 48 5c d3 e6 e8 49 ff ff
 RIP  [<ffffffff81130c11>] kill_css+0x21/0x1c0
  RSP <ffff88000e199dc8>
 CR2: 0000000000000008
 ---[ end trace e7aae1f877c4e1b4 ]---

Remove the unnecessary cgroup->subsys[] clearing from css_release().

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/cgroup.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index e2ff925..35daf89 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -4102,7 +4102,6 @@ static void css_release(struct percpu_ref *ref)
 		container_of(ref, struct cgroup_subsys_state, refcnt);
 	struct cgroup_subsys *ss = css->ss;
 
-	RCU_INIT_POINTER(css->cgroup->subsys[ss->id], NULL);
 	cgroup_idr_remove(&ss->css_idr, css->id);
 
 	call_rcu(&css->rcu_head, css_free_rcu_fn);
-- 
1.9.0

  parent reply	other threads:[~2014-05-09 19:32 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-09 19:32 [PATCHSET v2 cgroup/for-3.16] cgroup: post unified hierarchy fixes and updates Tejun Heo
2014-05-09 19:32 ` [PATCH 1/7] cgroup: fix offlining child waiting in cgroup_subtree_control_write() Tejun Heo
2014-05-09 19:32 ` [PATCH 2/7] cgroup: cgroup_idr_lock should be bh Tejun Heo
2014-05-09 19:32 ` Tejun Heo [this message]
2014-05-09 19:32 ` [PATCH 4/7] cgroup: update and fix parsing of "cgroup.subtree_control" Tejun Heo
     [not found]   ` <1399663975-315-5-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2014-05-13  3:51     ` Li Zefan
2014-05-13  3:51       ` Li Zefan
     [not found]       ` <537196D7.3020302-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2014-05-13 15:18         ` Tejun Heo
2014-05-13 15:18           ` Tejun Heo
2014-05-09 19:32 ` [PATCH 5/7] cgroup: use restart_syscall() for retries after offline waits in cgroup_subtree_control_write() Tejun Heo
     [not found]   ` <1399663975-315-6-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2014-05-13  5:00     ` Li Zefan
2014-05-13  5:00       ` Li Zefan
     [not found]       ` <5371A6DB.2050908-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2014-05-13 15:19         ` Tejun Heo
2014-05-13 15:19           ` Tejun Heo
2014-05-09 19:32 ` [PATCH 6/7] cgroup: use release_agent_path_lock in cgroup_release_agent_show() Tejun Heo
2014-05-09 19:32 ` [PATCH 7/7] cgroup: rename css_tryget*() to css_tryget_online*() Tejun Heo
     [not found]   ` <1399663975-315-8-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2014-05-09 19:47     ` [PATCH v2 " Tejun Heo
2014-05-09 19:47       ` Tejun Heo
     [not found] ` <1399663975-315-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2014-05-13  6:35   ` [PATCHSET v2 cgroup/for-3.16] cgroup: post unified hierarchy fixes and updates Li Zefan
2014-05-13  6:35     ` Li Zefan
2014-05-13 16:12   ` Tejun Heo
2014-05-13 16:12     ` 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=1399663975-315-4-git-send-email-tj@kernel.org \
    --to=tj@kernel.org \
    --cc=cgroups@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan@huawei.com \
    /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.