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




src/slave/containerizer/mesos/isolators/gpu/isolator.cpp
Line 159 (original), 179 (patched)
<https://reviews.apache.org/r/75074/#comment315036>

    Hierarchy == none means that no hierarchy matching the subsystem exists. 
Strange that this was only checking for errors. I'll change it to return Error 
if the result is not some



src/slave/containerizer/mesos/isolators/gpu/isolator.cpp
Lines 285-311 (original), 345-370 (patched)
<https://reviews.apache.org/r/75074/#comment315038>

    There was a mistake here. The function pushes allocator.allocate futures 
into a vector as it iterates through the container states. And all the futures 
are collected outside the 
    foreach loop.
    
    Not sure if we can avoid blocking here via await() because we need to wait 
for the deviceManager->state future before we can push the allocator.allocate 
into the futures vector


- Jason Zhou


On Aug. 1, 2024, 7:42 p.m., Jason Zhou wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/75074/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2024, 7:42 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Currently, the GPU isolator assumes we are only using cgroups v1, and
> makes use of the cgroups::devices::allow and deny functions to control
> GPU access.
> 
> In Cgroups2, we need to attach ebpf programs for the specific cgroups,
> which is done for us in the DeviceManager. Hence, we need to use the
> DeviceManager in the GPU isolator depending on whether cgroups v1 or v2
> is currently mounted
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 9bafb137ba67bfed5cfb24c1a3af9203fde9e821 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.hpp 
> e4f221d5f63ed1db044acabbbd427a30a0f69ced 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
> 99119f938e2eb5ca6a8b64d073c87ca5032a00b8 
> 
> 
> Diff: https://reviews.apache.org/r/75074/diff/3/
> 
> 
> Testing
> -------
> 
> Existing GPU isolator tests pass, with the exception of the 
> DefaultExecutorVerifyDeviceAccess test which is because we currently don't 
> support nested containers.
> 
> 
> Thanks,
> 
> Jason Zhou
> 
>

Reply via email to