You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by bm...@apache.org on 2016/06/14 20:42:20 UTC
mesos git commit: Fixed hard-coding of Nvidia control device numbers.
Repository: mesos
Updated Branches:
refs/heads/master e29ce1008 -> 3e9df075c
Fixed hard-coding of Nvidia control device numbers.
Previously, the major/minor numbers of `/dev/nvidiactl` and
`/dev/nvidia-uvm` were hard-coded. This actually caused problems for
`/dev/nvidia-uvm` because its major number is part of the
"Experimental" device range on Linux.
Because this range is experimental, there is no guarantee which device
number will be assigned to it on a given machine. We actually
encountered this problem in the wild, prompting this change.
We now use `os:stat::rdev()` to extract the major/minor numbers
programatically.
Review: https://reviews.apache.org/r/48370/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/3e9df075
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/3e9df075
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/3e9df075
Branch: refs/heads/master
Commit: 3e9df075c585270faee802380463c1e1236509a0
Parents: e29ce10
Author: Kevin Klues <kl...@gmail.com>
Authored: Tue Jun 14 13:38:13 2016 -0700
Committer: Benjamin Mahler <bm...@apache.org>
Committed: Tue Jun 14 13:40:17 2016 -0700
----------------------------------------------------------------------
.../isolators/cgroups/devices/gpus/nvidia.cpp | 88 ++++++++++----------
.../isolators/cgroups/devices/gpus/nvidia.hpp | 9 +-
2 files changed, 54 insertions(+), 43 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/3e9df075/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp b/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp
index 1a04126..b8c9889 100644
--- a/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp
+++ b/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp
@@ -67,49 +67,18 @@ namespace slave {
// TODO(klueska): Expand this when we support other GPU types.
static constexpr dev_t NVIDIA_MAJOR_DEVICE = 195;
-// We also need to grant/revoke access to both /dev/nvidiactl
-// and /dev/nvidia-uvm when we grant/revoke access to GPUs in
-// a container.
-//
-// TODO(klueska): For now, we hard code the major/minor
-// numbers for these devices. In the future we should use
-// stat() on the device path.
-static const cgroups::devices::Entry* NVIDIA_CTL_DEVICE_ENTRY =
- new cgroups::devices::Entry({
- .selector = {
- .type = Entry::Selector::Type::CHARACTER,
- .major = NVIDIA_MAJOR_DEVICE,
- .minor = 255,
- },
- .access = {
- .read = true,
- .write = true,
- .mknod = true,
- }
- });
-
-static const cgroups::devices::Entry* NVIDIA_UVM_DEVICE_ENTRY =
- new cgroups::devices::Entry({
- .selector = {
- .type = Entry::Selector::Type::CHARACTER,
- .major = 246, // Nvidia uses the local/experimental device range.
- .minor = 0,
- },
- .access = {
- .read = true,
- .write = true,
- .mknod = true,
- }
- });
-
CgroupsNvidiaGpuIsolatorProcess::CgroupsNvidiaGpuIsolatorProcess(
const Flags& _flags,
const string& _hierarchy,
- list<Gpu> gpus)
+ const list<Gpu>& gpus,
+ const cgroups::devices::Entry& uvmDeviceEntry,
+ const cgroups::devices::Entry& ctlDeviceEntry)
: flags(_flags),
hierarchy(_hierarchy),
- available(gpus) {}
+ available(gpus),
+ NVIDIA_CTL_DEVICE_ENTRY(ctlDeviceEntry),
+ NVIDIA_UVM_DEVICE_ENTRY(uvmDeviceEntry) {}
CgroupsNvidiaGpuIsolatorProcess::~CgroupsNvidiaGpuIsolatorProcess()
@@ -192,8 +161,43 @@ Try<Isolator*> CgroupsNvidiaGpuIsolatorProcess::create(const Flags& flags)
hierarchy.error());
}
+ // Create the device entries for
+ // `/dev/nvidiactl` and `/dev/nvidia-uvm`.
+ Try<dev_t> device = os::stat::rdev("/dev/nvidiactl");
+ if (device.isError()) {
+ return Error("Failed to obtain device ID for '/dev/nvidiactl': " +
+ device.error());
+ }
+
+ cgroups::devices::Entry ctlDeviceEntry;
+ ctlDeviceEntry.selector.type = Entry::Selector::Type::CHARACTER;
+ ctlDeviceEntry.selector.major = major(device.get());
+ ctlDeviceEntry.selector.minor = minor(device.get());
+ ctlDeviceEntry.access.read = true;
+ ctlDeviceEntry.access.write = true;
+ ctlDeviceEntry.access.mknod = true;
+
+ device = os::stat::rdev("/dev/nvidia-uvm");
+ if (device.isError()) {
+ return Error("Failed to obtain device ID for '/dev/nvidia-uvm': " +
+ device.error());
+ }
+
+ cgroups::devices::Entry uvmDeviceEntry;
+ uvmDeviceEntry.selector.type = Entry::Selector::Type::CHARACTER;
+ uvmDeviceEntry.selector.major = major(device.get());
+ uvmDeviceEntry.selector.minor = minor(device.get());
+ uvmDeviceEntry.access.read = true;
+ uvmDeviceEntry.access.write = true;
+ uvmDeviceEntry.access.mknod = true;
+
process::Owned<MesosIsolatorProcess> process(
- new CgroupsNvidiaGpuIsolatorProcess(flags, hierarchy.get(), gpus));
+ new CgroupsNvidiaGpuIsolatorProcess(
+ flags,
+ hierarchy.get(),
+ gpus,
+ ctlDeviceEntry,
+ uvmDeviceEntry));
return new MesosIsolator(process);
}
@@ -303,14 +307,14 @@ Future<Nothing> CgroupsNvidiaGpuIsolatorProcess::update(
// Grant access from /dev/nvidiactl and /dev/nvida-uvm
// if this container no longer has access to any GPUs.
if (info->allocated.empty()) {
- map<string, const cgroups::devices::Entry*> entries = {
+ map<string, cgroups::devices::Entry> entries = {
{ "/dev/nvidiactl", NVIDIA_CTL_DEVICE_ENTRY },
{ "/dev/nvidia-uvm", NVIDIA_UVM_DEVICE_ENTRY },
};
foreachkey (const string& device, entries) {
Try<Nothing> allow = cgroups::devices::allow(
- hierarchy, info->cgroup, *entries[device]);
+ hierarchy, info->cgroup, entries[device]);
if (allow.isError()) {
return Failure("Failed to grant cgroups access to"
@@ -370,14 +374,14 @@ Future<Nothing> CgroupsNvidiaGpuIsolatorProcess::update(
// Revoke access from /dev/nvidiactl and /dev/nvida-uvm
// if this container no longer has access to any GPUs.
if (info->allocated.empty()) {
- map<string, const cgroups::devices::Entry*> entries = {
+ map<string, cgroups::devices::Entry> entries = {
{ "/dev/nvidiactl", NVIDIA_CTL_DEVICE_ENTRY },
{ "/dev/nvidia-uvm", NVIDIA_UVM_DEVICE_ENTRY },
};
foreachkey (const string& device, entries) {
Try<Nothing> deny = cgroups::devices::deny(
- hierarchy, info->cgroup, *entries[device]);
+ hierarchy, info->cgroup, entries[device]);
if (deny.isError()) {
return Failure("Failed to deny cgroups access to"
http://git-wip-us.apache.org/repos/asf/mesos/blob/3e9df075/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp
----------------------------------------------------------------------
diff --git a/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp b/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp
index a67887a..7e38c8e 100644
--- a/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp
+++ b/src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.hpp
@@ -29,6 +29,8 @@
#include <stout/option.hpp>
#include <stout/try.hpp>
+#include "linux/cgroups.hpp"
+
#include "slave/flags.hpp"
#include "slave/containerizer/mesos/isolator.hpp"
@@ -120,7 +122,9 @@ private:
CgroupsNvidiaGpuIsolatorProcess(
const Flags& _flags,
const std::string& hierarchy,
- std::list<Gpu> gpus);
+ const std::list<Gpu>& gpus,
+ const cgroups::devices::Entry& ctlDeviceEntry,
+ const cgroups::devices::Entry& uvmDeviceEntry);
const Flags flags;
@@ -131,6 +135,9 @@ private:
hashmap<ContainerID, Info*> infos;
std::list<Gpu> available;
+
+ const cgroups::devices::Entry NVIDIA_CTL_DEVICE_ENTRY;
+ const cgroups::devices::Entry NVIDIA_UVM_DEVICE_ENTRY;
};
} // namespace slave {