* Dhaval Giani <[email protected]> [2009-04-21 12:05:55]:
> On Tue, Apr 21, 2009 at 11:57:34AM +0530, Balbir Singh wrote:
> > * Dhaval Giani <[email protected]> [2009-04-20 19:27:36]:
> >
> > > Add new APIs to iterate through the tasks file to get
> > > the list of all the tasks in a cgroup.
> > >
> > > Signed-off-by: Dhaval Giani <[email protected]>
> > >
> > > ---
> > > include/libcgroup.h | 4 ++
> > > src/api.c | 88
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > src/libcgroup.map | 6 +++
> > > 3 files changed, 98 insertions(+)
> > >
> > > Index: libcg/include/libcgroup.h
> > > ===================================================================
> > > --- libcg.orig/include/libcgroup.h
> > > +++ libcg/include/libcgroup.h
> > > @@ -274,6 +274,10 @@ int cgroup_read_stats_next(void **handle
> > >
> > > int cgroup_read_stats_end(void **handle);
> > >
> > > +int cgroup_get_task_begin(char *cgroup, char *controller, void **handle,
> > > + pid_t *pid);
> > > +int cgroup_get_task_next(void **handle, pid_t *pid);
> > > +int cgroup_get_task_end(void **handle);
> >
> > I want to see docbook style comments for each function with each
> > parameter documented.
> >
>
> Will do. Would you prefer a delta, or a respin?
>
Respin, please
> > > /* The wrappers for filling libcg structures */
> > >
> > > struct cgroup *cgroup_new_cgroup(const char *name);
> > > Index: libcg/src/api.c
> > > ===================================================================
> > > --- libcg.orig/src/api.c
> > > +++ libcg/src/api.c
> > > @@ -2407,3 +2407,91 @@ int cgroup_read_stats_begin(char *contro
> > > *handle = fp;
> > > return ret;
> > > }
> > > +
> > > +int cgroup_get_task_end(void **handle)
> > > +{
> > > + FILE *tasks;
> > > +
> > > + if (!cgroup_initialized)
> > > + return ECGROUPNOTINITIALIZED;
> > > +
> >
> > I think we should encapsulate this in an inline function in the future
> >
> > > + if (!handle)
> > > + return ECGINVAL;
> > > +
> > > + tasks = (FILE *) *handle;
> > ^
> > Extra space here
>
> hmm. checkpatch did not crib about it. Isn't this more readable?
>
Not to me.. I like having no space after casts. Coding style is not
clear on that.
> >
> > > +
> > > + if (!tasks)
> > > + return ECGINVAL;
> > > +
> > > + fclose(tasks);
> > > + tasks = NULL;
> > > + *handle = tasks;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +int cgroup_get_task_next(void **handle, pid_t *pid)
> > > +{
> > > + FILE *tasks;
> > > + int ret;
> > > +
> > > + if (!cgroup_initialized)
> > > + return ECGROUPNOTINITIALIZED;
> > > +
> > > + if (!handle)
> > > + return ECGINVAL;
> > > +
> > > + tasks = (FILE *) *handle;
> > > +
> > See above
> >
> > > + if (feof(tasks))
> > > + return ECGEOF;
> > > +
> > > + ret = fscanf(tasks, "%u", pid);
> > > +
> > > + if (ret != 1) {
> > > + *handle = tasks;
> > > + if (feof(tasks))
> > > + return ECGEOF;
> > > + last_errno = errno;
> > > + return ECGOTHER;
> > > + }
> > > +
> > > + *handle = tasks;
> >
> > Do we need to do this all the time? Shouldn't doing this at begin be
> > enough?
> >
>
> apparently not. my testing was failing at this very point. So I added
> it.
Really? Overwriting the handle each time? Something is broken, the
handle should refer to the same pointer, unless the code is doing
something wrong. What problem did you see?
>
> > > + return 0;
> > > +}
> > > +
> > > +int cgroup_get_task_begin(char *cgroup, char *controller, void **handle,
> > > + pid_t *pid)
> > > +{
> > > + int ret = 0;
> > > + char path[FILENAME_MAX];
> > > + FILE *tasks = (FILE *) *handle;
> > > + char *fullpath = NULL;
> > > +
> > > + if (!cgroup_initialized)
> > > + return ECGROUPNOTINITIALIZED;
> > > +
> > > + if (!cg_build_path(cgroup, path, controller))
> > > + return ECGOTHER;
> >
> > I suspect path is relative, if so please document in the the header
> > file (see earlier comment).
> >
>
> Doesn't matter. path is internal. I am not sure why that is needed.
>
> > > +
> > > + ret = asprintf(&fullpath, "%s/tasks", path);
> > > +
> > > + if (ret < 0) {
> > > + last_errno = errno;
> > > + return ECGOTHER;
> > > + }
> > > +
> > > + tasks = fopen(fullpath, "r");
> > > + free(fullpath);
> > > + fullpath = NULL;
> > > +
> > > + if (!tasks) {
> > > + last_errno = errno;
> > > + return ECGOTHER;
> > > + }
> > > +
> > > + ret = cgroup_get_task_next(&tasks, pid);
> > > + *handle = tasks;
> > > +
> > > + return ret;
> > > +}
> > > Index: libcg/src/libcgroup.map
> > > ===================================================================
> > > --- libcg.orig/src/libcgroup.map
> > > +++ libcg/src/libcgroup.map
> > > @@ -57,3 +57,9 @@ global:
> > > cgroup_read_stats_end;
> > > } CGROUP_0.32.1;
> > >
> > > +CGROUP_0.34 {
> > > +global:
> > > + cgroup_get_task_begin;
> > > + cgroup_get_task_end;
> > > + cgroup_get_task_next;
> > > +} CGROUP_0.33;
> > >
> > >
> > >
> >
> > --
> > Balbir
>
> --
> regards,
> Dhaval
>
--
Balbir
------------------------------------------------------------------------------
Stay on top of everything new and different, both inside and
around Java (TM) technology - register by April 22, and save
$200 on the JavaOne (SM) conference, June 2-5, 2009, San Francisco.
300 plus technical and hands-on sessions. Register today.
Use priority code J9JMT32. http://p.sf.net/sfu/p
_______________________________________________
Libcg-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/libcg-devel
