mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kevin Klues <klue...@gmail.com>
Subject Re: Review Request 44361: Added configure flags to build with Nvidia GPU support.
Date Mon, 14 Mar 2016 01:12:26 GMT


> On March 8, 2016, 7:33 p.m., Ben Mahler wrote:
> > configure.ac, lines 957-976
> > <https://reviews.apache.org/r/44361/diff/1/?file=1280267#file1280267line957>
> >
> >     Could we flatten this to make it a bit easier to read?

There are alot of examples in configure.ac that still do it this way (e.g.:

```
AC_CHECK_HEADERS([event2/event.h],
                 [AC_CHECK_LIB([event],
                 ...
```

However, I agree that splitting them up is clearer and should still be correct since we always
error if we can't find the headers.

Updated.


> On March 8, 2016, 7:33 p.m., Ben Mahler wrote:
> > configure.ac, lines 215-216
> > <https://reviews.apache.org/r/44361/diff/1/?file=1280267#file1280267line215>
> >
> >     Could you do a sweep and capitalize NVML, Nvidia, and GPU in the comments? The
convention is to do acronyms in all caps, but we unfortunately do this for classes as well,
like URL, which we should change at some point since it leads to confusing name boundaries
(e.g. HTTPConnection would be better named as HttpConnection, this doesn't exist but it's
just an example. Another example is http:: vs JSON::, ideally namespace names are only lower
case).

Done.


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44361/#review122582
-----------------------------------------------------------


On March 14, 2016, 1:12 a.m., Kevin Klues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44361/
> -----------------------------------------------------------
> 
> (Updated March 14, 2016, 1:12 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Rob Todd, and Vikrama Ditya.
> 
> 
> Bugs: MESOS-4861
>     https://issues.apache.org/jira/browse/MESOS-4861
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is the initial commit to begin adding native support for GPUs in
> Mesos. This initial version will only include support for Nvidia GPUs
> that can be managed by the Nvidia Management Library (nvml).
> 
> The configure flags added in this commit can be used to enable Nvidia
> GPU support, as well as specify the installation directories of the
> nvml header and library files if not already installed in standard
> include/library paths on the system.
> 
> In a subsequent commit, we will use these configure flags to
> conditionally build support for Nvidia GPUs into Mesos.
> 
> 
> Diffs
> -----
> 
>   configure.ac 8e4f03593df4a8ba13f00292963e351acc3f71c1 
> 
> Diff: https://reviews.apache.org/r/44361/diff/
> 
> 
> Testing
> -------
> 
> I ran `bootstrap` to generate configure.
> 
> I then ran:
> 
> ```
> mkdir build; cd build
> ../configure --enable-nvidia-gpu-support
> ../configure --enable-nvidia-gpu-support --with-nvml-include=<path_to_headers>
> ../configure --enable-nvidia-gpu-support --with-nvml-include=<bogus_path>
> ../configure --enable-nvidia-gpu-support --with-nvml-lib=<path_to_lib>
> ../configure --enable-nvidia-gpu-support --with-nvml-lib=<bogus_path>
> ../configure --enable-nvidia-gpu-support --with-nvml-include=<bogus_path> --with-nvml-lib=<path_to_lib>
> ../configure --enable-nvidia-gpu-support --with-nvml-include=<path_to_headers>
--with-nvml-lib=<bogus_path>
> ../configure --enable-nvidia-gpu-support --with-nvml-include=<bogus_path> --with-nvml-lib=<bogus_path>
> ../configure --enable-nvidia-gpu-support --with-nvml-include=<path_to_headers>
--with-nvml-lib=<path_to_lib>
> ```
> 
> And verified the proper errors / successes in each case (only the last one is a success).
> 
> The exact command I ran in the success case for my configuration was:
> ```
> ../configure --enable-nvidia-gpu-support --with-nvml-include=/opt/nvidia-gdk/usr/include
--with-nvml-lib=/opt/nvidia-gdk/usr/src/gdk/nvml/lib
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message