> On June 11, 2016, 2:27 p.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp, lines 37-38 > > <https://reviews.apache.org/r/45574/diff/7/?file=1347677#file1347677line37> > > > > So even after we introduce the unified cgroups isolator, we still need > > to keep the code of the original `cgroups/xxx` isolators? But I think it > > does not make sense since we have consolidated their code into the unified > > cgroups isolator, so ideally under > > `src/slave/containerizer/mesos/isolators/cgroups`, we will only have > > `cgroups.hpp/cpp` and `subsystem.hpp/cpp`, right?
Yes, those legacy files would be clean up, include `cgroups/perf_event.hpp`. Now I include it because I don't want to move or copy `PerfEventHandleManager` in this patch. > On June 11, 2016, 2:27 p.m., Qian Zhang wrote: > > src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp, lines 57-75 > > <https://reviews.apache.org/r/45574/diff/7/?file=1347678#file1347678line57> > > > > In this method, I think you missed the part that we need to ensure no > > other subsystem is attached to the hierarchy, please check the the > > following code as a reference, and actually > > `CgroupsCpushareIsolatorProcess::create()`, > > `CgroupsNetClsIsolatorProcess::create()` also have the similar logic. > > > > https://github.com/apache/mesos/blob/0.28.2/src/slave/containerizer/mesos/isolators/cgroups/mem.cpp#L96:L105 We not need check this because the purpose of this epic is to allow multiple subsystems attached to the same hierarchy. - haosdent ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45574/#review137118 ----------------------------------------------------------- On June 19, 2016, 10:32 a.m., haosdent huang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45574/ > ----------------------------------------------------------- > > (Updated June 19, 2016, 10:32 a.m.) > > > Review request for mesos, Gilbert Song, Guangya Liu, Ian Downes, Jie Yu, > Kevin Klues, and Qian Zhang. > > > Bugs: MESOS-5047 > https://issues.apache.org/jira/browse/MESOS-5047 > > > Repository: mesos > > > Description > ------- > > Add `PerfEventSubsystem` for cgroups unified isolator. > > > Diffs > ----- > > src/linux/perf.hpp 674d5f886ea41b939a8e48832ee6595a78b2f6ce > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION > src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION > src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/45574/diff/ > > > Testing > ------- > > > Thanks, > > haosdent huang > >
