memcg, cgroup: kill css id
Subsystems (3)
v6 cover memcg, cgroup: kill css id
Hi Andrew,
The whole patchset has been acked and reviewed by Michal and Tejun. Could you merge it into mm tree?
===========
This patchset converts memcg to use cgroup->id, and then we remove cgroup css_id.
As we've removed memcg's own refcnt, converting memcg to use cgroup->id is very straight-forward.
v6:
- rebased against mmotm 2013-09-20-15-59
- moved cgroup id check from mem_cgroup_css_alloc() to mem_cgroup_css_online()
v5:
- rebased against mmotm 2013-08-07-16-55
v4:
- make cgroup_from_id() inline and check if cgroup_mutex is held.
- add a comment for idr_remove() in cgroup_offline)fn().
v2->v3:
- some minor cleanups suggested by Michal.
- fixed the call to idr_alloc() in cgroup_init() in the first patch.
Li Zefan (5):
memcg: convert to use cgroup_is_descendant()
memcg: convert to use cgroup id
memcg: fail to create cgroup if the cgroup id is too big
memcg: stop using css id
cgroup: kill css_id
include/linux/cgroup.h | 37 -------- kernel/cgroup.c | 248 +------------------------------------------------ mm/memcontrol.c | 66 +++++++------ 3 files changed, 41 insertions(+), 310 deletions(-)
Signature · 7 lines
1.8.0.2 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v6 1/5 memcg: convert to use cgroup_is_descendant()
This is a preparation to kill css_id.
--- mm/memcontrol.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
mm/memcontrol.c +1 -1
@@ -1405,7 +1405,7 @@ bool __mem_cgroup_same_or_subtree(const struct mem_cgroup *root_memcg,return true;if (!root_memcg->use_hierarchy || !memcg)return false; -return css_is_ancestor (&memcg->css, &root_memcg->css); +return cgroup_is_descendant (memcg->css.cgroup, root_memcg->css.cgroup); } static bool mem_cgroup_same_or_subtree(const struct mem_cgroup *root_memcg,
Signature · 7 lines
1.8.0.2 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v6 2/5 memcg: convert to use cgroup id
In reply to Li ZefanUse cgroup id instead of css id. This is a preparation to kill css id.
Note, as memcg treat 0 as an invalid id, while cgroup id starts with 0, we define memcg_id == cgroup_id + 1.
--- mm/memcontrol.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-)
mm/memcontrol.c +22 -10
@@ -498,6 +498,23 @@ static inline bool mem_cgroup_is_root(struct mem_cgroup *memcg)return (memcg == root_mem_cgroup); } +static inline unsigned short mem_cgroup_id (struct mem_cgroup *memcg) +{ +/* +* The ID of the root cgroup is 0, but memcg treat 0 as an +* invalid ID, so we return (cgroup_id + 1). +*/ +return memcg->css.cgroup->id +1 ; +} + +static inline struct mem_cgroup *mem_cgroup_from_id (unsigned short id) +{ +struct cgroup_subsys_state *css; + + css =css_from_id (id -1 , &mem_cgroup_subsys); +return mem_cgroup_from_css (css); +} +/* Writing them here to avoid exposing memcg's inner layout */ #if defined(CONFIG_INET ) && defined(CONFIG_MEMCG_KMEM )
@@ -2850,15 +2867,10 @@ static void __mem_cgroup_cancel_local_charge(struct mem_cgroup *memcg,*/ static struct mem_cgroup *mem_cgroup_lookup (unsigned short id) { -struct cgroup_subsys_state *css; -/* ID 0 is unused ID */ if (!id)return NULL ; - css =css_lookup (&mem_cgroup_subsys, id); -if (!css) -return NULL ; -return mem_cgroup_from_css (css); +return mem_cgroup_from_id (id); }struct mem_cgroup *try_get_mem_cgroup_from_page (struct page *page)
@@ -4368,7 +4380,7 @@ mem_cgroup_uncharge_swapcache(struct page *page, swp_entry_t ent, bool swapout)* css_get() was called in uncharge(). */ if (do_swap_account && swapout && memcg) - swap_cgroup_record(ent, css_id(&memcg->css)); + swap_cgroup_record(ent, mem_cgroup_id(memcg)); }#endif
@@ -4420,8 +4432,8 @@ static int mem_cgroup_move_swap_account(swp_entry_t entry, {unsigned short old_id, new_id; - old_id =css_id (&from->css); - new_id =css_id (&to->css); + old_id =mem_cgroup_id (from); + new_id =mem_cgroup_id (to);if (swap_cgroup_cmpxchg (entry, old_id, new_id) == old_id) {mem_cgroup_swap_statistics (from, false);
@@ -6571,7 +6583,7 @@ static enum mc_target_type get_mctgt_type(struct vm_area_struct *vma, }/* There is a swap entry and a page doesn't exist or isn't charged */ if (ent.val && !ret && -css_id (&mc.from->css) ==lookup_swap_cgroup_id (ent)) { +mem_cgroup_id (mc.from) ==lookup_swap_cgroup_id (ent)) { ret =MC_TARGET_SWAP ;if (target) target->ent = ent;
Signature · 7 lines
1.8.0.2 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v6 3/5 memcg: fail to create cgroup if the cgroup id is too big
In reply to Li Zefanmemcg requires the cgroup id to be smaller than 65536.
This is a preparation to kill css id.
--- mm/memcontrol.c | 9 +++++++++ 1 file changed, 9 insertions(+)
mm/memcontrol.c +9 -0
@@ -498,6 +498,12 @@ static inline bool mem_cgroup_is_root(struct mem_cgroup *memcg)return (memcg == root_mem_cgroup); } +/* +* We restrict the id in the range of [1, 65535], so it can fit into +* an unsigned short. +*/ +#define MEM_CGROUP_ID_MAX USHRT_MAX +static inline unsigned short mem_cgroup_id (struct mem_cgroup *memcg) {/*
@@ -6312,6 +6318,9 @@ mem_cgroup_css_online(struct cgroup_subsys_state *css)struct mem_cgroup *parent =mem_cgroup_from_css (css_parent (css));int error =0 ; +if (css->cgroup->id >MEM_CGROUP_ID_MAX ) +return -ENOSPC ; +if (!parent)return 0 ;
Signature · 1 line
1.8.0.2
PATCH v6 4/5 memcg: stop using css id
In reply to Li ZefanNow memcg uses cgroup id instead of css id. Update some comments and set mem_cgroup_subsys->use_id to 0.
--- mm/memcontrol.c | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-)
mm/memcontrol.c +8 -15
@@ -592,16 +592,11 @@ static void disarm_sock_keys(struct mem_cgroup *memcg)#ifdef CONFIG_MEMCG_KMEM /* * This will be the memcg's index in each cache's ->memcg_params->memcg_caches. -* There are two main reasons for not using the css_id for this: -* 1) this works better in sparse environments, where we have a lot of memcgs, -* but only a few kmem-limited. Or also, if we have, for instance, 200 -* memcgs, and none but the 200th is kmem-limited, we'd have to have a -* 200 entry array for that. -* -* 2) In order not to violate the cgroup API, we would like to do all memory -* allocation in ->create(). At that point, we haven't yet allocated the -* css_id. Having a separate index prevents us from messing with the cgroup -* core for this +* The main reason for not using cgroup id for this: +* this works better in sparse environments, where we have a lot of memcgs, +* but only a few kmem-limited. Or also, if we have, for instance, 200 +* memcgs, and none but the 200th is kmem-limited, we'd have to have a +* 200 entry array for that. * * The current size of the caches array is stored in * memcg_limited_groups_array_size. It will double each time we have to
@@ -616,14 +611,14 @@ int memcg_limited_groups_array_size;* cgroups is a reasonable guess. In the future, it could be a parameter or * tunable, but that is strictly not necessary. * -* MAX_SIZE should be as large as the number of css_ids. Ideally, we could get +* MAX_SIZE should be as large as the number of cgrp_ids. Ideally, we could get * this constant directly from cgroup, but it is understandable that this is * better kept as an internal representation in cgroup.c. In any case, the -* css_id space is not getting any smaller, and we don't have to necessarily +* cgrp_id space is not getting any smaller, and we don't have to necessarily * increase ours as well if it increases. */ #define MEMCG_CACHES_MIN_SIZE 4 -#define MEMCG_CACHES_MAX_SIZE 65535 +#define MEMCG_CACHES_MAX_SIZE MEM_CGROUP_ID_MAX/* * A lot of the calls to the cache allocation functions are expected to be
@@ -6215,7 +6210,6 @@ static void __mem_cgroup_free(struct mem_cgroup *memcg)size_t size =memcg_size ();mem_cgroup_remove_from_trees (memcg); -free_css_id (&mem_cgroup_subsys, &memcg->css);for_each_node (node)free_mem_cgroup_per_zone_info (memcg, node);
@@ -7012,7 +7006,6 @@ struct cgroup_subsys mem_cgroup_subsys = { .bind = mem_cgroup_bind, .base_cftypes = mem_cgroup_files, .early_init = 0, - .use_id = 1, };#ifdef CONFIG_MEMCG_SWAP
Signature · 1 line
1.8.0.2
PATCH v6 5/5 cgroup: kill css_id
In reply to Li ZefanThe only user of css_id was memcg, and it has been convered to use cgroup->id, so kill css_id.
--- include/linux/cgroup.h | 37 -------- kernel/cgroup.c | 248 +------------------------------------------------ 2 files changed, 1 insertion(+), 284 deletions(-)
include/linux/cgroup.h +0 -37
@@ -612,11 +612,6 @@ struct cgroup_subsys {int subsys_id;int disabled;int early_init; -/* -* True if this subsys uses ID. ID is not available before cgroup_init() -* (not available in early_init time.) -*/ -bool use_id;/* * If %false, this subsystem is properly hierarchical -
@@ -642,9 +637,6 @@ struct cgroup_subsys {*/ struct cgroupfs_root *root;struct list_head sibling; -/* used when use_id == true */ -struct idr idr; -spinlock_t id_lock;/* list of cftype_sets */ struct list_head cftsets;
@@ -875,35 +867,6 @@ int css_scan_tasks(struct cgroup_subsys_state *css,int cgroup_attach_task_all (struct task_struct *from,struct task_struct *);int cgroup_transfer_tasks (struct cgroup *to,struct cgroup *from); -/* -* CSS ID is ID for cgroup_subsys_state structs under subsys. This only works -* if cgroup_subsys.use_id == true. It can be used for looking up and scanning. -* CSS ID is assigned at cgroup allocation (create) automatically -* and removed when subsys calls free_css_id() function. This is because -* the lifetime of cgroup_subsys_state is subsys's matter. -* -* Looking up and scanning function should be called under rcu_read_lock(). -* Taking cgroup_mutex is not necessary for following calls. -* But the css returned by this routine can be "not populated yet" or "being -* destroyed". The caller should check css and cgroup's status. -*/ - -/* -* Typically Called at ->destroy(), or somewhere the subsys frees -* cgroup_subsys_state. -*/ -void free_css_id (struct cgroup_subsys *ss,struct cgroup_subsys_state *css); - -/* Find a cgroup_subsys_state which has given ID */ - -struct cgroup_subsys_state *css_lookup (struct cgroup_subsys *ss,int id); - -/* Returns true if root is ancestor of cg */ -bool css_is_ancestor (struct cgroup_subsys_state *cg, -const struct cgroup_subsys_state *root); - -/* Get id and depth of css */ -unsigned short css_id (struct cgroup_subsys_state *css);struct cgroup_subsys_state *css_from_dir (struct dentry *dentry,struct cgroup_subsys *ss);
kernel/cgroup.c +1 -247
314 lines not on this page · raw message
Signature · 1 line
1.8.0.2
Tejun Heo
maintainerHello,
Quoted 2 lines · Li Zefan
The whole patchset has been acked and reviewed by Michal and Tejun. Could you merge it into mm tree?
Ah... I really hoped that this had been merged during -rc1 window. Andrew, would it be okay to carry this series through cgroup tree? It doesn't really have much to do with mm proper and it's a PITA to have to keep updating css_id code from cgroup side when it's scheduled to go away. If carried in -mm, it's likely to cause conflicts with ongoing cgroup changes too.
Thanks.
Signature · 1 line
tejun
Tejun Heo
maintainerQuoted 6 lines · Tejun Heo
Hello,
Quoted 2 lines · Li Zefan
The whole patchset has been acked and reviewed by Michal and Tejun. Could you merge it into mm tree?
Quoted 6 lines
Ah... I really hoped that this had been merged during -rc1 window. Andrew, would it be okay to carry this series through cgroup tree? It doesn't really have much to do with mm proper and it's a PITA to have to keep updating
css_idcode from cgroup side when it's scheduled to go away. If carried in -mm, it's likely to cause conflicts with ongoing cgroup changes too.
Also, wasn't this already in -mm during the last devel cycle? ISTR conflicts with it in -mm with other cgroup core changes. Is there any specific reason why this wasn't merged during the merge windw?
Thanks.
Signature · 1 line
tejun
Quoted 7 lines · Tejun Heo
Quoted 6 lines · Tejun Heo
Hello,
Quoted 2 lines · Li Zefan
The whole patchset has been acked and reviewed by Michal and Tejun. Could you merge it into mm tree?
Quoted 6 lines
Quoted 6 lines
Ah... I really hoped that this had been merged during -rc1 window. Andrew, would it be okay to carry this series through cgroup tree? It doesn't really have much to do with mm proper and it's a PITA to have to keep updating
css_idcode from cgroup side when it's scheduled to go away. If carried in -mm, it's likely to cause conflicts with ongoing cgroup changes too.
I would love to see this patchset go through cgroup tree. The changes to memcg is quite small, and as -mm tree is based on -next it won't cause future conflicts.
Quoted 5 lines
Also, wasn't this already in -mm during the last devel cycle? ISTR conflicts with it in -mm with other cgroup core changes. Is there any specific reason why this wasn't merged during the merge windw?
No, it never went into -mm tree... I guess it's because Andrew was too busy and overlooked this patchset?
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Andrew Morton
maintainerQuoted 15 lines · On Tue, 24 Sep 2013 08:32:03 +0800 Li Zefan <[email protected]> wrote:
Quoted 7 lines · Tejun Heo
Quoted 6 lines · Tejun Heo
Hello,
Quoted 2 lines · Li Zefan
The whole patchset has been acked and reviewed by Michal and Tejun. Could you merge it into mm tree?
Quoted 6 lines
Quoted 6 lines
Ah... I really hoped that this had been merged during -rc1 window. Andrew, would it be okay to carry this series through cgroup tree? It doesn't really have much to do with mm proper and it's a PITA to have to keep updating
css_idcode from cgroup side when it's scheduled to go away. If carried in -mm, it's likely to cause conflicts with ongoing cgroup changes too.
Quoted 2 lines
I would love to see this patchset go through cgroup tree. The changes to memcg is quite small,
It seems logical to put this in the cgroup tree as that's where most of the impact occurs.
Quoted 2 lines
and as -mm tree is based on -next it won't cause
future conflicts.
That's no longer the case - I'm staging -mm patches ahead of linux-next now. Except in cases where that's impractical, such as the 3.12 memcg changes which were pretty heavily impacted by cgroups tree changes.
Quoted 8 lines
Quoted 5 lines
Also, wasn't this already in -mm during the last devel cycle? ISTR conflicts with it in -mm with other cgroup core changes. Is there any specific reason why this wasn't merged during the merge windw?
No, it never went into -mm tree... I guess it's because Andrew was too busy and overlooked this patchset?
I'm not sure what happened to the August 7 patchset, actually. I don't often overlook stuff - I'll skip things if the timing is terrible or if the review comments indicate that another version is coming. But none of that seems to be the case here. hmm...
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Tejun Heo
maintainerHello, Andrew.
Quoted 5 lines · Andrew Morton
Quoted 2 lines
I would love to see this patchset go through cgroup tree. The changes to memcg is quite small,
It seems logical to put this in the cgroup tree as that's where most of the impact occurs.
Cool, applying the changes to cgroup/for-3.13.
Quoted 6 lines
Quoted 2 lines
and as -mm tree is based on -next it won't cause
future conflicts.That's no longer the case - I'm staging -mm patches ahead of linux-next now. Except in cases where that's impractical, such as the 3.12 memcg changes which were pretty heavily impacted by cgroups tree changes.
Please note that cgroup is likely to continue to go through a lot of changes for the foreseeable future and memcg is likely to be affected heavily.
Thanks.
Signature · 7 lines
tejun -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Tejun Heo
maintainer(cc'ing Stephen, hi!)
Quoted 10 lines · Tejun Heo
Hello, Andrew.
Quoted 5 lines · Andrew Morton
Quoted 2 lines
I would love to see this patchset go through cgroup tree. The changes to memcg is quite small,
It seems logical to put this in the cgroup tree as that's where most of the impact occurs.
Cool, applying the changes to cgroup/for-3.13.
Stephen, Andrew, cgroup/for-3.13 will cause a minor conflict in mm/memcontrol.c with the patch which reverts Michal's reclaim changes.
static void __mem_cgroup_free(struct mem_cgroup *memcg)
{
int node;
size_t size = memcg_size(); <<<<<<< HEAD
=======
mem_cgroup_remove_from_trees(memcg); free_css_id(&mem_cgroup_subsys, &memcg->css);
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
1fa8f71dfa6e28c89afad7ac71dcb19b8c8da8b7
for_each_node(node) free_mem_cgroup_per_zone_info(memcg, node);
It's a context conflict and just removing free_css_id() call resolves it.
static void __mem_cgroup_free(struct mem_cgroup *memcg)
{
int node;
size_t size = memcg_size();
mem_cgroup_remove_from_trees(memcg);
for_each_node(node)
free_mem_cgroup_per_zone_info(memcg, node);Thanks.
Signature · 7 lines
tejun -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Hi Tejun,
Quoted 2 lines · On Mon, 23 Sep 2013 21:52:11 -0400 Tejun Heo…
(cc'ing Stephen, hi!)
Hi :-)
Quoted 32 lines · Tejun Heo
Quoted 9 lines · Andrew Morton
Quoted 5 lines
Quoted 2 lines
I would love to see this patchset go through cgroup tree. The changes to memcg is quite small,
It seems logical to put this in the cgroup tree as that's where most of the impact occurs.
Cool, applying the changes to cgroup/for-3.13.
Stephen, Andrew, cgroup/for-3.13 will cause a minor conflict in
mm/memcontrol.cwith the patch which reverts Michal's reclaim changes.static void
__mem_cgroup_free(struct mem_cgroup *memcg){ int node; size_t size = memcg_size();<<<<<<< HEAD
=======mem_cgroup_remove_from_trees(memcg); free_css_id(&mem_cgroup_subsys, &memcg->css);Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
1fa8f71dfa6e28c89afad7ac71dcb19b8c8da8b7for_each_node(node) free_mem_cgroup_per_zone_info(memcg, node);It's a context conflict and just removing
free_css_id()call resolves it.static void
__mem_cgroup_free(struct mem_cgroup *memcg)
Quoted 8 lines
{
int node;
size_t size = memcg_size();
mem_cgroup_remove_from_trees(memcg);
for_each_node(node)
free_mem_cgroup_per_zone_info(memcg, node);Thanks for the heads up, I guess I'll see that tomorrow.
Signature · 2 lines
Cheers, Stephen Rothwell [email protected]
Quoted 9 lines · Li Zefan
Hi Andrew,
The whole patchset has been acked and reviewed by Michal and Tejun. Could you merge it into mm tree?
===========
This patchset converts memcg to use
cgroup->id, and then we remove cgroupcss_id.
Applied to cgroup/for-3.13.
Thanks.
Signature · 7 lines
tejun -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"[email protected]"> [email protected] </a>