On Wed, Apr 08, 2009 at 05:33:01PM +0900, Ken'ichi Ohmichi wrote:
>
> Hi,
>
> I tested a cgrlesengd daemon with huge load, which makes many 'su',
> in long time. And the daemon was killed by an OOM killer. So the
> daemon has memory leak. This patch fixes this problem.
>
> The daemon allocates memory at cg_prepare_cgroup(), but it does not
> free the memory. This patch adds necessary free() to cgroup_change_
> cgroup_path by calling cgroup_free_controllers(). In addition, this
> patch adds free()s for handling error and flushes the counters of the
> allocations in cgroup_free_controllers().
>
>
> Thanks
> Ken'ichi Ohmichi
>
> Signed-off-by: Ken'ichi Ohmichi <[email protected]>
> ---
> diff -rpuN a/src/api.c b/src/api.c
> --- a/src/api.c 2009-04-01 14:59:19.000000000 +0900
> +++ b/src/api.c 2009-04-08 17:47:27.000000000 +0900
> @@ -1715,6 +1715,7 @@ unlock_error:
> * XX: Need to figure out how to cleanup? Cleanup just the stuff
> * we added, or the whole structure.
> */
> + cgroup_free_controllers(cgroup);
This is wrong. We still leave a leak in. We need a
cgroup_free(&cgroup)
will free the entire cgroup. (Needed esp since I plan to move the
char name[] to char *name in the structure in the future).
> cgroup = NULL;
> return error;
> }
> @@ -1760,6 +1761,7 @@ static int cg_prepare_cgroup(struct cgro
> " failed\n",
> cg_mount_table[i].name);
>
> pthread_rwlock_unlock(&cg_mount_table_lock);
> + cgroup_free_controllers(cgroup);
> return ECGROUPNOTALLOWED;
> }
> }
> @@ -1773,6 +1775,7 @@ static int cg_prepare_cgroup(struct cgro
> if (!cptr) {
> cgroup_dbg("Adding controller '%s' failed\n",
> controller);
> + cgroup_free_controllers(cgroup);
> return ECGROUPNOTALLOWED;
> }
> }
> @@ -1991,11 +1994,10 @@ int cgroup_change_cgroup_path(char *dest
> return ret;
> /* Add task to cgroup */
> ret = cgroup_attach_task_pid(&cgroup, pid);
> - if (ret) {
> + if (ret)
> cgroup_dbg("cgroup_attach_task_pid failed:%d\n", ret);
> - return ret;
> - }
> - return 0;
> + cgroup_free_controllers(&cgroup);
> + return ret;
> }
>
> /**
> diff -rpuN a/src/wrapper.c b/src/wrapper.c
> --- a/src/wrapper.c 2009-04-01 14:59:19.000000000 +0900
> +++ b/src/wrapper.c 2009-04-08 17:47:22.000000000 +0900
> @@ -83,8 +83,10 @@ void cgroup_free_controllers(struct cgro
> for (i = 0; i < cgroup->index; i++) {
> for (j = 0; j < cgroup->controller[i]->index; j++)
> free(cgroup->controller[i]->values[j]);
> + cgroup->controller[i]->index = 0;
> free(cgroup->controller[i]);
> }
> + cgroup->index = 0;
I don't like this change. I don't want external users to call this
function. It has to be limited within the library. Balbir, what do you
say about it? (I do agree though the change is right.)
thanks,
--
regards,
Dhaval
------------------------------------------------------------------------------
This SF.net email is sponsored by:
High Quality Requirements in a Collaborative Environment.
Download a free trial of Rational Requirements Composer Now!
http://p.sf.net/sfu/www-ibm-com
_______________________________________________
Libcg-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libcg-devel
