You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2012/05/30 07:17:23 UTC
Re: Review Request: Add basic functions to operate Linux control groups
(cgroups) directly.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5230/
-----------------------------------------------------------
(Updated 2012-05-30 05:17:23.183514)
Review request for mesos and Benjamin Hindman.
Summary
-------
These functions are very basic functions that are required to control Linux cgroups. See src/linux.cgroups.h|cpp for details.
This patch should be applied after https://reviews.apache.org/r/5186/ is applied.
Diffs
-----
src/Makefile.am 96cb4c6
src/linux/cgroups.hpp PRE-CREATION
src/linux/cgroups.cpp PRE-CREATION
src/tests/cgroups_tests.cpp PRE-CREATION
Diff: https://reviews.apache.org/r/5230/diff
Testing
-------
On Linux machines, make check.
Manual tests for some functions since root permission is required.
Thanks,
Jie
Re: Review Request: Add basic functions to operate Linux control groups
(cgroups) directly.
Posted by Jie Yu <yu...@gmail.com>.
> On 2012-05-30 17:39:15, Benjamin Hindman wrote:
> > src/linux/cgroups.hpp, line 51
> > <https://reviews.apache.org/r/5230/diff/2/?file=110712#file110712line51>
> >
> > Move the '{' to a new line (here and the rest of this patch please).
Done.
> On 2012-05-30 17:39:15, Benjamin Hindman wrote:
> > src/linux/cgroups.hpp, line 56
> > <https://reviews.apache.org/r/5230/diff/2/?file=110712#file110712line56>
> >
> > If you want to expose this information then we need to add more documentation and make it "understandable". In particular, all this code tells me is that there are multiple "entries" in /proc/cgroups and that each entry has a subsystem name, hierarchy, number of cgroups, and enabled bit. But given just that I do not know how any other semantics. For example, is there an entry for each subsystem that is present in each hierarchy? Your map of 'entries' below would tell me no, but then I'm totally confused by why there is a hierarchy number (I thought there could be multiple hierarchies and a subsystem could be attached to more than one hierarchy if it was the only subsystem attached to each hierarchy). Again, I think we want some higher-level abstractions here. In particular, I think we want to be able to "ask" certain questions. For example, 'what subsystems are enabled on this machine?' (e.g., cgroups::subsystems() as you have below). Or, given a hierarchy, 'what subsystems are attached to that hierarchy?'. Or, given a cgroup, 'what hierarchy is it a part of?'. Or, given a task, 'which cgroups is it a part of?'. From just these one could even construct a "cross-cutting" question, e.g., what subsystems are controlling this task (determined by (1) getting cgroups the task is a part of then (2) what hierarchy the cgroups are a part of and then (3) what subsystems are attached to that hierarchy). My guess is that these will be much more useful and intuitive then forcing clients of this API to work with the low-level /proc/cgroups format.
In fact, I don't want to expose these to user. How to make them internal to this module? static for functions? what about structs?
Here is my plan:
(1) I want to make the following functions internal to this module: (internal functions)
info/info(pid)
createHierarch/removeHierarchy
createCgroup/removeCgroup
readControl/writeControl
(2) I want to expose the following functions to user: (external functions)
subsystems()
enabled()
enabled(subsystem)
...
(there will be more like you suggested, but these functions will use the above internal functions)
I am currently working on cgroups_isolation_module. More external functions will be added to this module to satisfy the needs of the isolation module.
--- "I thought there could be multiple hierarchies and a subsystem could be attached to more than one hierarchy if it was the only subsystem attached to each hierarchy"'
No, a subsystem can only be attached to one hierarchy. But a hierarchy can have more than one subsystems been attached.
> On 2012-05-30 17:39:15, Benjamin Hindman wrote:
> > src/linux/cgroups.hpp, line 164
> > <https://reviews.apache.org/r/5230/diff/2/?file=110712#file110712line164>
> >
> > Just a thought. My hunch is that the cgroups_isolation_module will probably just use a single hierarchy. It's possible that when we introduce net_cls we'll have multiple hierarchies, one for each network class we care about. But for now, it seems like we want all subsytems attached to a single hierarchy. Or maybe I'm missing something?
Yes, you are right. We will just create a single hierarchy root (e.g. /mnt/cgroups) and attach all subsystems (e.g. cpu, memory) to this hierarchy for now.
> On 2012-05-30 17:39:15, Benjamin Hindman wrote:
> > src/linux/cgroups.cpp, line 43
> > <https://reviews.apache.org/r/5230/diff/2/?file=110713#file110713line43>
> >
> > Again, let's not use 'rv' here. In this case, you can just call this thing 'info'.
Done.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5230/#review8219
-----------------------------------------------------------
On 2012-05-30 05:17:23, Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5230/
> -----------------------------------------------------------
>
> (Updated 2012-05-30 05:17:23)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Summary
> -------
>
> These functions are very basic functions that are required to control Linux cgroups. See src/linux.cgroups.h|cpp for details.
>
> This patch should be applied after https://reviews.apache.org/r/5186/ is applied.
>
>
> Diffs
> -----
>
> src/Makefile.am 96cb4c6
> src/linux/cgroups.hpp PRE-CREATION
> src/linux/cgroups.cpp PRE-CREATION
> src/tests/cgroups_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/5230/diff
>
>
> Testing
> -------
>
> On Linux machines, make check.
>
> Manual tests for some functions since root permission is required.
>
>
> Thanks,
>
> Jie
>
>
Re: Review Request: Add basic functions to operate Linux control groups
(cgroups) directly.
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/5230/#review8219
-----------------------------------------------------------
src/linux/cgroups.hpp
<https://reviews.apache.org/r/5230/#comment17803>
Move the '{' to a new line (here and the rest of this patch please).
src/linux/cgroups.hpp
<https://reviews.apache.org/r/5230/#comment17817>
If you want to expose this information then we need to add more documentation and make it "understandable". In particular, all this code tells me is that there are multiple "entries" in /proc/cgroups and that each entry has a subsystem name, hierarchy, number of cgroups, and enabled bit. But given just that I do not know how any other semantics. For example, is there an entry for each subsystem that is present in each hierarchy? Your map of 'entries' below would tell me no, but then I'm totally confused by why there is a hierarchy number (I thought there could be multiple hierarchies and a subsystem could be attached to more than one hierarchy if it was the only subsystem attached to each hierarchy). Again, I think we want some higher-level abstractions here. In particular, I think we want to be able to "ask" certain questions. For example, 'what subsystems are enabled on this machine?' (e.g., cgroups::subsystems() as you have below). Or, given a hierarchy, 'what subsystems are attached to that hierarchy?'. Or, given a cgroup, 'what hierarchy is it a part of?'. Or, given a task, 'which cgroups is it a part of?'. From just these one could even construct a "cross-cutting" question, e.g., what subsystems are controlling this task (determined by (1) getting cgroups the task is a part of then (2) what hierarchy the cgroups are a part of and then (3) what subsystems are attached to that hierarchy). My guess is that these will be much more useful and intuitive then forcing clients of this API to work with the low-level /proc/cgroups format.
src/linux/cgroups.hpp
<https://reviews.apache.org/r/5230/#comment17818>
Just a thought. My hunch is that the cgroups_isolation_module will probably just use a single hierarchy. It's possible that when we introduce net_cls we'll have multiple hierarchies, one for each network class we care about. But for now, it seems like we want all subsytems attached to a single hierarchy. Or maybe I'm missing something?
src/linux/cgroups.cpp
<https://reviews.apache.org/r/5230/#comment17819>
Again, let's not use 'rv' here. In this case, you can just call this thing 'info'.
- Benjamin
On 2012-05-30 05:17:23, Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/5230/
> -----------------------------------------------------------
>
> (Updated 2012-05-30 05:17:23)
>
>
> Review request for mesos and Benjamin Hindman.
>
>
> Summary
> -------
>
> These functions are very basic functions that are required to control Linux cgroups. See src/linux.cgroups.h|cpp for details.
>
> This patch should be applied after https://reviews.apache.org/r/5186/ is applied.
>
>
> Diffs
> -----
>
> src/Makefile.am 96cb4c6
> src/linux/cgroups.hpp PRE-CREATION
> src/linux/cgroups.cpp PRE-CREATION
> src/tests/cgroups_tests.cpp PRE-CREATION
>
> Diff: https://reviews.apache.org/r/5230/diff
>
>
> Testing
> -------
>
> On Linux machines, make check.
>
> Manual tests for some functions since root permission is required.
>
>
> Thanks,
>
> Jie
>
>