From d5b1fe68baa7213f198e5be8cd1a1037258ab2c8 Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Fri, 28 Dec 2012 13:18:28 -0800 Subject: cgroup: remove unused dummy cgroup_fork_callbacks() 5edee61ede ("cgroup: cgroup_subsys->fork() should be called after the task is added to css_set") removed cgroup_fork_callbacks() but forgot to remove its dummy version for !CONFIG_CGROUPS. Remove it. Signed-off-by: Tejun Heo Reported-by: Herton Ronaldo Krzesinski diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h index 7d73905..942e687 100644 --- a/include/linux/cgroup.h +++ b/include/linux/cgroup.h @@ -706,7 +706,6 @@ struct cgroup_subsys_state *cgroup_css_from_dir(struct file *f, int id); static inline int cgroup_init_early(void) { return 0; } static inline int cgroup_init(void) { return 0; } static inline void cgroup_fork(struct task_struct *p) {} -static inline void cgroup_fork_callbacks(struct task_struct *p) {} static inline void cgroup_post_fork(struct task_struct *p) {} static inline void cgroup_exit(struct task_struct *p, int callbacks) {} -- cgit v0.10.2 From 12a9d2fef1d35770d3cdc2cd1faabb83c45bc0fa Mon Sep 17 00:00:00 2001 From: Tejun Heo Date: Mon, 7 Jan 2013 08:49:33 -0800 Subject: cgroup: implement cgroup_rightmost_descendant() Implement cgroup_rightmost_descendant() which returns the right most descendant of the specified cgroup. This can be used to skip the cgroup's subtree while iterating with cgroup_for_each_descendant_pre(). Signed-off-by: Tejun Heo Acked-by: Michal Hocko Acked-by: Li Zefan diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h index 942e687..8118a31 100644 --- a/include/linux/cgroup.h +++ b/include/linux/cgroup.h @@ -558,6 +558,7 @@ static inline struct cgroup* task_cgroup(struct task_struct *task, struct cgroup *cgroup_next_descendant_pre(struct cgroup *pos, struct cgroup *cgroup); +struct cgroup *cgroup_rightmost_descendant(struct cgroup *pos); /** * cgroup_for_each_descendant_pre - pre-order walk of a cgroup's descendants diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 4855892..6643f70 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -3017,6 +3017,32 @@ struct cgroup *cgroup_next_descendant_pre(struct cgroup *pos, } EXPORT_SYMBOL_GPL(cgroup_next_descendant_pre); +/** + * cgroup_rightmost_descendant - return the rightmost descendant of a cgroup + * @pos: cgroup of interest + * + * Return the rightmost descendant of @pos. If there's no descendant, + * @pos is returned. This can be used during pre-order traversal to skip + * subtree of @pos. + */ +struct cgroup *cgroup_rightmost_descendant(struct cgroup *pos) +{ + struct cgroup *last, *tmp; + + WARN_ON_ONCE(!rcu_read_lock_held()); + + do { + last = pos; + /* ->prev isn't RCU safe, walk ->next till the end */ + pos = NULL; + list_for_each_entry_rcu(tmp, &last->children, sibling) + pos = tmp; + } while (pos); + + return last; +} +EXPORT_SYMBOL_GPL(cgroup_rightmost_descendant); + static struct cgroup *cgroup_leftmost_descendant(struct cgroup *pos) { struct cgroup *last; -- cgit v0.10.2 From 92e015b1cfc24e3cb072385f25171b8599cc7ef3 Mon Sep 17 00:00:00 2001 From: Greg Thelen Date: Fri, 4 Jan 2013 13:05:17 -0800 Subject: cgroups: move cgroup_event_listener.c to tools/cgroup Move the cgroup_event_listener.c tool from Documentation into the new tools/cgroup directory. This change involves wiring cgroup_event_listener.c into the tools/ make system so that is can be built with: $ make tools/cgroup Signed-off-by: Greg Thelen Signed-off-by: Tejun Heo diff --git a/Documentation/cgroups/00-INDEX b/Documentation/cgroups/00-INDEX index f78b90a..f5635a0 100644 --- a/Documentation/cgroups/00-INDEX +++ b/Documentation/cgroups/00-INDEX @@ -4,8 +4,6 @@ blkio-controller.txt - Description for Block IO Controller, implementation and usage details. cgroups.txt - Control Groups definition, implementation details, examples and API. -cgroup_event_listener.c - - A user program for cgroup listener. cpuacct.txt - CPU Accounting Controller; account CPU usage for groups of tasks. cpusets.txt diff --git a/Documentation/cgroups/cgroup_event_listener.c b/Documentation/cgroups/cgroup_event_listener.c deleted file mode 100644 index 3e082f9..0000000 --- a/Documentation/cgroups/cgroup_event_listener.c +++ /dev/null @@ -1,110 +0,0 @@ -/* - * cgroup_event_listener.c - Simple listener of cgroup events - * - * Copyright (C) Kirill A. Shutemov - */ - -#include -#include -#include -#include -#include -#include -#include -#include - -#include - -#define USAGE_STR "Usage: cgroup_event_listener \n" - -int main(int argc, char **argv) -{ - int efd = -1; - int cfd = -1; - int event_control = -1; - char event_control_path[PATH_MAX]; - char line[LINE_MAX]; - int ret; - - if (argc != 3) { - fputs(USAGE_STR, stderr); - return 1; - } - - cfd = open(argv[1], O_RDONLY); - if (cfd == -1) { - fprintf(stderr, "Cannot open %s: %s\n", argv[1], - strerror(errno)); - goto out; - } - - ret = snprintf(event_control_path, PATH_MAX, "%s/cgroup.event_control", - dirname(argv[1])); - if (ret >= PATH_MAX) { - fputs("Path to cgroup.event_control is too long\n", stderr); - goto out; - } - - event_control = open(event_control_path, O_WRONLY); - if (event_control == -1) { - fprintf(stderr, "Cannot open %s: %s\n", event_control_path, - strerror(errno)); - goto out; - } - - efd = eventfd(0, 0); - if (efd == -1) { - perror("eventfd() failed"); - goto out; - } - - ret = snprintf(line, LINE_MAX, "%d %d %s", efd, cfd, argv[2]); - if (ret >= LINE_MAX) { - fputs("Arguments string is too long\n", stderr); - goto out; - } - - ret = write(event_control, line, strlen(line) + 1); - if (ret == -1) { - perror("Cannot write to cgroup.event_control"); - goto out; - } - - while (1) { - uint64_t result; - - ret = read(efd, &result, sizeof(result)); - if (ret == -1) { - if (errno == EINTR) - continue; - perror("Cannot read from eventfd"); - break; - } - assert(ret == sizeof(result)); - - ret = access(event_control_path, W_OK); - if ((ret == -1) && (errno == ENOENT)) { - puts("The cgroup seems to have removed."); - ret = 0; - break; - } - - if (ret == -1) { - perror("cgroup.event_control " - "is not accessible any more"); - break; - } - - printf("%s %s: crossed\n", argv[1], argv[2]); - } - -out: - if (efd >= 0) - close(efd); - if (event_control >= 0) - close(event_control); - if (cfd >= 0) - close(cfd); - - return (ret != 0); -} diff --git a/Documentation/cgroups/memcg_test.txt b/Documentation/cgroups/memcg_test.txt index fc8fa97..ce94a83 100644 --- a/Documentation/cgroups/memcg_test.txt +++ b/Documentation/cgroups/memcg_test.txt @@ -399,8 +399,7 @@ Under below explanation, we assume CONFIG_MEM_RES_CTRL_SWAP=y. 9.10 Memory thresholds Memory controller implements memory thresholds using cgroups notification - API. You can use Documentation/cgroups/cgroup_event_listener.c to test - it. + API. You can use tools/cgroup/cgroup_event_listener.c to test it. (Shell-A) Create cgroup and run event listener # mkdir /cgroup/A diff --git a/tools/Makefile b/tools/Makefile index 1f9a529..fc57a28 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -3,6 +3,7 @@ include scripts/Makefile.include help: @echo 'Possible targets:' @echo '' + @echo ' cgroup - cgroup tools' @echo ' cpupower - a tool for all things x86 CPU power' @echo ' firewire - the userspace part of nosy, an IEEE-1394 traffic sniffer' @echo ' lguest - a minimal 32-bit x86 hypervisor' @@ -33,7 +34,7 @@ help: cpupower: FORCE $(call descend,power/$@) -firewire lguest perf usb virtio vm: FORCE +cgroup firewire lguest perf usb virtio vm: FORCE $(call descend,$@) selftests: FORCE @@ -45,7 +46,7 @@ turbostat x86_energy_perf_policy: FORCE cpupower_install: $(call descend,power/$(@:_install=),install) -firewire_install lguest_install perf_install usb_install virtio_install vm_install: +cgroup_install firewire_install lguest_install perf_install usb_install virtio_install vm_install: $(call descend,$(@:_install=),install) selftests_install: @@ -54,14 +55,14 @@ selftests_install: turbostat_install x86_energy_perf_policy_install: $(call descend,power/x86/$(@:_install=),install) -install: cpupower_install firewire_install lguest_install perf_install \ - selftests_install turbostat_install usb_install virtio_install \ - vm_install x86_energy_perf_policy_install +install: cgroup_install cpupower_install firewire_install lguest_install \ + perf_install selftests_install turbostat_install usb_install \ + virtio_install vm_install x86_energy_perf_policy_install cpupower_clean: $(call descend,power/cpupower,clean) -firewire_clean lguest_clean perf_clean usb_clean virtio_clean vm_clean: +cgroup_clean firewire_clean lguest_clean perf_clean usb_clean virtio_clean vm_clean: $(call descend,$(@:_clean=),clean) selftests_clean: @@ -70,8 +71,8 @@ selftests_clean: turbostat_clean x86_energy_perf_policy_clean: $(call descend,power/x86/$(@:_clean=),clean) -clean: cpupower_clean firewire_clean lguest_clean perf_clean selftests_clean \ - turbostat_clean usb_clean virtio_clean vm_clean \ - x86_energy_perf_policy_clean +clean: cgroup_clean cpupower_clean firewire_clean lguest_clean perf_clean \ + selftests_clean turbostat_clean usb_clean virtio_clean \ + vm_clean x86_energy_perf_policy_clean .PHONY: FORCE diff --git a/tools/cgroup/.gitignore b/tools/cgroup/.gitignore new file mode 100644 index 0000000..633cd9b --- /dev/null +++ b/tools/cgroup/.gitignore @@ -0,0 +1 @@ +cgroup_event_listener diff --git a/tools/cgroup/Makefile b/tools/cgroup/Makefile new file mode 100644 index 0000000..b428619 --- /dev/null +++ b/tools/cgroup/Makefile @@ -0,0 +1,11 @@ +# Makefile for cgroup tools + +CC = $(CROSS_COMPILE)gcc +CFLAGS = -Wall -Wextra + +all: cgroup_event_listener +%: %.c + $(CC) $(CFLAGS) -o $@ $^ + +clean: + $(RM) cgroup_event_listener diff --git a/tools/cgroup/cgroup_event_listener.c b/tools/cgroup/cgroup_event_listener.c new file mode 100644 index 0000000..3e082f9 --- /dev/null +++ b/tools/cgroup/cgroup_event_listener.c @@ -0,0 +1,110 @@ +/* + * cgroup_event_listener.c - Simple listener of cgroup events + * + * Copyright (C) Kirill A. Shutemov + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#include + +#define USAGE_STR "Usage: cgroup_event_listener \n" + +int main(int argc, char **argv) +{ + int efd = -1; + int cfd = -1; + int event_control = -1; + char event_control_path[PATH_MAX]; + char line[LINE_MAX]; + int ret; + + if (argc != 3) { + fputs(USAGE_STR, stderr); + return 1; + } + + cfd = open(argv[1], O_RDONLY); + if (cfd == -1) { + fprintf(stderr, "Cannot open %s: %s\n", argv[1], + strerror(errno)); + goto out; + } + + ret = snprintf(event_control_path, PATH_MAX, "%s/cgroup.event_control", + dirname(argv[1])); + if (ret >= PATH_MAX) { + fputs("Path to cgroup.event_control is too long\n", stderr); + goto out; + } + + event_control = open(event_control_path, O_WRONLY); + if (event_control == -1) { + fprintf(stderr, "Cannot open %s: %s\n", event_control_path, + strerror(errno)); + goto out; + } + + efd = eventfd(0, 0); + if (efd == -1) { + perror("eventfd() failed"); + goto out; + } + + ret = snprintf(line, LINE_MAX, "%d %d %s", efd, cfd, argv[2]); + if (ret >= LINE_MAX) { + fputs("Arguments string is too long\n", stderr); + goto out; + } + + ret = write(event_control, line, strlen(line) + 1); + if (ret == -1) { + perror("Cannot write to cgroup.event_control"); + goto out; + } + + while (1) { + uint64_t result; + + ret = read(efd, &result, sizeof(result)); + if (ret == -1) { + if (errno == EINTR) + continue; + perror("Cannot read from eventfd"); + break; + } + assert(ret == sizeof(result)); + + ret = access(event_control_path, W_OK); + if ((ret == -1) && (errno == ENOENT)) { + puts("The cgroup seems to have removed."); + ret = 0; + break; + } + + if (ret == -1) { + perror("cgroup.event_control " + "is not accessible any more"); + break; + } + + printf("%s %s: crossed\n", argv[1], argv[2]); + } + +out: + if (efd >= 0) + close(efd); + if (event_control >= 0) + close(event_control); + if (cfd >= 0) + close(cfd); + + return (ret != 0); +} -- cgit v0.10.2 From 799105d514384b80cbe3182dbcb4ed30aa07e1f5 Mon Sep 17 00:00:00 2001 From: Greg Thelen Date: Mon, 7 Jan 2013 11:50:17 -0800 Subject: cgroups: fix cgroup_event_listener error handling The error handling in cgroup_event_listener.c did not correctly deal with either an error opening either or cgroup.event_control. Due to an uninitialized variable the program exit code was undefined if either of these opens failed. This patch simplifies and corrects cgroup_event_listener.c error handling by: 1. using err*() rather than printf(),exit() 2. depending on process exit to close open files With this patch failures always return non-zero error. Signed-off-by: Greg Thelen Acked-by: Li Zefan Signed-off-by: Tejun Heo diff --git a/tools/cgroup/cgroup_event_listener.c b/tools/cgroup/cgroup_event_listener.c index 3e082f9..4eb5507 100644 --- a/tools/cgroup/cgroup_event_listener.c +++ b/tools/cgroup/cgroup_event_listener.c @@ -5,6 +5,7 @@ */ #include +#include #include #include #include @@ -15,7 +16,7 @@ #include -#define USAGE_STR "Usage: cgroup_event_listener \n" +#define USAGE_STR "Usage: cgroup_event_listener " int main(int argc, char **argv) { @@ -26,49 +27,33 @@ int main(int argc, char **argv) char line[LINE_MAX]; int ret; - if (argc != 3) { - fputs(USAGE_STR, stderr); - return 1; - } + if (argc != 3) + errx(1, "%s", USAGE_STR); cfd = open(argv[1], O_RDONLY); - if (cfd == -1) { - fprintf(stderr, "Cannot open %s: %s\n", argv[1], - strerror(errno)); - goto out; - } + if (cfd == -1) + err(1, "Cannot open %s", argv[1]); ret = snprintf(event_control_path, PATH_MAX, "%s/cgroup.event_control", dirname(argv[1])); - if (ret >= PATH_MAX) { - fputs("Path to cgroup.event_control is too long\n", stderr); - goto out; - } + if (ret >= PATH_MAX) + errx(1, "Path to cgroup.event_control is too long"); event_control = open(event_control_path, O_WRONLY); - if (event_control == -1) { - fprintf(stderr, "Cannot open %s: %s\n", event_control_path, - strerror(errno)); - goto out; - } + if (event_control == -1) + err(1, "Cannot open %s", event_control_path); efd = eventfd(0, 0); - if (efd == -1) { - perror("eventfd() failed"); - goto out; - } + if (efd == -1) + err(1, "eventfd() failed"); ret = snprintf(line, LINE_MAX, "%d %d %s", efd, cfd, argv[2]); - if (ret >= LINE_MAX) { - fputs("Arguments string is too long\n", stderr); - goto out; - } + if (ret >= LINE_MAX) + errx(1, "Arguments string is too long"); ret = write(event_control, line, strlen(line) + 1); - if (ret == -1) { - perror("Cannot write to cgroup.event_control"); - goto out; - } + if (ret == -1) + err(1, "Cannot write to cgroup.event_control"); while (1) { uint64_t result; @@ -77,34 +62,21 @@ int main(int argc, char **argv) if (ret == -1) { if (errno == EINTR) continue; - perror("Cannot read from eventfd"); - break; + err(1, "Cannot read from eventfd"); } assert(ret == sizeof(result)); ret = access(event_control_path, W_OK); if ((ret == -1) && (errno == ENOENT)) { - puts("The cgroup seems to have removed."); - ret = 0; - break; - } - - if (ret == -1) { - perror("cgroup.event_control " - "is not accessible any more"); + puts("The cgroup seems to have removed."); break; } + if (ret == -1) + err(1, "cgroup.event_control is not accessible any more"); + printf("%s %s: crossed\n", argv[1], argv[2]); } -out: - if (efd >= 0) - close(efd); - if (event_control >= 0) - close(event_control); - if (cfd >= 0) - close(cfd); - - return (ret != 0); + return 0; } -- cgit v0.10.2 From 0ac801fe07374148714b5ef53df90ac5b1673c0c Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Thu, 10 Jan 2013 11:49:27 +0800 Subject: cgroup: use new hashtable implementation Switch cgroup to use the new hashtable implementation. No functional changes. Signed-off-by: Li Zefan Signed-off-by: Tejun Heo diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 6643f70..54b3908 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -52,7 +52,7 @@ #include #include #include -#include +#include #include #include #include @@ -376,22 +376,18 @@ static int css_set_count; * account cgroups in empty hierarchies. */ #define CSS_SET_HASH_BITS 7 -#define CSS_SET_TABLE_SIZE (1 << CSS_SET_HASH_BITS) -static struct hlist_head css_set_table[CSS_SET_TABLE_SIZE]; +static DEFINE_HASHTABLE(css_set_table, CSS_SET_HASH_BITS); -static struct hlist_head *css_set_hash(struct cgroup_subsys_state *css[]) +static unsigned long css_set_hash(struct cgroup_subsys_state *css[]) { int i; - int index; - unsigned long tmp = 0UL; + unsigned long key = 0UL; for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) - tmp += (unsigned long)css[i]; - tmp = (tmp >> 16) ^ tmp; + key += (unsigned long)css[i]; + key = (key >> 16) ^ key; - index = hash_long(tmp, CSS_SET_HASH_BITS); - - return &css_set_table[index]; + return key; } /* We don't maintain the lists running through each css_set to its @@ -418,7 +414,7 @@ static void __put_css_set(struct css_set *cg, int taskexit) } /* This css_set is dead. unlink it and release cgroup refcounts */ - hlist_del(&cg->hlist); + hash_del(&cg->hlist); css_set_count--; list_for_each_entry_safe(link, saved_link, &cg->cg_links, @@ -550,9 +546,9 @@ static struct css_set *find_existing_css_set( { int i; struct cgroupfs_root *root = cgrp->root; - struct hlist_head *hhead; struct hlist_node *node; struct css_set *cg; + unsigned long key; /* * Build the set of subsystem state objects that we want to see in the @@ -572,8 +568,8 @@ static struct css_set *find_existing_css_set( } } - hhead = css_set_hash(template); - hlist_for_each_entry(cg, node, hhead, hlist) { + key = css_set_hash(template); + hash_for_each_possible(css_set_table, cg, node, hlist, key) { if (!compare_css_sets(cg, oldcg, cgrp, template)) continue; @@ -657,8 +653,8 @@ static struct css_set *find_css_set( struct list_head tmp_cg_links; - struct hlist_head *hhead; struct cg_cgroup_link *link; + unsigned long key; /* First see if we already have a cgroup group that matches * the desired set */ @@ -704,8 +700,8 @@ static struct css_set *find_css_set( css_set_count++; /* Add this cgroup group to the hash table */ - hhead = css_set_hash(res->subsys); - hlist_add_head(&res->hlist, hhead); + key = css_set_hash(res->subsys); + hash_add(css_set_table, &res->hlist, key); write_unlock(&css_set_lock); @@ -1597,6 +1593,8 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type, struct cgroupfs_root *existing_root; const struct cred *cred; int i; + struct hlist_node *node; + struct css_set *cg; BUG_ON(sb->s_root != NULL); @@ -1650,14 +1648,8 @@ static struct dentry *cgroup_mount(struct file_system_type *fs_type, /* Link the top cgroup in this hierarchy into all * the css_set objects */ write_lock(&css_set_lock); - for (i = 0; i < CSS_SET_TABLE_SIZE; i++) { - struct hlist_head *hhead = &css_set_table[i]; - struct hlist_node *node; - struct css_set *cg; - - hlist_for_each_entry(cg, node, hhead, hlist) - link_css_set(&tmp_cg_links, cg, root_cgrp); - } + hash_for_each(css_set_table, i, node, cg, hlist) + link_css_set(&tmp_cg_links, cg, root_cgrp); write_unlock(&css_set_lock); free_cg_links(&tmp_cg_links); @@ -4464,6 +4456,9 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss) { struct cgroup_subsys_state *css; int i, ret; + struct hlist_node *node, *tmp; + struct css_set *cg; + unsigned long key; /* check name and function validity */ if (ss->name == NULL || strlen(ss->name) > MAX_CGROUP_TYPE_NAMELEN || @@ -4529,23 +4524,17 @@ int __init_or_module cgroup_load_subsys(struct cgroup_subsys *ss) * this is all done under the css_set_lock. */ write_lock(&css_set_lock); - for (i = 0; i < CSS_SET_TABLE_SIZE; i++) { - struct css_set *cg; - struct hlist_node *node, *tmp; - struct hlist_head *bucket = &css_set_table[i], *new_bucket; - - hlist_for_each_entry_safe(cg, node, tmp, bucket, hlist) { - /* skip entries that we already rehashed */ - if (cg->subsys[ss->subsys_id]) - continue; - /* remove existing entry */ - hlist_del(&cg->hlist); - /* set new value */ - cg->subsys[ss->subsys_id] = css; - /* recompute hash and restore entry */ - new_bucket = css_set_hash(cg->subsys); - hlist_add_head(&cg->hlist, new_bucket); - } + hash_for_each_safe(css_set_table, i, node, tmp, cg, hlist) { + /* skip entries that we already rehashed */ + if (cg->subsys[ss->subsys_id]) + continue; + /* remove existing entry */ + hash_del(&cg->hlist); + /* set new value */ + cg->subsys[ss->subsys_id] = css; + /* recompute hash and restore entry */ + key = css_set_hash(cg->subsys); + hash_add(css_set_table, node, key); } write_unlock(&css_set_lock); @@ -4577,7 +4566,6 @@ EXPORT_SYMBOL_GPL(cgroup_load_subsys); void cgroup_unload_subsys(struct cgroup_subsys *ss) { struct cg_cgroup_link *link; - struct hlist_head *hhead; BUG_ON(ss->module == NULL); @@ -4611,11 +4599,12 @@ void cgroup_unload_subsys(struct cgroup_subsys *ss) write_lock(&css_set_lock); list_for_each_entry(link, &dummytop->css_sets, cgrp_link_list) { struct css_set *cg = link->cg; + unsigned long key; - hlist_del(&cg->hlist); + hash_del(&cg->hlist); cg->subsys[ss->subsys_id] = NULL; - hhead = css_set_hash(cg->subsys); - hlist_add_head(&cg->hlist, hhead); + key = css_set_hash(cg->subsys); + hash_add(css_set_table, &cg->hlist, key); } write_unlock(&css_set_lock); @@ -4657,9 +4646,6 @@ int __init cgroup_init_early(void) list_add(&init_css_set_link.cg_link_list, &init_css_set.cg_links); - for (i = 0; i < CSS_SET_TABLE_SIZE; i++) - INIT_HLIST_HEAD(&css_set_table[i]); - for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) { struct cgroup_subsys *ss = subsys[i]; @@ -4693,7 +4679,7 @@ int __init cgroup_init(void) { int err; int i; - struct hlist_head *hhead; + unsigned long key; err = bdi_init(&cgroup_backing_dev_info); if (err) @@ -4712,8 +4698,8 @@ int __init cgroup_init(void) } /* Add init_css_set to the hash table */ - hhead = css_set_hash(init_css_set.subsys); - hlist_add_head(&init_css_set.hlist, hhead); + key = css_set_hash(init_css_set.subsys); + hash_add(css_set_table, &init_css_set.hlist, key); BUG_ON(!init_root_id(&rootnode)); cgroup_kobj = kobject_create_and_add("cgroup", fs_kobj); -- cgit v0.10.2 From 5d65bc0ca1bceb73204dab943922ba3c83276a8c Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Mon, 14 Jan 2013 17:23:26 +0800 Subject: cgroup: remove synchronize_rcu() from cgroup_attach_{task|proc}() These 2 syncronize_rcu()s make attaching a task to a cgroup quite slow, and it can't be ignored in some situations. A real case from Colin Cross: Android uses cgroups heavily to manage thread priorities, putting threads in a background group with reduced cpu.shares when they are not visible to the user, and in a foreground group when they are. Some RPCs from foreground threads to background threads will temporarily move the background thread into the foreground group for the duration of the RPC. This results in many calls to cgroup_attach_task. In cgroup_attach_task() it's task->cgroups that is protected by RCU, and put_css_set() calls kfree_rcu() to free it. If we remove this synchronize_rcu(), there can be threads in RCU-read sections accessing their old cgroup via current->cgroups with concurrent rmdir operation, but this is safe. # time for ((i=0; i<50; i++)) { echo $$ > /mnt/sub/tasks; echo $$ > /mnt/tasks; } real 0m2.524s user 0m0.008s sys 0m0.004s With this patch: real 0m0.004s user 0m0.004s sys 0m0.000s tj: These synchronize_rcu()s are utterly confused. synchornize_rcu() necessarily has to come between two operations to guarantee that the changes made by the former operation are visible to all rcu readers before proceeding to the latter operation. Here, synchornize_rcu() are at the end of attach operations with nothing beyond it. Its only effect would be delaying completion of write(2) to sysfs tasks/procs files until all rcu readers see the change, which doesn't mean anything. Signed-off-by: Li Zefan Signed-off-by: Tejun Heo Reported-by: Colin Cross diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 54b3908..ce27351 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -1974,7 +1974,6 @@ int cgroup_attach_task(struct cgroup *cgrp, struct task_struct *tsk) ss->attach(cgrp, &tset); } - synchronize_rcu(); out: if (retval) { for_each_subsys(root, ss) { @@ -2143,7 +2142,6 @@ static int cgroup_attach_proc(struct cgroup *cgrp, struct task_struct *leader) /* * step 5: success! and cleanup */ - synchronize_rcu(); retval = 0; out_put_css_set_refs: if (retval) { -- cgit v0.10.2 From 130e3695a3edf6bf21464f2826720a79a6afdee0 Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Mon, 14 Jan 2013 17:24:18 +0800 Subject: cgroup: remove synchronize_rcu() from rebind_subsystems() Nothing's protected by RCU in rebind_subsystems(), and I can't think of a reason why it is needed. Signed-off-by: Li Zefan Signed-off-by: Tejun Heo diff --git a/kernel/cgroup.c b/kernel/cgroup.c index ce27351..a893985 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -1079,7 +1079,6 @@ static int rebind_subsystems(struct cgroupfs_root *root, } } root->subsys_mask = root->actual_subsys_mask = final_subsys_mask; - synchronize_rcu(); return 0; } -- cgit v0.10.2 From 2739d3cce9816805fe26774fea2527d5b16e924d Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Mon, 21 Jan 2013 18:18:33 +0800 Subject: cgroup: fix bogus kernel warnings when cgroup_create() failed If cgroup_create() failed and cgroup_destroy_locked() is called to do cleanup, we'll see a bunch of warnings: cgroup_addrm_files: failed to remove 2MB.limit_in_bytes, err=-2 cgroup_addrm_files: failed to remove 2MB.usage_in_bytes, err=-2 cgroup_addrm_files: failed to remove 2MB.max_usage_in_bytes, err=-2 cgroup_addrm_files: failed to remove 2MB.failcnt, err=-2 cgroup_addrm_files: failed to remove prioidx, err=-2 cgroup_addrm_files: failed to remove ifpriomap, err=-2 ... We failed to remove those files, because cgroup_create() has failed before creating those cgroup files. To fix this, we simply don't warn if cgroup_rm_file() can't find the cft entry. Signed-off-by: Li Zefan Signed-off-by: Tejun Heo diff --git a/kernel/cgroup.c b/kernel/cgroup.c index a893985..ad3359f 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -921,13 +921,17 @@ static void remove_dir(struct dentry *d) dput(parent); } -static int cgroup_rm_file(struct cgroup *cgrp, const struct cftype *cft) +static void cgroup_rm_file(struct cgroup *cgrp, const struct cftype *cft) { struct cfent *cfe; lockdep_assert_held(&cgrp->dentry->d_inode->i_mutex); lockdep_assert_held(&cgroup_mutex); + /* + * If we're doing cleanup due to failure of cgroup_create(), + * the corresponding @cfe may not exist. + */ list_for_each_entry(cfe, &cgrp->files, node) { struct dentry *d = cfe->dentry; @@ -940,9 +944,8 @@ static int cgroup_rm_file(struct cgroup *cgrp, const struct cftype *cft) list_del_init(&cfe->node); dput(d); - return 0; + break; } - return -ENOENT; } /** @@ -2758,14 +2761,14 @@ static int cgroup_addrm_files(struct cgroup *cgrp, struct cgroup_subsys *subsys, if ((cft->flags & CFTYPE_ONLY_ON_ROOT) && cgrp->parent) continue; - if (is_add) + if (is_add) { err = cgroup_add_file(cgrp, subsys, cft); - else - err = cgroup_rm_file(cgrp, cft); - if (err) { - pr_warning("cgroup_addrm_files: failed to %s %s, err=%d\n", - is_add ? "add" : "remove", cft->name, err); + if (err) + pr_warn("cgroup_addrm_files: failed to add %s, err=%d\n", + cft->name, err); ret = err; + } else { + cgroup_rm_file(cgrp, cft); } } return ret; -- cgit v0.10.2 From b5d646f5d5a135064232ff3a140a47a5b84bc911 Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Thu, 24 Jan 2013 14:43:51 +0800 Subject: cgroup: remove a NULL check in cgroup_exit() init_task.cgroups is initialized at boot phase, and whenver a ask is forked, it's cgroups pointer is inherited from its parent, and it's never set to NULL afterwards. Signed-off-by: Li Zefan Signed-off-by: Tejun Heo diff --git a/kernel/cgroup.c b/kernel/cgroup.c index ad3359f..8da9048 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -4994,8 +4994,7 @@ void cgroup_exit(struct task_struct *tsk, int run_callbacks) } task_unlock(tsk); - if (cg) - put_css_set_taskexit(cg); + put_css_set_taskexit(cg); } /** -- cgit v0.10.2 From fe1c06ca7523baa668c1eaf1e1016fa64753c32e Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Thu, 24 Jan 2013 14:30:22 +0800 Subject: cgroup: initialize cgrp->dentry before css_alloc() With this change, we're guaranteed that cgroup_path() won't see NULL cgrp->dentry, and thus we can remove the NULL check in it. (Well, it's not strictly true, because dummptop.dentry is always NULL but we already handle that separately.) Signed-off-by: Li Zefan Signed-off-by: Tejun Heo diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 8da9048..a049322 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -1767,7 +1767,7 @@ int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen) rcu_lockdep_assert(rcu_read_lock_held() || cgroup_lock_is_held(), "cgroup_path() called without proper locking"); - if (!dentry || cgrp == dummytop) { + if (cgrp == dummytop) { /* * Inactive subsystems have no dentry for their root * cgroup @@ -4153,6 +4153,9 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry, init_cgroup_housekeeping(cgrp); + dentry->d_fsdata = cgrp; + cgrp->dentry = dentry; + cgrp->parent = parent; cgrp->root = parent->root; cgrp->top_cgroup = parent->top_cgroup; @@ -4190,8 +4193,6 @@ static long cgroup_create(struct cgroup *parent, struct dentry *dentry, lockdep_assert_held(&dentry->d_inode->i_mutex); /* allocation complete, commit to creation */ - dentry->d_fsdata = cgrp; - cgrp->dentry = dentry; list_add_tail(&cgrp->allcg_node, &root->allcg_list); list_add_tail_rcu(&cgrp->sibling, &cgrp->parent->children); root->number_of_cgroups++; -- cgit v0.10.2 From ace783b9bbfa2182b4a561498db3f09a0c56bc79 Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Thu, 24 Jan 2013 14:30:48 +0800 Subject: sched: split out css_online/css_offline from tg creation/destruction This is a preparaton for later patches. - What do we gain from cpu_cgroup_css_online(): After ss->css_alloc() and before ss->css_online(), there's a small window that tg->css.cgroup is NULL. With this change, tg won't be seen before ss->css_online(), where it's added to the global list, so we're guaranteed we'll never see NULL tg->css.cgroup. - What do we gain from cpu_cgroup_css_offline(): tg is freed via RCU, so is cgroup. Without this change, This is how synchronization works: cgroup_rmdir() no ss->css_offline() diput() syncornize_rcu() ss->css_free() <-- unregister tg, and free it via call_rcu() kfree_rcu(cgroup) <-- wait possible refs to cgroup, and free cgroup We can't just kfree(cgroup), because tg might access tg->css.cgroup. With this change: cgroup_rmdir() ss->css_offline() <-- unregister tg diput() synchronize_rcu() <-- wait possible refs to tg and cgroup ss->css_free() <-- free tg kfree_rcu(cgroup) <-- free cgroup As you see, kfree_rcu() is redundant now. Signed-off-by: Li Zefan Signed-off-by: Tejun Heo Acked-by: Ingo Molnar diff --git a/include/linux/sched.h b/include/linux/sched.h index 206bb08..577eb97 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -2750,7 +2750,10 @@ extern void normalize_rt_tasks(void); extern struct task_group root_task_group; extern struct task_group *sched_create_group(struct task_group *parent); +extern void sched_online_group(struct task_group *tg, + struct task_group *parent); extern void sched_destroy_group(struct task_group *tg); +extern void sched_offline_group(struct task_group *tg); extern void sched_move_task(struct task_struct *tsk); #ifdef CONFIG_FAIR_GROUP_SCHED extern int sched_group_set_shares(struct task_group *tg, unsigned long shares); diff --git a/kernel/sched/auto_group.c b/kernel/sched/auto_group.c index 0984a21..64de5f8 100644 --- a/kernel/sched/auto_group.c +++ b/kernel/sched/auto_group.c @@ -35,6 +35,7 @@ static inline void autogroup_destroy(struct kref *kref) ag->tg->rt_se = NULL; ag->tg->rt_rq = NULL; #endif + sched_offline_group(ag->tg); sched_destroy_group(ag->tg); } @@ -76,6 +77,8 @@ static inline struct autogroup *autogroup_create(void) if (IS_ERR(tg)) goto out_free; + sched_online_group(tg, &root_task_group); + kref_init(&ag->kref); init_rwsem(&ag->lock); ag->id = atomic_inc_return(&autogroup_seq_nr); diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 257002c..1061672 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -7159,7 +7159,6 @@ static void free_sched_group(struct task_group *tg) struct task_group *sched_create_group(struct task_group *parent) { struct task_group *tg; - unsigned long flags; tg = kzalloc(sizeof(*tg), GFP_KERNEL); if (!tg) @@ -7171,6 +7170,17 @@ struct task_group *sched_create_group(struct task_group *parent) if (!alloc_rt_sched_group(tg, parent)) goto err; + return tg; + +err: + free_sched_group(tg); + return ERR_PTR(-ENOMEM); +} + +void sched_online_group(struct task_group *tg, struct task_group *parent) +{ + unsigned long flags; + spin_lock_irqsave(&task_group_lock, flags); list_add_rcu(&tg->list, &task_groups); @@ -7180,12 +7190,6 @@ struct task_group *sched_create_group(struct task_group *parent) INIT_LIST_HEAD(&tg->children); list_add_rcu(&tg->siblings, &parent->children); spin_unlock_irqrestore(&task_group_lock, flags); - - return tg; - -err: - free_sched_group(tg); - return ERR_PTR(-ENOMEM); } /* rcu callback to free various structures associated with a task group */ @@ -7198,6 +7202,12 @@ static void free_sched_group_rcu(struct rcu_head *rhp) /* Destroy runqueue etc associated with a task group */ void sched_destroy_group(struct task_group *tg) { + /* wait for possible concurrent references to cfs_rqs complete */ + call_rcu(&tg->rcu, free_sched_group_rcu); +} + +void sched_offline_group(struct task_group *tg) +{ unsigned long flags; int i; @@ -7209,9 +7219,6 @@ void sched_destroy_group(struct task_group *tg) list_del_rcu(&tg->list); list_del_rcu(&tg->siblings); spin_unlock_irqrestore(&task_group_lock, flags); - - /* wait for possible concurrent references to cfs_rqs complete */ - call_rcu(&tg->rcu, free_sched_group_rcu); } /* change task's runqueue when it moves between groups. @@ -7563,6 +7570,19 @@ static struct cgroup_subsys_state *cpu_cgroup_css_alloc(struct cgroup *cgrp) return &tg->css; } +static int cpu_cgroup_css_online(struct cgroup *cgrp) +{ + struct task_group *tg = cgroup_tg(cgrp); + struct task_group *parent; + + if (!cgrp->parent) + return 0; + + parent = cgroup_tg(cgrp->parent); + sched_online_group(tg, parent); + return 0; +} + static void cpu_cgroup_css_free(struct cgroup *cgrp) { struct task_group *tg = cgroup_tg(cgrp); @@ -7570,6 +7590,13 @@ static void cpu_cgroup_css_free(struct cgroup *cgrp) sched_destroy_group(tg); } +static void cpu_cgroup_css_offline(struct cgroup *cgrp) +{ + struct task_group *tg = cgroup_tg(cgrp); + + sched_offline_group(tg); +} + static int cpu_cgroup_can_attach(struct cgroup *cgrp, struct cgroup_taskset *tset) { @@ -7925,6 +7952,8 @@ struct cgroup_subsys cpu_cgroup_subsys = { .name = "cpu", .css_alloc = cpu_cgroup_css_alloc, .css_free = cpu_cgroup_css_free, + .css_online = cpu_cgroup_css_online, + .css_offline = cpu_cgroup_css_offline, .can_attach = cpu_cgroup_can_attach, .attach = cpu_cgroup_attach, .exit = cpu_cgroup_exit, -- cgit v0.10.2 From 2a73991b76cbd38c4a0c6704449ccc08c89c3ff3 Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Thu, 24 Jan 2013 14:31:11 +0800 Subject: sched: remove redundant NULL cgroup check in task_group_path() A task_group won't be online (thus no one can see it) until cpu_cgroup_css_online(), and at that time tg->css.cgroup has been initialized, so this NULL check is redundant. Signed-off-by: Li Zefan Signed-off-by: Tejun Heo diff --git a/kernel/sched/debug.c b/kernel/sched/debug.c index 2cd3c1b..38df0db 100644 --- a/kernel/sched/debug.c +++ b/kernel/sched/debug.c @@ -110,13 +110,6 @@ static char *task_group_path(struct task_group *tg) if (autogroup_path(tg, group_path, PATH_MAX)) return group_path; - /* - * May be NULL if the underlying cgroup isn't fully-created yet - */ - if (!tg->css.cgroup) { - group_path[0] = '\0'; - return group_path; - } cgroup_path(tg->css.cgroup, group_path, PATH_MAX); return group_path; } -- cgit v0.10.2 From 86a3db5643c7d29bb36ca85c7a4bb67ad4d88d77 Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Thu, 24 Jan 2013 14:31:27 +0800 Subject: cgroup: remove duplicate RCU free on struct cgroup When destroying a cgroup, though in cgroup_diput() we've called synchronize_rcu(), we then still have to free it via call_rcu(). The story is, long ago to fix a race between reading /proc/sched_debug and freeing cgroup, the code was changed to utilize call_rcu(). See commit a47295e6bc42ad35f9c15ac66f598aa24debd4e2 ("cgroups: make cgroup_path() RCU-safe") As we've fixed cpu cgroup that cpu_cgroup_offline_css() is used to unregister a task_group so there won't be concurrent access to this task_group after synchronize_rcu() in diput(). Now we can just kfree(cgrp). Signed-off-by: Li Zefan Signed-off-by: Tejun Heo diff --git a/kernel/cgroup.c b/kernel/cgroup.c index a049322..af99391 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -892,7 +892,7 @@ static void cgroup_diput(struct dentry *dentry, struct inode *inode) simple_xattrs_free(&cgrp->xattrs); ida_simple_remove(&cgrp->root->cgroup_ida, cgrp->id); - kfree_rcu(cgrp, rcu_head); + kfree(cgrp); } else { struct cfent *cfe = __d_cfe(dentry); struct cgroup *cgrp = dentry->d_parent->d_fsdata; -- cgit v0.10.2 From be44562613851235d801d41d5b3976dc4333f622 Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Thu, 24 Jan 2013 14:31:42 +0800 Subject: cgroup: remove synchronize_rcu() from cgroup_diput() Free cgroup via call_rcu(). The actual work is done through workqueue. Signed-off-by: Li Zefan Signed-off-by: Tejun Heo diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h index 8118a31..900af59 100644 --- a/include/linux/cgroup.h +++ b/include/linux/cgroup.h @@ -203,6 +203,7 @@ struct cgroup { /* For RCU-protected deletion */ struct rcu_head rcu_head; + struct work_struct free_work; /* List of events which userspace want to receive */ struct list_head event_list; diff --git a/kernel/cgroup.c b/kernel/cgroup.c index af99391..02e4f20 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -852,12 +852,52 @@ static struct inode *cgroup_new_inode(umode_t mode, struct super_block *sb) return inode; } +static void cgroup_free_fn(struct work_struct *work) +{ + struct cgroup *cgrp = container_of(work, struct cgroup, free_work); + struct cgroup_subsys *ss; + + mutex_lock(&cgroup_mutex); + /* + * Release the subsystem state objects. + */ + for_each_subsys(cgrp->root, ss) + ss->css_free(cgrp); + + cgrp->root->number_of_cgroups--; + mutex_unlock(&cgroup_mutex); + + /* + * Drop the active superblock reference that we took when we + * created the cgroup + */ + deactivate_super(cgrp->root->sb); + + /* + * if we're getting rid of the cgroup, refcount should ensure + * that there are no pidlists left. + */ + BUG_ON(!list_empty(&cgrp->pidlists)); + + simple_xattrs_free(&cgrp->xattrs); + + ida_simple_remove(&cgrp->root->cgroup_ida, cgrp->id); + kfree(cgrp); +} + +static void cgroup_free_rcu(struct rcu_head *head) +{ + struct cgroup *cgrp = container_of(head, struct cgroup, rcu_head); + + schedule_work(&cgrp->free_work); +} + static void cgroup_diput(struct dentry *dentry, struct inode *inode) { /* is dentry a directory ? if so, kfree() associated cgroup */ if (S_ISDIR(inode->i_mode)) { struct cgroup *cgrp = dentry->d_fsdata; - struct cgroup_subsys *ss; + BUG_ON(!(cgroup_is_removed(cgrp))); /* It's possible for external users to be holding css * reference counts on a cgroup; css_put() needs to @@ -865,34 +905,7 @@ static void cgroup_diput(struct dentry *dentry, struct inode *inode) * the reference count in order to know if it needs to * queue the cgroup to be handled by the release * agent */ - synchronize_rcu(); - - mutex_lock(&cgroup_mutex); - /* - * Release the subsystem state objects. - */ - for_each_subsys(cgrp->root, ss) - ss->css_free(cgrp); - - cgrp->root->number_of_cgroups--; - mutex_unlock(&cgroup_mutex); - - /* - * Drop the active superblock reference that we took when we - * created the cgroup - */ - deactivate_super(cgrp->root->sb); - - /* - * if we're getting rid of the cgroup, refcount should ensure - * that there are no pidlists left. - */ - BUG_ON(!list_empty(&cgrp->pidlists)); - - simple_xattrs_free(&cgrp->xattrs); - - ida_simple_remove(&cgrp->root->cgroup_ida, cgrp->id); - kfree(cgrp); + call_rcu(&cgrp->rcu_head, cgroup_free_rcu); } else { struct cfent *cfe = __d_cfe(dentry); struct cgroup *cgrp = dentry->d_parent->d_fsdata; @@ -1391,6 +1404,7 @@ static void init_cgroup_housekeeping(struct cgroup *cgrp) INIT_LIST_HEAD(&cgrp->allcg_node); INIT_LIST_HEAD(&cgrp->release_list); INIT_LIST_HEAD(&cgrp->pidlists); + INIT_WORK(&cgrp->free_work, cgroup_free_fn); mutex_init(&cgrp->pidlist_mutex); INIT_LIST_HEAD(&cgrp->event_list); spin_lock_init(&cgrp->event_list_lock); -- cgit v0.10.2 From 9ed8a659703876a9fe96ab86d1b296c2f0084242 Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Thu, 24 Jan 2013 14:32:02 +0800 Subject: cgroup: remove bogus comments in cgroup_diput() Since commit 48ddbe194623ae089cc0576e60363f2d2e85662a ("cgroup: make css->refcnt clearing on cgroup removal optional"), each css holds a ref on cgroup's dentry, so cgroup_diput() won't be called until all css' refs go down to 0, which invalids the comments. Signed-off-by: Li Zefan Signed-off-by: Tejun Heo diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 02e4f20..8008522 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -899,12 +899,6 @@ static void cgroup_diput(struct dentry *dentry, struct inode *inode) struct cgroup *cgrp = dentry->d_fsdata; BUG_ON(!(cgroup_is_removed(cgrp))); - /* It's possible for external users to be holding css - * reference counts on a cgroup; css_put() needs to - * be able to access the cgroup after decrementing - * the reference count in order to know if it needs to - * queue the cgroup to be handled by the release - * agent */ call_rcu(&cgrp->rcu_head, cgroup_free_rcu); } else { struct cfent *cfe = __d_cfe(dentry); -- cgit v0.10.2 From 71b5707e119653039e6e95213f00479668c79b75 Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Thu, 24 Jan 2013 14:43:28 +0800 Subject: cgroup: fix exit() vs rmdir() race In cgroup_exit() put_css_set_taskexit() is called without any lock, which might lead to accessing a freed cgroup: thread1 thread2 --------------------------------------------- exit() cgroup_exit() put_css_set_taskexit() atomic_dec(cgrp->count); rmdir(); /* not safe !! */ check_for_release(cgrp); rcu_read_lock() can be used to make sure the cgroup is alive. Signed-off-by: Li Zefan Signed-off-by: Tejun Heo Cc: stable@vger.kernel.org diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 8008522..5d4c92e 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -422,12 +422,20 @@ static void __put_css_set(struct css_set *cg, int taskexit) struct cgroup *cgrp = link->cgrp; list_del(&link->cg_link_list); list_del(&link->cgrp_link_list); + + /* + * We may not be holding cgroup_mutex, and if cgrp->count is + * dropped to 0 the cgroup can be destroyed at any time, hence + * rcu_read_lock is used to keep it alive. + */ + rcu_read_lock(); if (atomic_dec_and_test(&cgrp->count) && notify_on_release(cgrp)) { if (taskexit) set_bit(CGRP_RELEASABLE, &cgrp->flags); check_for_release(cgrp); } + rcu_read_unlock(); kfree(link); } -- cgit v0.10.2 From 63f43f55c9bbc14f76b582644019b8a07dc8219a Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Fri, 25 Jan 2013 16:08:01 +0800 Subject: cpuset: fix cpuset_print_task_mems_allowed() vs rename() race rename() will change dentry->d_name. The result of this race can be worse than seeing partially rewritten name, but we might access a stale pointer because rename() will re-allocate memory to hold a longer name. It's safe in the protection of dentry->d_lock. v2: check NULL dentry before acquiring dentry lock. Signed-off-by: Li Zefan Signed-off-by: Tejun Heo Cc: stable@vger.kernel.org diff --git a/kernel/cpuset.c b/kernel/cpuset.c index 7bb63ee..5bb9bf1 100644 --- a/kernel/cpuset.c +++ b/kernel/cpuset.c @@ -2511,8 +2511,16 @@ void cpuset_print_task_mems_allowed(struct task_struct *tsk) dentry = task_cs(tsk)->css.cgroup->dentry; spin_lock(&cpuset_buffer_lock); - snprintf(cpuset_name, CPUSET_NAME_LEN, - dentry ? (const char *)dentry->d_name.name : "/"); + + if (!dentry) { + strcpy(cpuset_name, "/"); + } else { + spin_lock(&dentry->d_lock); + strlcpy(cpuset_name, (const char *)dentry->d_name.name, + CPUSET_NAME_LEN); + spin_unlock(&dentry->d_lock); + } + nodelist_scnprintf(cpuset_nodelist, CPUSET_NODELIST_LEN, tsk->mems_allowed); printk(KERN_INFO "%s cpuset=%s mems_allowed=%s\n", -- cgit v0.10.2 From 810cbee4fad570ff167132d4ecf247d99c48f71d Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Mon, 18 Feb 2013 18:56:14 +0800 Subject: cgroup: fix cgroup_rmdir() vs close(eventfd) race commit 205a872bd6f9a9a09ef035ef1e90185a8245cc58 ("cgroup: fix lockdep warning for event_control") solved a deadlock by introducing a new bug. Move cgrp->event_list to a temporary list doesn't mean you can traverse this list locklessly, because at the same time cgroup_event_wake() can be called and remove the event from the list. The result of this race is disastrous. We adopt the way how kvm irqfd code implements race-free event removal, which is now described in the comments in cgroup_event_wake(). v3: - call eventfd_signal() no matter it's eventfd close or cgroup removal that removes the cgroup event. Acked-by: Kirill A. Shutemov Signed-off-by: Li Zefan Signed-off-by: Tejun Heo diff --git a/kernel/cgroup.c b/kernel/cgroup.c index 5d4c92e..feda814 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -3786,8 +3786,13 @@ static void cgroup_event_remove(struct work_struct *work) remove); struct cgroup *cgrp = event->cgrp; + remove_wait_queue(event->wqh, &event->wait); + event->cft->unregister_event(cgrp, event->cft, event->eventfd); + /* Notify userspace the event is going away. */ + eventfd_signal(event->eventfd, 1); + eventfd_ctx_put(event->eventfd); kfree(event); dput(cgrp->dentry); @@ -3807,15 +3812,25 @@ static int cgroup_event_wake(wait_queue_t *wait, unsigned mode, unsigned long flags = (unsigned long)key; if (flags & POLLHUP) { - __remove_wait_queue(event->wqh, &event->wait); - spin_lock(&cgrp->event_list_lock); - list_del_init(&event->list); - spin_unlock(&cgrp->event_list_lock); /* - * We are in atomic context, but cgroup_event_remove() may - * sleep, so we have to call it in workqueue. + * If the event has been detached at cgroup removal, we + * can simply return knowing the other side will cleanup + * for us. + * + * We can't race against event freeing since the other + * side will require wqh->lock via remove_wait_queue(), + * which we hold. */ - schedule_work(&event->remove); + spin_lock(&cgrp->event_list_lock); + if (!list_empty(&event->list)) { + list_del_init(&event->list); + /* + * We are in atomic context, but cgroup_event_remove() + * may sleep, so we have to call it in workqueue. + */ + schedule_work(&event->remove); + } + spin_unlock(&cgrp->event_list_lock); } return 0; @@ -4375,20 +4390,14 @@ static int cgroup_destroy_locked(struct cgroup *cgrp) /* * Unregister events and notify userspace. * Notify userspace about cgroup removing only after rmdir of cgroup - * directory to avoid race between userspace and kernelspace. Use - * a temporary list to avoid a deadlock with cgroup_event_wake(). Since - * cgroup_event_wake() is called with the wait queue head locked, - * remove_wait_queue() cannot be called while holding event_list_lock. + * directory to avoid race between userspace and kernelspace. */ spin_lock(&cgrp->event_list_lock); - list_splice_init(&cgrp->event_list, &tmp_list); - spin_unlock(&cgrp->event_list_lock); - list_for_each_entry_safe(event, tmp, &tmp_list, list) { + list_for_each_entry_safe(event, tmp, &cgrp->event_list, list) { list_del_init(&event->list); - remove_wait_queue(event->wqh, &event->wait); - eventfd_signal(event->eventfd, 1); schedule_work(&event->remove); } + spin_unlock(&cgrp->event_list_lock); return 0; } -- cgit v0.10.2 From f169007b2773f285e098cb84c74aac0154d65ff7 Mon Sep 17 00:00:00 2001 From: Li Zefan Date: Mon, 18 Feb 2013 14:13:35 +0800 Subject: cgroup: fail if monitored file and event_control are in different cgroup If we pass fd of memory.usage_in_bytes of cgroup A to cgroup.event_control of cgroup B, then we won't get memory usage notification from A but B! What's worse, if A and B are in different mount hierarchy, we'll end up accessing NULL pointer! Disallow this kind of invalid usage. Signed-off-by: Li Zefan Acked-by: Kirill A. Shutemov Signed-off-by: Tejun Heo diff --git a/kernel/cgroup.c b/kernel/cgroup.c index feda814..b5c6432 100644 --- a/kernel/cgroup.c +++ b/kernel/cgroup.c @@ -3856,6 +3856,7 @@ static int cgroup_write_event_control(struct cgroup *cgrp, struct cftype *cft, const char *buffer) { struct cgroup_event *event = NULL; + struct cgroup *cgrp_cfile; unsigned int efd, cfd; struct file *efile = NULL; struct file *cfile = NULL; @@ -3911,6 +3912,16 @@ static int cgroup_write_event_control(struct cgroup *cgrp, struct cftype *cft, goto fail; } + /* + * The file to be monitored must be in the same cgroup as + * cgroup.event_control is. + */ + cgrp_cfile = __d_cgrp(cfile->f_dentry->d_parent); + if (cgrp_cfile != cgrp) { + ret = -EINVAL; + goto fail; + } + if (!event->cft->register_event || !event->cft->unregister_event) { ret = -EINVAL; goto fail; -- cgit v0.10.2