On 07/26/2010 10:49 AM, Dhaval Giani wrote:
> Split cgroup_init so that cgroup_init_state will get the table and
> store it in the local state. The next step will be separate out the
> APIs to use state and finally we will introduce a global state.
>
> Signed-off-by: Dhaval Giani<[email protected]>
>
> ---
> include/Makefile.am | 2 -
> include/libcgroup.h | 3 +
> include/libcgroup/deprecated.h | 26 ++++++++++++++++
> include/libcgroup/init.h | 10 ++----
> src/Makefile.am | 2 -
> src/api.c | 66
> ++++++++++++++++++++++++-----------------
> src/deprecated-api.c | 66
> +++++++++++++++++++++++++++++++++++++++++
> src/libcgroup-internal.h | 7 ++++
> src/libcgroup.map | 2 +
> 9 files changed, 148 insertions(+), 36 deletions(-)
>
> Index: libcg/src/api.c
> ===================================================================
> --- libcg.orig/src/api.c
> +++ libcg/src/api.c
> @@ -65,9 +65,6 @@ static __thread char errtext[MAXLEN];
> /* Task command name length */
> #define TASK_COMM_LEN 16
>
> -/* Check if cgroup_init has been called or not. */
> -static int cgroup_initialized;
> -
> /* Check if the rules cache has been loaded or not. */
> static bool cgroup_rules_loaded;
>
> @@ -80,12 +77,6 @@ static struct cgroup_rule_list trl;
> /* Lock for the list of rules (rl) */
> static pthread_rwlock_t rl_lock = PTHREAD_RWLOCK_INITIALIZER;
>
> -/* Namespace */
> -__thread char *cg_namespace_table[CG_CONTROLLER_MAX];
> -
> -pthread_rwlock_t cg_mount_table_lock = PTHREAD_RWLOCK_INITIALIZER;
> -struct cg_mount_table_s cg_mount_table[CG_CONTROLLER_MAX];
> -
> const char const *cgroup_strerror_codes[] = {
> "Cgroup is not compiled in",
> "Cgroup is not mounted",
> @@ -637,15 +628,23 @@ unlock:
> return ret;
> }
>
> -/**
> - * cgroup_init(), initializes the MOUNT_POINT.
> - *
> - * This code is theoretically thread safe now. Its not really tested
> - * so it can blow up. If does for you, please let us know with your
> - * test case and we can really make it thread safe.
> - *
> - */
> -int cgroup_init(void)
> +void cgroup_free_context(struct cgroup_context_s **cg_context)
> +{
> + int i;
> + struct cgroup_context_s *context = *cg_context;
> +
> + for (i = 0; i< context->size; i++) {
> + if (context->cg_namespace[i]) {
> + free(context->cg_namespace[i]);
> + context->cg_namespace[i] = NULL;
> + }
> + }
> +
> + free(context);
> + *cg_context = NULL;
> +}
> +
> +int cgroup_init_context(struct cgroup_context_s **cgroup_context)
> {
> FILE *proc_mount = NULL;
> struct mntent *ent = NULL;
> @@ -665,7 +664,17 @@ int cgroup_init(void)
> char mntent_buffer[4 * FILENAME_MAX];
> char *strtok_buffer = NULL;
>
> - pthread_rwlock_wrlock(&cg_mount_table_lock);
> + struct cgroup_context_s *cg_context = calloc(1,
> + sizeof(struct cgroup_context_s));
> +
> + if (!cg_context) {
> + last_errno = errno;
> + ret = ECGOTHER;
> + goto unlock_exit;
> + }
> +
> + for (i = 0; i< CG_CONTROLLER_MAX; i++)
> + cg_context->cg_namespace[i] = NULL;
>
> proc_cgroup = fopen("/proc/cgroups", "re");
>
> @@ -695,6 +704,7 @@ int cgroup_init(void)
> }
> free(buf);
>
> + i = 0;
> while (!feof(proc_cgroup)) {
> err = fscanf(proc_cgroup, "%s %d %d %d", subsys_name,
> &hierarchy,&num_cgroups,&enabled);
> @@ -742,19 +752,20 @@ int cgroup_init(void)
> duplicate = 0;
> for (j = 0; j< found_mnt; j++) {
> if (strncmp(mntopt, cg_mount_table[j].name,
> - FILENAME_MAX) == 0) {
> + FILENAME_MAX) == 0) {
> duplicate = 1;
> break;
> }
> }
> if (duplicate) {
> cgroup_dbg("controller %s is already mounted on
> %s\n",
> - mntopt, cg_mount_table[j].path);
> + mntopt, cg_context->mount[j].path);
> continue;
> }
>
> - strcpy(cg_mount_table[found_mnt].name, controllers[i]);
> - strcpy(cg_mount_table[found_mnt].path, ent->mnt_dir);
> + strcpy(cg_context->mount[found_mnt].name,
> + controllers[i]);
> + strcpy(cg_context->mount[found_mnt].path, ent->mnt_dir);
> cgroup_dbg("Found cgroup option %s, count %d\n",
> ent->mnt_opts, found_mnt);
> found_mnt++;
> @@ -797,15 +808,15 @@ int cgroup_init(void)
> free(temp_ent);
>
> if (!found_mnt) {
> - cg_mount_table[0].name[0] = '\0';
> + cg_context->mount[0].name[0] = '\0';
> ret = ECGROUPNOTMOUNTED;
> goto unlock_exit;
> }
>
> found_mnt++;
> - cg_mount_table[found_mnt].name[0] = '\0';
> + cg_context->size = found_mnt;
> + *cgroup_context = cg_context;
>
> - cgroup_initialized = 1;
>
> unlock_exit:
> if (proc_cgroup)
> @@ -819,7 +830,8 @@ unlock_exit:
> controllers[i] = NULL;
> }
>
> - pthread_rwlock_unlock(&cg_mount_table_lock);
> + if (ret)
> + cgroup_free_context(&cg_context);
maybe also *cgroup_context=NULL, just to be sure?
>
> return ret;
> }
> Index: libcg/src/libcgroup-internal.h
> ===================================================================
> --- libcg.orig/src/libcgroup-internal.h
> +++ libcg/src/libcgroup-internal.h
> @@ -135,6 +135,8 @@ struct cgroup_context_s {
> unsigned size;
> };
>
> +struct cgroup_context_s *cgroup_context;
This should probably go to deprecated-api.c, we don't want anyone to use
it except the deprecated API.
> +
> /**
> * per thread errno variable, to be used when return code is ECGOTHER
> */
> @@ -171,6 +173,11 @@ int cgroup_config_insert_into_mount_tabl
> int cgroup_config_insert_into_namespace_table(char *name, char
> *mount_point);
> void cgroup_config_cleanup_mount_table(void);
> void cgroup_config_cleanup_namespace_table(void);
> +
> +/*
> + * Deprecated Stuff
> + */
> +extern int cgroup_initialized;
> __END_DECLS
>
> #endif
> Index: libcg/src/deprecated-api.c
> ===================================================================
> --- /dev/null
> +++ libcg/src/deprecated-api.c
> @@ -0,0 +1,66 @@
> +/*
> + * Copyright Dhaval Giani, 2010
> + *
> + * Author: Dhaval Giani<[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of version 2.1 of the GNU Lesser General Public License
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it would be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
> + */
> +
> +#include<libcgroup.h>
> +#include<libcgroup-internal.h>
> +#include<string.h>
> +
> +/* Check if cgroup_init has been called or not. */
> +int cgroup_initialized;
> +
> +/* Namespace */
> +__thread char *cg_namespace_table[CG_CONTROLLER_MAX];
> +
> +pthread_rwlock_t cg_mount_table_lock = PTHREAD_RWLOCK_INITIALIZER;
> +struct cg_mount_table_s cg_mount_table[CG_CONTROLLER_MAX];
I understand that the three definitions above are for direct usage e.g.
by api.c, they are not deprecated.
> +struct cgroup *context_cgroup;
> +
> +/**
> + * cgroup_init(), initializes the MOUNT_POINT.
> + *
> + * This code is theoretically thread safe now. Its not really tested
> + * so it can blow up. If does for you, please let us know with your
> + * test case and we can really make it thread safe.
> + *
> + * Deprecate this call in favour of keeping local state
> + */
> +int cgroup_init(void)
> +{
> + int ret = 0;
> + int i;
> +
> + pthread_rwlock_wrlock(&cg_mount_table_lock);
> +
> + ret = cgroup_init_context(&cgroup_context);
> +
> + if (ret)
> + goto out;
> +
> + /* Move everything from state into the global table
> + * Once the changes are complte, this code goes away.
> + * This is only for the APIs not converted yet.
> + */
> + cgroup_initialized = 1;
I am not sure I understand this correctly... If these 4 patches are
accepted, will applications using cgroup_init work? It looks like they
could. Will applications using cgroup_init_context work? I have bad
feeling about it - if the comment above is correct, they probably wont,
because global cg_mount_table is not filled.
> +
> + for (i = 0; i< cgroup_context->size; i++) {
> + strcpy(cg_mount_table[i].name, cgroup_context->mount[i].name);
> + strcpy(cg_mount_table[i].path, cgroup_context->mount[i].path);
> + }
> + cg_mount_table[i].name[0] = '\0';
> +
> + pthread_rwlock_unlock(&cg_mount_table_lock);
> +out:
> + return ret;
> +}
> +
> Index: libcg/include/libcgroup/deprecated.h
> ===================================================================
> --- /dev/null
> +++ libcg/include/libcgroup/deprecated.h
> @@ -0,0 +1,26 @@
> +#ifndef _LIBCGROUP_DEPRECATED_H
> +#define _LIBCGROUP_DEPRECATED_H
> +
> +#ifndef _LIBCGROUP_H_INSIDE
> +#error "Only<libcgroup.h> should be included directly."
> +#endif
> +
> +#include<features.h>
> +
> +__BEGIN_DECLS
> +
> +/**
> + *
> + * TODO: Extend docbook
> + *
> + */
> +
> +/**
> + * Initialize libcgroup. Information about mounted hierarchies are examined
> + * and cached internally (just what's mounted where, not the groups
> themselves).
> + */
> +int cgroup_init(void);
> +
> +__END_DECLS
> +
> +#endif
> Index: libcg/include/libcgroup/init.h
> ===================================================================
> --- libcg.orig/include/libcgroup/init.h
> +++ libcg/include/libcgroup/init.h
> @@ -30,11 +30,10 @@ __BEGIN_DECLS
> * on exit.
> */
>
> -/**
> - * Initialize libcgroup. Information about mounted hierarchies are examined
> - * and cached internally (just what's mounted where, not the groups
> themselves).
> - */
> -int cgroup_init(void);
> +struct cgroup_context_s;
> +
> +int cgroup_init_context(struct cgroup_context_s **cgroup_cgroup);
> +void cgroup_free_context(struct cgroup_context_s **cg_context);
>
> /**
> * Returns path where is mounted given controller. Applications should rely
> on
> @@ -50,7 +49,6 @@ int cgroup_get_subsys_mount_point(const
> * @}
> */
>
> -struct cgroup_context_s;
> __END_DECLS
>
> #endif /* _LIBCGROUP_INIT_H */
> Index: libcg/src/Makefile.am
> ===================================================================
> --- libcg.orig/src/Makefile.am
> +++ libcg/src/Makefile.am
> @@ -7,7 +7,7 @@ CLEANFILES = lex.c parse.c parse.h
>
> INCLUDES = -I$(top_srcdir)/include
> lib_LTLIBRARIES = libcgroup.la
> -libcgroup_la_SOURCES = parse.h parse.y lex.l api.c config.c
> libcgroup-internal.h libcgroup.map wrapper.c
> +libcgroup_la_SOURCES = parse.h parse.y lex.l api.c config.c
> libcgroup-internal.h libcgroup.map wrapper.c deprecated-api.c
> libcgroup_la_LIBADD = -lpthread
> libcgroup_la_LDFLAGS = -Wl,--version-script,$(srcdir)/libcgroup.map \
> -version-number
> $(LIBRARY_VERSION_MAJOR):$(LIBRARY_VERSION_MINOR):$(LIBRARY_VERSION_RELEASE)
> Index: libcg/include/libcgroup.h
> ===================================================================
> --- libcg.orig/include/libcgroup.h
> +++ libcg/include/libcgroup.h
> @@ -19,11 +19,12 @@
> #define _LIBCGROUP_H_INSIDE
>
> #include<libcgroup/error.h>
> -#include<libcgroup/init.h>
> #include<libcgroup/iterators.h>
> #include<libcgroup/groups.h>
> #include<libcgroup/tasks.h>
> #include<libcgroup/config.h>
> +#include<libcgroup/init.h>
> +#include<libcgroup/deprecated.h>
>
> #undef _LIBCGROUP_H_INSIDE
>
> Index: libcg/include/Makefile.am
> ===================================================================
> --- libcg.orig/include/Makefile.am
> +++ libcg/include/Makefile.am
> @@ -1,2 +1,2 @@
> # Using 'nobase_', we what groups.h in/usr/include/libcgroup/ directory
> -nobase_include_HEADERS = libcgroup.h libcgroup/error.h libcgroup/init.h
> libcgroup/groups.h libcgroup/tasks.h libcgroup/iterators.h libcgroup/config.h
> +nobase_include_HEADERS = libcgroup.h libcgroup/error.h libcgroup/init.h
> libcgroup/groups.h libcgroup/tasks.h libcgroup/iterators.h libcgroup/config.h
> libcgroup/deprecated.h
> Index: libcg/src/libcgroup.map
> ===================================================================
> --- libcg.orig/src/libcgroup.map
> +++ libcg/src/libcgroup.map
> @@ -91,4 +91,6 @@ CGROUP_0.36 {
>
> CGROUP_0.37 {
> cgroup_get_procs;
> + cgroup_init_context;
> + cgroup_free_context;
> } CGROUP_0.36;
>
>
>
> ------------------------------------------------------------------------------
> The Palm PDK Hot Apps Program offers developers who use the
> Plug-In Development Kit to bring their C/C++ apps to Palm for a share
> of $1 Million in cash or HP Products. Visit us here for more details:
> http://ad.doubleclick.net/clk;226879339;13503038;l?
> http://clk.atdmt.com/CRS/go/247765532/direct/01/
> _______________________________________________
> Libcg-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/libcg-devel
------------------------------------------------------------------------------
The Palm PDK Hot Apps Program offers developers who use the
Plug-In Development Kit to bring their C/C++ apps to Palm for a share
of $1 Million in cash or HP Products. Visit us here for more details:
http://ad.doubleclick.net/clk;226879339;13503038;l?
http://clk.atdmt.com/CRS/go/247765532/direct/01/
_______________________________________________
Libcg-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libcg-devel
