> On Nov. 19, 2012, 5:05 p.m., Vinod Kone wrote: > > src/slave/cgroups_isolation_module.hpp, line 62 > > <https://reviews.apache.org/r/8108/diff/1/?file=191732#file191732line62> > > > > you call this 'usage' when used as a parameter to grow() , but call it > > 'cpus' here. > > > > how about calling to 'global' and 'local' respectively? > > Ben Mahler wrote: > I guess this wasn't as obvious as I anticipated. I've added jie-style > docs to CPUSet to make this more clear. > > I'm going to drop this with the following justification: > > CPUSet holds cpus, these are local to the CPUSet. There can be many > CPUSets (each CgroupInfo would contain one). > CgroupIsolationModule holds cpus, these are all the cpus. > > In the context you mentioned, the CgroupIsolationModule.cpus is passed > into grow(), so that the CPUSet can know about the cpu "usage". > > So, I think they both make sense to be named as cpus (since the context > differentiates them). I also liked usage because the Cgroup needs to know > about the cpu "usage" when performing new allocations. > > Please re-open if you disagree, and let me know if you have more naming > suggestions ;) > > > > Vinod Kone wrote: > My only problem is that you are using 3 different terms to refer to the > same thing "cpus", "usage" and "allocation" in your code/doc. It kinda gets > confusing, without looking at the docs.
I'd argue they are not the same thing though: cpus: the cpus belonging to this CPUSet allocation: the amount we've newly allocated during a grow() usage: the cpu usage If it's just me and these are confusing for people I'd be happy to rename, but I'd like short names that are clear based on the context (which is what I thought I had set up here ;)) - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8108/#review13570 ----------------------------------------------------------- On Nov. 19, 2012, 11:03 p.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8108/ > ----------------------------------------------------------- > > (Updated Nov. 19, 2012, 11:03 p.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Description > ------- > > This is the first pass at adding cpuset isolation for pinning cgroups to cpus. > > We decided to start with a simplistic grow/shrink allocation technique, as > such this initial technique: > -Does not take cache locality into account. > -Does not actively fight fragmentation*, but does a good job at preventing > it in many cases, given it's simplicity. > -Note that when cpus resource requests are integral (non-fractional), then > fragmentation does not occur. > > *By fragmentation, I'm referring to the case where we've spread a cgroup over > more cpus than necessary, due to other cgroups sharing the same cpus. > High fragmentation would mean a lot of shared cpus across cgroups. > No fragmentation would mean each cgroup has a unique set of cpus. > > I've punted on documenting the pitfalls of this technique, wiring up the > handler, and adding tests for now. > > Note that this is diffed off of benh's changes: > https://reviews.apache.org/r/8058/ > https://reviews.apache.org/r/8059/ > > > Diffs > ----- > > src/linux/proc.hpp 27e15bf8695aa694b0d5bdb6881b9fa55a447528 > src/slave/cgroups_isolation_module.hpp > 9f80fc5a969b959b34eaea4cac40700662d7f8b2 > src/slave/cgroups_isolation_module.cpp > 8211618d7729350654e2d17946c5b912ed9dda6a > third_party/libprocess/include/stout/stringify.hpp > dcc5b95a54e6f34f93867e015d8c855fd7d6f950 > third_party/libprocess/include/stout/strings.hpp > 914c280a994733764957d19f37b48d151bb93778 > > Diff: https://reviews.apache.org/r/8108/diff/ > > > Testing > ------- > > None as of yet. > > > Thanks, > > Ben Mahler > >
