* [PATCH] cgroups: don't put struct cgroupfs_root protected by RCU
@ 2008-11-21 8:49 Lai Jiangshan
2008-11-21 19:01 ` Paul Menage
[not found] ` <49267618.6080802-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
0 siblings, 2 replies; 4+ messages in thread
From: Lai Jiangshan @ 2008-11-21 8:49 UTC (permalink / raw)
To: Andrew Morton, Paul Menage, Linux Kernel Mailing List,
Linux Containers
we don't access struct cgroupfs_root in fast path,
so we should not put struct cgroupfs_root protected by RCU
but the comment in struct cgroup_subsys.root confuse us.
struct cgroup_subsys.root is used in these places:
1 find_css_set(): if (ss->root->subsys_list.next == &ss->sibling)
2 rebind_subsystems(): if (ss->root != &rootnode)
rcu_assign_pointer(ss->root, root);
rcu_assign_pointer(subsys[i]->root, &rootnode);
3 cgroup_has_css_refs(): if (ss->root != cgrp->root)
4 cgroup_init_subsys(): ss->root = &rootnode;
5 proc_cgroupstats_show(): ss->name, ss->root->subsys_bits,
ss->root->number_of_cgroups, !ss->disabled);
6 cgroup_clone(): root = subsys->root;
if ((root != subsys->root) ||
all these place we have held cgroup_lock() or we don't dereference to
struct cgroupfs_root. It's means wo don't need RCU when use
struct cgroup_subsys.root, and we should not put struct cgroupfs_root
protected by RCU.
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 1164963..22901ff 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -343,7 +345,6 @@ struct cgroup_subsys {
#define MAX_CGROUP_TYPE_NAMELEN 32
const char *name;
- /* Protected by RCU */
struct cgroupfs_root *root;
struct list_head sibling;
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 358e775..ddc10ac 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -717,7 +722,7 @@ static int rebind_subsystems(struct cgroupfs_root *root,
cgrp->subsys[i] = dummytop->subsys[i];
cgrp->subsys[i]->cgroup = cgrp;
list_add(&ss->sibling, &root->subsys_list);
- rcu_assign_pointer(ss->root, root);
+ ss->root = root;
if (ss->bind)
ss->bind(ss, cgrp);
@@ -729,7 +734,7 @@ static int rebind_subsystems(struct cgroupfs_root *root,
ss->bind(ss, dummytop);
dummytop->subsys[i]->cgroup = dummytop;
cgrp->subsys[i] = NULL;
- rcu_assign_pointer(subsys[i]->root, &rootnode);
+ subsys[i]->root = &rootnode;
list_del(&ss->sibling);
} else if (bit & final_bits) {
/* Subsystem state should already exist */
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] cgroups: don't put struct cgroupfs_root protected by RCU
@ 2008-11-21 8:49 Lai Jiangshan
0 siblings, 0 replies; 4+ messages in thread
From: Lai Jiangshan @ 2008-11-21 8:49 UTC (permalink / raw)
To: Andrew Morton, Paul Menage, Linux Kernel Mailing List,
Linux Containers
we don't access struct cgroupfs_root in fast path,
so we should not put struct cgroupfs_root protected by RCU
but the comment in struct cgroup_subsys.root confuse us.
struct cgroup_subsys.root is used in these places:
1 find_css_set(): if (ss->root->subsys_list.next == &ss->sibling)
2 rebind_subsystems(): if (ss->root != &rootnode)
rcu_assign_pointer(ss->root, root);
rcu_assign_pointer(subsys[i]->root, &rootnode);
3 cgroup_has_css_refs(): if (ss->root != cgrp->root)
4 cgroup_init_subsys(): ss->root = &rootnode;
5 proc_cgroupstats_show(): ss->name, ss->root->subsys_bits,
ss->root->number_of_cgroups, !ss->disabled);
6 cgroup_clone(): root = subsys->root;
if ((root != subsys->root) ||
all these place we have held cgroup_lock() or we don't dereference to
struct cgroupfs_root. It's means wo don't need RCU when use
struct cgroup_subsys.root, and we should not put struct cgroupfs_root
protected by RCU.
Signed-off-by: Lai Jiangshan <laijs-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
---
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 1164963..22901ff 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -343,7 +345,6 @@ struct cgroup_subsys {
#define MAX_CGROUP_TYPE_NAMELEN 32
const char *name;
- /* Protected by RCU */
struct cgroupfs_root *root;
struct list_head sibling;
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 358e775..ddc10ac 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -717,7 +722,7 @@ static int rebind_subsystems(struct cgroupfs_root *root,
cgrp->subsys[i] = dummytop->subsys[i];
cgrp->subsys[i]->cgroup = cgrp;
list_add(&ss->sibling, &root->subsys_list);
- rcu_assign_pointer(ss->root, root);
+ ss->root = root;
if (ss->bind)
ss->bind(ss, cgrp);
@@ -729,7 +734,7 @@ static int rebind_subsystems(struct cgroupfs_root *root,
ss->bind(ss, dummytop);
dummytop->subsys[i]->cgroup = dummytop;
cgrp->subsys[i] = NULL;
- rcu_assign_pointer(subsys[i]->root, &rootnode);
+ subsys[i]->root = &rootnode;
list_del(&ss->sibling);
} else if (bit & final_bits) {
/* Subsystem state should already exist */
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] cgroups: don't put struct cgroupfs_root protected by RCU
[not found] ` <49267618.6080802-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
@ 2008-11-21 19:01 ` Paul Menage
0 siblings, 0 replies; 4+ messages in thread
From: Paul Menage @ 2008-11-21 19:01 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Linux Containers, Andrew Morton, Linux Kernel Mailing List
On Fri, Nov 21, 2008 at 12:49 AM, Lai Jiangshan <laijs-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> wrote:
>
> we don't access struct cgroupfs_root in fast path,
> so we should not put struct cgroupfs_root protected by RCU
>
> but the comment in struct cgroup_subsys.root confuse us.
>
> struct cgroup_subsys.root is used in these places:
>
> 1 find_css_set(): if (ss->root->subsys_list.next == &ss->sibling)
> 2 rebind_subsystems(): if (ss->root != &rootnode)
> rcu_assign_pointer(ss->root, root);
> rcu_assign_pointer(subsys[i]->root, &rootnode);
> 3 cgroup_has_css_refs(): if (ss->root != cgrp->root)
> 4 cgroup_init_subsys(): ss->root = &rootnode;
> 5 proc_cgroupstats_show(): ss->name, ss->root->subsys_bits,
> ss->root->number_of_cgroups, !ss->disabled);
> 6 cgroup_clone(): root = subsys->root;
> if ((root != subsys->root) ||
>
> all these place we have held cgroup_lock() or we don't dereference to
> struct cgroupfs_root. It's means wo don't need RCU when use
> struct cgroup_subsys.root, and we should not put struct cgroupfs_root
> protected by RCU.
>
> Signed-off-by: Lai Jiangshan <laijs-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
Reviewed-by: Paul Menage <menage-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Thanks, seems reasonable.
> ---
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index 1164963..22901ff 100644
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -343,7 +345,6 @@ struct cgroup_subsys {
> #define MAX_CGROUP_TYPE_NAMELEN 32
> const char *name;
>
> - /* Protected by RCU */
> struct cgroupfs_root *root;
>
> struct list_head sibling;
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index 358e775..ddc10ac 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -717,7 +722,7 @@ static int rebind_subsystems(struct cgroupfs_root *root,
> cgrp->subsys[i] = dummytop->subsys[i];
> cgrp->subsys[i]->cgroup = cgrp;
> list_add(&ss->sibling, &root->subsys_list);
> - rcu_assign_pointer(ss->root, root);
> + ss->root = root;
> if (ss->bind)
> ss->bind(ss, cgrp);
>
> @@ -729,7 +734,7 @@ static int rebind_subsystems(struct cgroupfs_root *root,
> ss->bind(ss, dummytop);
> dummytop->subsys[i]->cgroup = dummytop;
> cgrp->subsys[i] = NULL;
> - rcu_assign_pointer(subsys[i]->root, &rootnode);
> + subsys[i]->root = &rootnode;
> list_del(&ss->sibling);
> } else if (bit & final_bits) {
> /* Subsystem state should already exist */
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] cgroups: don't put struct cgroupfs_root protected by RCU
2008-11-21 8:49 [PATCH] cgroups: don't put struct cgroupfs_root protected by RCU Lai Jiangshan
@ 2008-11-21 19:01 ` Paul Menage
[not found] ` <49267618.6080802-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
1 sibling, 0 replies; 4+ messages in thread
From: Paul Menage @ 2008-11-21 19:01 UTC (permalink / raw)
To: Lai Jiangshan; +Cc: Andrew Morton, Linux Kernel Mailing List, Linux Containers
On Fri, Nov 21, 2008 at 12:49 AM, Lai Jiangshan <laijs@cn.fujitsu.com> wrote:
>
> we don't access struct cgroupfs_root in fast path,
> so we should not put struct cgroupfs_root protected by RCU
>
> but the comment in struct cgroup_subsys.root confuse us.
>
> struct cgroup_subsys.root is used in these places:
>
> 1 find_css_set(): if (ss->root->subsys_list.next == &ss->sibling)
> 2 rebind_subsystems(): if (ss->root != &rootnode)
> rcu_assign_pointer(ss->root, root);
> rcu_assign_pointer(subsys[i]->root, &rootnode);
> 3 cgroup_has_css_refs(): if (ss->root != cgrp->root)
> 4 cgroup_init_subsys(): ss->root = &rootnode;
> 5 proc_cgroupstats_show(): ss->name, ss->root->subsys_bits,
> ss->root->number_of_cgroups, !ss->disabled);
> 6 cgroup_clone(): root = subsys->root;
> if ((root != subsys->root) ||
>
> all these place we have held cgroup_lock() or we don't dereference to
> struct cgroupfs_root. It's means wo don't need RCU when use
> struct cgroup_subsys.root, and we should not put struct cgroupfs_root
> protected by RCU.
>
> Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
Reviewed-by: Paul Menage <menage@google.com>
Thanks, seems reasonable.
> ---
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index 1164963..22901ff 100644
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -343,7 +345,6 @@ struct cgroup_subsys {
> #define MAX_CGROUP_TYPE_NAMELEN 32
> const char *name;
>
> - /* Protected by RCU */
> struct cgroupfs_root *root;
>
> struct list_head sibling;
> diff --git a/kernel/cgroup.c b/kernel/cgroup.c
> index 358e775..ddc10ac 100644
> --- a/kernel/cgroup.c
> +++ b/kernel/cgroup.c
> @@ -717,7 +722,7 @@ static int rebind_subsystems(struct cgroupfs_root *root,
> cgrp->subsys[i] = dummytop->subsys[i];
> cgrp->subsys[i]->cgroup = cgrp;
> list_add(&ss->sibling, &root->subsys_list);
> - rcu_assign_pointer(ss->root, root);
> + ss->root = root;
> if (ss->bind)
> ss->bind(ss, cgrp);
>
> @@ -729,7 +734,7 @@ static int rebind_subsystems(struct cgroupfs_root *root,
> ss->bind(ss, dummytop);
> dummytop->subsys[i]->cgroup = dummytop;
> cgrp->subsys[i] = NULL;
> - rcu_assign_pointer(subsys[i]->root, &rootnode);
> + subsys[i]->root = &rootnode;
> list_del(&ss->sibling);
> } else if (bit & final_bits) {
> /* Subsystem state should already exist */
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2008-11-21 19:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-11-21 8:49 [PATCH] cgroups: don't put struct cgroupfs_root protected by RCU Lai Jiangshan
2008-11-21 19:01 ` Paul Menage
[not found] ` <49267618.6080802-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2008-11-21 19:01 ` Paul Menage
-- strict thread matches above, loose matches on Subject: below --
2008-11-21 8:49 Lai Jiangshan
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.