From e9b0b26d8bf46719b05e8ebf7bccb829ba1cec93 Mon Sep 17 00:00:00 2001 From: Ken'ichi Ohmichi Date: Tue, 7 Apr 2009 14:02:24 +0900 Subject: Fix infinite loop if receiving a NLMSG_NOOP packet. Hi, I tested 'cgred' service and I saw the problem that some processes are not moved to a right cgroup. This problem did not occur always, and it did sometimes. I reviewed cgrulesengd.c and found the bug cgrulesengd stays in an infinite loop if receiving a NLMSG_NOOP packet. This patch fixes this problem. Signed-off-by: Ken'ichi Ohmichi Acked-by: Balbir Singh Signed-off-by: Dhaval Giani --- src/daemon/cgrulesengd.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/daemon/cgrulesengd.c b/src/daemon/cgrulesengd.c index 8efdce1..4e00e18 100644 --- a/src/daemon/cgrulesengd.c +++ b/src/daemon/cgrulesengd.c @@ -386,8 +386,10 @@ int cgre_create_netlink_socket_process_msg() continue; while (NLMSG_OK(nlh, recv_len)) { cn_hdr = NLMSG_DATA(nlh); - if (nlh->nlmsg_type == NLMSG_NOOP) + if (nlh->nlmsg_type == NLMSG_NOOP) { + nlh = NLMSG_NEXT(nlh, recv_len); continue; + } if ((nlh->nlmsg_type == NLMSG_ERROR) || (nlh->nlmsg_type == NLMSG_OVERRUN)) break; -- cgit From 973dac482d4a46386ad14602df2cf37470258238 Mon Sep 17 00:00:00 2001 From: Ken'ichi Ohmichi Date: Mon, 13 Apr 2009 09:47:36 +0900 Subject: Fix the lacks of pthread_rwlock_unlock() calls. Two pthread_rwlock_unlock() calls are necessary if *cgroup is null and fprintf()/fflush() fails in cgroup_attach_task_pid(): src/api.c:785 785 pthread_rwlock_rdlock(&cg_mount_table_lock); 786 for(i = 0; i < CG_CONTROLLER_MAX && 787 cg_mount_table[i].name[0]!='\0'; i++) { [snip] 805 ret = fprintf(tasks, "%d", tid); 806 if (ret < 0) { <> 807 cgroup_dbg("Error writing tid %d to %s:%s\n", 808 tid, path, strerror(errno)); 809 fclose(tasks); 810 last_errno = errno; 811 return ECGOTHER; 812 } 813 814 ret = fflush(tasks); 815 if (ret) { <> 816 last_errno = errno; 817 cgroup_dbg("Error writing tid %d to %s:%s\n", 818 tid, path, strerror(errno)); 819 fclose(tasks); 820 return ECGOTHER; 821 } For the readability, this patch merges almost the same lines into one function(__cgroup_attach_task_pid()) and adds pthread_rwlock_unlock() call for the case the function fails. Signed-off-by: Ken'ichi Ohmichi Signed-off-by: Dhaval Giani --- src/api.c | 106 +++++++++++++++++++++++++------------------------------------- 1 file changed, 43 insertions(+), 63 deletions(-) (limited to 'src') diff --git a/src/api.c b/src/api.c index a45802e..99e71fe 100644 --- a/src/api.c +++ b/src/api.c @@ -762,6 +762,43 @@ char *cg_build_path(char *name, char *path, char *type) return path; } +static int __cgroup_attach_task_pid(char *path, pid_t tid) +{ + int ret = 0; + FILE *tasks = NULL; + + tasks = fopen(path, "w"); + if (!tasks) { + switch (errno) { + case EPERM: + return ECGROUPNOTOWNER; + case ENOENT: + return ECGROUPNOTEXIST; + default: + return ECGROUPNOTALLOWED; + } + } + ret = fprintf(tasks, "%d", tid); + if (ret < 0) { + last_errno = errno; + ret = ECGOTHER; + goto err; + } + ret = fflush(tasks); + if (ret) { + last_errno = errno; + ret = ECGOTHER; + goto err; + } + fclose(tasks); + return 0; +err: + cgroup_dbg("Error writing tid %d to %s:%s\n", + tid, path, strerror(errno)); + fclose(tasks); + return ret; +} + /** cgroup_attach_task_pid is used to assign tasks to a cgroup. * struct cgroup *cgroup: The cgroup to assign the thread to. * pid_t tid: The thread to be assigned to the cgroup. @@ -773,7 +810,6 @@ char *cg_build_path(char *name, char *path, char *type) int cgroup_attach_task_pid(struct cgroup *cgroup, pid_t tid) { char path[FILENAME_MAX]; - FILE *tasks = NULL; int i, ret = 0; if (!cgroup_initialized) { @@ -789,37 +825,11 @@ int cgroup_attach_task_pid(struct cgroup *cgroup, pid_t tid) cg_mount_table[i].name)) continue; strncat(path, "/tasks", sizeof(path) - strlen(path)); - - tasks = fopen(path, "w"); - if (!tasks) { - pthread_rwlock_unlock(&cg_mount_table_lock); - switch (errno) { - case EPERM: - return ECGROUPNOTOWNER; - case ENOENT: - return ECGROUPNOTEXIST; - default: - return ECGROUPNOTALLOWED; - } - } - ret = fprintf(tasks, "%d", tid); - if (ret < 0) { - cgroup_dbg("Error writing tid %d to %s:%s\n", - tid, path, strerror(errno)); - fclose(tasks); - last_errno = errno; - return ECGOTHER; - } - - ret = fflush(tasks); + ret = __cgroup_attach_task_pid(path, tid); if (ret) { - last_errno = errno; - cgroup_dbg("Error writing tid %d to %s:%s\n", - tid, path, strerror(errno)); - fclose(tasks); - return ECGOTHER; + pthread_rwlock_unlock(&cg_mount_table_lock); + return ret; } - fclose(tasks); } pthread_rwlock_unlock(&cg_mount_table_lock); } else { @@ -835,40 +845,10 @@ int cgroup_attach_task_pid(struct cgroup *cgroup, pid_t tid) if (!cg_build_path(cgroup->name, path, cgroup->controller[i]->name)) continue; - strncat(path, "/tasks", sizeof(path) - strlen(path)); - - tasks = fopen(path, "w"); - if (!tasks) { - cgroup_dbg("fopen failed for %s:%s", path, - strerror(errno)); - - switch (errno) { - case EPERM: - return ECGROUPNOTOWNER; - case ENOENT: - return ECGROUPNOTEXIST; - default: - return ECGROUPNOTALLOWED; - } - } - ret = fprintf(tasks, "%d", tid); - if (ret < 0) { - last_errno = errno; - cgroup_dbg("Error writing tid %d to %s:%s\n", - tid, path, strerror(errno)); - fclose(tasks); - return ECGOTHER; - } - ret = fflush(tasks); - if (ret) { - last_errno = errno; - cgroup_dbg("Error writing tid %d to %s:%s\n", - tid, path, strerror(errno)); - fclose(tasks); - return ECGOTHER; - } - fclose(tasks); + ret = __cgroup_attach_task_pid(path, tid); + if (ret) + return ret; } } return 0; -- cgit From 8d8a747b161508ebfd48a6dac52f1438e2714a2b Mon Sep 17 00:00:00 2001 From: Bharata B Rao Date: Mon, 13 Apr 2009 15:49:06 +0530 Subject: Fix a few compilation warnings in api.c gcc -DHAVE_CONFIG_H -I. -I.. -I../include -g -O2 -Wall -MT api.lo -MD -MP -MF .deps/api.Tpo -c api.c -fPIC -DPIC -o .libs/api.o api.c:52:1: warning: "VERSION" redefined In file included from ./libcgroup-internal.h:21, from api.c:31: ../config.h:129:1: warning: this is the location of the previous definition api.c: In function 'cgroup_parse_rules': api.c:353: warning: implicit declaration of function 'isblank' api.c: In function 'cgroup_modify_cgroup': api.c:1073: warning: implicit declaration of function 'asprintf' This patch fixes the warning arising due to isblank, asprintf and getline. Signed-off-by: Bharata B Rao Signed-off-by: Dhaval Giani --- src/api.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src') diff --git a/src/api.c b/src/api.c index 99e71fe..4d5b524 100644 --- a/src/api.c +++ b/src/api.c @@ -25,6 +25,10 @@ * for mistakes in APIs for reading statistics. */ +#ifndef _GNU_SOURCE +#define _GNU_SOURCE +#endif + #include #include #include -- cgit