You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ian Downes <ia...@gmail.com> on 2014/06/24 01:25:13 UTC
Review Request 22898: Add check for perf support.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22898/
-----------------------------------------------------------
Review request for mesos, Ben Mahler and Vinod Kone.
Bugs: MESOS-1394
https://issues.apache.org/jira/browse/MESOS-1394
Repository: mesos-git
Description
-------
Adds a supported() check for perf that requires version >= 2.6.39.
Diffs
-----
src/linux/perf.hpp 0d510c5fa273ef1170b892efc5d610d5bcabe7db
src/linux/perf.cpp fc5acf4dbef13fc1f8112f4dca2c5444c0edcdfa
src/slave/containerizer/isolators/cgroups/perf_event.cpp 1bd5dfaa915ed6ea01a7d5c1d914ea6a78c80c90
Diff: https://reviews.apache.org/r/22898/diff/
Testing
-------
make check # Linux
Thanks,
Ian Downes
Re: Review Request 22898: Add check for perf support.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22898/#review46482
-----------------------------------------------------------
Two things:
(1) How does this surface to the operator? Will we dump a stack trace when it's not supported or will we exit cleanly with a message? I think we should do the latter.
(2) Do we need the error case, since we can assume linux? (See my comments below).
src/linux/perf.hpp
<https://reviews.apache.org/r/22898/#comment81867>
// Returns whether the given events are accepted by `perf stat`.
src/linux/perf.hpp
<https://reviews.apache.org/r/22898/#comment81866>
I think it's important to describe what this is returning, since it's telling us whether it's supported:
// Returns whether perf is supported on this host.
If we say "check if perf is supported" that reads like a block of code that decides to exit or bail if perf is not supported, no?
src/linux/perf.cpp
<https://reviews.apache.org/r/22898/#comment81868>
else if?
src/linux/perf.cpp
<https://reviews.apache.org/r/22898/#comment81869>
ditto
src/linux/perf.cpp
<https://reviews.apache.org/r/22898/#comment81870>
period
src/linux/perf.cpp
<https://reviews.apache.org/r/22898/#comment81871>
Is this error possible if we're on a linux machine? I'm wondering if we can just CHECK_SOME and avoid the error / failure propagation altogether.
On linux, I only see:
ERRORS
EFAULT buf is not valid.
- Ben Mahler
On June 23, 2014, 11:25 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22898/
> -----------------------------------------------------------
>
> (Updated June 23, 2014, 11:25 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1394
> https://issues.apache.org/jira/browse/MESOS-1394
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds a supported() check for perf that requires version >= 2.6.39.
>
>
> Diffs
> -----
>
> src/linux/perf.hpp 0d510c5fa273ef1170b892efc5d610d5bcabe7db
> src/linux/perf.cpp fc5acf4dbef13fc1f8112f4dca2c5444c0edcdfa
> src/slave/containerizer/isolators/cgroups/perf_event.cpp 1bd5dfaa915ed6ea01a7d5c1d914ea6a78c80c90
>
> Diff: https://reviews.apache.org/r/22898/diff/
>
>
> Testing
> -------
>
> make check # Linux
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 22898: Add check for perf support.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22898/#review46696
-----------------------------------------------------------
Ship it!
Ship It!
- Ben Mahler
On June 24, 2014, 11:12 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22898/
> -----------------------------------------------------------
>
> (Updated June 24, 2014, 11:12 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1394
> https://issues.apache.org/jira/browse/MESOS-1394
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds a supported() check for perf that requires version >= 2.6.39.
>
>
> Diffs
> -----
>
> src/linux/perf.hpp 0d510c5fa273ef1170b892efc5d610d5bcabe7db
> src/linux/perf.cpp fc5acf4dbef13fc1f8112f4dca2c5444c0edcdfa
> src/slave/containerizer/isolators/cgroups/perf_event.cpp 1bd5dfaa915ed6ea01a7d5c1d914ea6a78c80c90
>
> Diff: https://reviews.apache.org/r/22898/diff/
>
>
> Testing
> -------
>
> make check # Linux
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 22898: Add check for perf support.
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22898/#review46594
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On June 24, 2014, 11:12 p.m., Ian Downes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22898/
> -----------------------------------------------------------
>
> (Updated June 24, 2014, 11:12 p.m.)
>
>
> Review request for mesos, Ben Mahler and Vinod Kone.
>
>
> Bugs: MESOS-1394
> https://issues.apache.org/jira/browse/MESOS-1394
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds a supported() check for perf that requires version >= 2.6.39.
>
>
> Diffs
> -----
>
> src/linux/perf.hpp 0d510c5fa273ef1170b892efc5d610d5bcabe7db
> src/linux/perf.cpp fc5acf4dbef13fc1f8112f4dca2c5444c0edcdfa
> src/slave/containerizer/isolators/cgroups/perf_event.cpp 1bd5dfaa915ed6ea01a7d5c1d914ea6a78c80c90
>
> Diff: https://reviews.apache.org/r/22898/diff/
>
>
> Testing
> -------
>
> make check # Linux
>
>
> Thanks,
>
> Ian Downes
>
>
Re: Review Request 22898: Add check for perf support.
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22898/
-----------------------------------------------------------
(Updated June 24, 2014, 4:12 p.m.)
Review request for mesos, Ben Mahler and Vinod Kone.
Changes
-------
Simplified and addressed comments.
Bugs: MESOS-1394
https://issues.apache.org/jira/browse/MESOS-1394
Repository: mesos-git
Description
-------
Adds a supported() check for perf that requires version >= 2.6.39.
Diffs (updated)
-----
src/linux/perf.hpp 0d510c5fa273ef1170b892efc5d610d5bcabe7db
src/linux/perf.cpp fc5acf4dbef13fc1f8112f4dca2c5444c0edcdfa
src/slave/containerizer/isolators/cgroups/perf_event.cpp 1bd5dfaa915ed6ea01a7d5c1d914ea6a78c80c90
Diff: https://reviews.apache.org/r/22898/diff/
Testing
-------
make check # Linux
Thanks,
Ian Downes